diff options
54 files changed, 453 insertions, 577 deletions
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index ce736c035b..8aa5c7f669 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -986,6 +986,10 @@ Gerrit will create a merge commit based on the information of MergePatchSetInput and add a new patch set to the change corresponding to the new merge commit. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the current patch set, the same email will be used as the committer email in the +new patch set; otherwise, the user's preferred email will be used. + .Request ---- POST /changes/test~master~Ic5466d107c5294414710935a8ef3b0180fb848dc/merge HTTP/1.0 @@ -1039,6 +1043,10 @@ returned that describes the resulting change. Creates a new patch set with a new commit message. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the current patch set, the same email will be used as the committer email in the +new patch set; otherwise, the user's preferred email will be used. + The new commit message must be provided in the request body inside a link:#commit-message-input[CommitMessageInput] entity. If a Change-Id footer is specified, it must match the current Change-Id footer. If @@ -1314,6 +1322,10 @@ the error message is contained in the response body. Rebases a change. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the current patch set, the same email will be used as the committer email in the +new patch set; otherwise, the user's preferred email will be used. + Optionally, the parent revision can be changed to another patch set through the link:#rebase-input[RebaseInput] entity. @@ -1440,6 +1452,10 @@ change (as approvals of the uploader are ignored). Rebases an ancestry chain of changes. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the current patch set, the same email will be used as the committer email in the +new patch set; otherwise, the user's preferred email will be used. + The operated change is treated as the chain tip. All unsubmitted ancestors are rebased. Requires a linear ancestry relation (single parenting throughout the chain). @@ -1919,6 +1935,11 @@ message is contained in the response body. Submits a change. +If the submission results in a new patch set (due to a rebase or cherry-pick merge method), the +committer email will remain the same as the one used in the previous commit, provided that one of +the secondary emails associated with the user performing the operation was used as the committer +email in the previous commit. Otherwise, the user's preferred email will be used. + The request body only needs to include a link:#submit-input[ SubmitInput] entity if submitting on behalf of another user. @@ -2276,6 +2297,10 @@ otherwise only by administrators. Creates a new patch set on a destination change from the provided patch. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the current patch set, the same email will be used as the committer email in the +new patch set; otherwise, the user's preferred email will be used. + The patch must be provided in the request body, inside a link:#applypatchpatchset-input[ApplyPatchPatchSetInput] entity. @@ -2891,6 +2916,11 @@ Adds and/or removes custom keyed values from a change. The custom keyed values to add or remove must be provided in the request body inside a link:#custom-keyed-values-input[CustomKeyedValuesInput] entity. +Note that custom keyed values are expected to be small in both key and value. +A typical use-case would be storing the ID to some external system, in which +case the key would be something unique to that system and the value would be +the ID. + .Request ---- POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/custom_keyed_values HTTP/1.0 @@ -3183,11 +3213,15 @@ provided fetch info map is also included. [[put-edit-file]] === Change file content in Change Edit -- -'PUT /changes/link:#change-id[\{change-id\}]/edit/path%2fto%2ffile +'PUT /changes/link:#change-id[\{change-id\}]/edit/path%2fto%2ffile' -- Put content of a file to a change edit. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the latest patch set, the same email will be used as the committer email in the +new change edit commit; otherwise, the user's preferred email will be used. + .Request ---- PUT /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit/foo HTTP/1.0 @@ -3234,7 +3268,7 @@ content that the file already has, '409 no changes were made' is returned. [[post-edit]] === Restore file content or rename files in Change Edit -- -'POST /changes/link:#change-id[\{change-id\}]/edit +'POST /changes/link:#change-id[\{change-id\}]/edit' -- Creates empty change edit, restores file content or renames files in change @@ -3242,6 +3276,10 @@ edit. The request body needs to include a link:#change-edit-input[ChangeEditInput] entity when a file within change edit should be restored or old and new file names to rename a file. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the latest patch set, the same email will be used as the committer email in the +new change edit commit; otherwise, the user's preferred email will be used. + .Request ---- POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit HTTP/1.0 @@ -3278,13 +3316,17 @@ old and new file names are provided, the file is renamed. As response [[put-change-edit-message]] === Change commit message in Change Edit -- -'PUT /changes/link:#change-id[\{change-id\}]/edit:message +'PUT /changes/link:#change-id[\{change-id\}]/edit:message' -- Modify commit message. The request body needs to include a link:#change-edit-message-input[ChangeEditMessageInput] entity. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the latest patch set, the same email will be used as the committer email in the +new change edit commit; otherwise, the user's preferred email will be used. + .Request ---- PUT /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit:message HTTP/1.0 @@ -3313,6 +3355,10 @@ Deletes a file from a change edit. This deletes the file from the repository completely. This is not the same as reverting or restoring a file to its previous contents. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the latest patch set, the same email will be used as the committer email in the +new change edit commit; otherwise, the user's preferred email will be used. + .Request ---- DELETE /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit/foo HTTP/1.0 @@ -3480,11 +3526,15 @@ As response "`204 No Content`" is returned. [[rebase-edit]] === Rebase Change Edit -- -'POST /changes/link:#change-id[\{change-id\}]/edit:rebase +'POST /changes/link:#change-id[\{change-id\}]/edit:rebase' -- Rebases change edit on top of latest patch set. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the latest patch set, the same email will be used as the committer email in the +new change edit commit; otherwise, the user's preferred email will be used. + .Request ---- POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit:rebase HTTP/1.0 @@ -5594,6 +5644,10 @@ it will be updated accordingly. A fix can only be applied if no change edit exists and the fix refers to the current patch set, or the fix refers to the patch set on which the change edit is based. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the current patch set, the same email will be used as the committer email in the +new change edit commit; otherwise, the user's preferred email will be used. + .Request ---- POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/fixes/8f605a55_f6aa4ecc/apply HTTP/1.0 @@ -5660,6 +5714,10 @@ Applies a list of <<fix-replacement-info,FixReplacementInfo>> loaded from the application of the fixes creates a new change edit. `Apply Provided Fix` can only be applied on the current patchset. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the current patch set, the same email will be used as the committer email in the +new change edit commit; otherwise, the user's preferred email will be used. + .Request ---- POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/fix:apply HTTP/1.0 @@ -6315,6 +6373,10 @@ Deletes the reviewed flag of the calling user from a file of a revision. Cherry picks a revision to a destination branch. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the original revision, the same email will be used as the committer email +in the new patch set; otherwise, the user's preferred email will be used. + To cherry pick a commit with no change-id associated with it, see link:rest-api-projects.html#cherry-pick-commit[CherryPickCommit]. diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt index 528be41d94..73fac68f3b 100644 --- a/Documentation/rest-api-projects.txt +++ b/Documentation/rest-api-projects.txt @@ -2667,6 +2667,10 @@ The content is returned as base64 encoded string. Cherry-picks a commit of a project to a destination branch. +If one of the secondary emails associated with the user performing the operation was used as the +committer email in the original commit, the same email will be used as the committer email in the +new patch set; otherwise, the user's preferred email will be used. + To cherry pick a change revision, see link:rest-api-changes.html#cherry-pick[CherryPick]. The destination branch must be provided in the request body inside a diff --git a/java/com/google/gerrit/server/IdentifiedUser.java b/java/com/google/gerrit/server/IdentifiedUser.java index 36d78886ec..d45d329b19 100644 --- a/java/com/google/gerrit/server/IdentifiedUser.java +++ b/java/com/google/gerrit/server/IdentifiedUser.java @@ -468,9 +468,7 @@ public class IdentifiedUser extends CurrentUser { public PersonIdent newCommitterIdent(Instant when, ZoneId zoneId) { final Account ua = getAccount(); - String name = ua.fullName(); String email = ua.preferredEmail(); - if (email == null || email.isEmpty()) { // No preferred email is configured. Use a generic identity so we // don't leak an address the user may have given us, but doesn't @@ -491,17 +489,16 @@ public class IdentifiedUser extends CurrentUser { email = user + "@" + host; } + String name = getCommitterName(ua, email); + return new PersonIdent(name, email, when, zoneId); + } - if (name == null || name.isEmpty()) { - final int at = email.indexOf('@'); - if (0 < at) { - name = email.substring(0, at); - } else { - name = anonymousCowardName; - } + public Optional<PersonIdent> newCommitterIdent(String email, Instant when, ZoneId zoneId) { + if (!hasEmailAddress(email)) { + return Optional.empty(); } - - return new PersonIdent(name, email, when, zoneId); + String name = getCommitterName(getAccount(), email); + return Optional.of(new PersonIdent(name, email, when, zoneId)); } @Override @@ -551,4 +548,17 @@ public class IdentifiedUser extends CurrentUser { public boolean hasSameAccountId(CurrentUser other) { return getAccountId().get() == other.getAccountId().get(); } + + protected String getCommitterName(Account ua, String email) { + String name = ua.fullName(); + if (name == null || name.isEmpty()) { + final int at = email.indexOf('@'); + if (0 < at) { + name = email.substring(0, at); + } else { + name = anonymousCowardName; + } + } + return name; + } } diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java index 540e4384b0..f46196fa1a 100644 --- a/java/com/google/gerrit/server/change/RebaseChangeOp.java +++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java @@ -58,6 +58,7 @@ import com.google.inject.assistedinject.AssistedInject; import java.io.IOException; import java.util.List; import java.util.Map; +import java.util.Optional; import org.eclipse.jgit.diff.Sequence; import org.eclipse.jgit.dircache.DirCache; import org.eclipse.jgit.lib.CommitBuilder; @@ -504,7 +505,11 @@ public class RebaseChangeOp implements BatchUpdateOp { if (committerIdent != null) { cb.setCommitter(committerIdent); } else { - cb.setCommitter(ctx.newCommitterIdent()); + PersonIdent committerIdent = + Optional.ofNullable(original.getCommitterIdent()) + .map(ident -> ctx.newCommitterIdent(ident.getEmailAddress(), ctx.getIdentifiedUser())) + .orElseGet(ctx::newCommitterIdent); + cb.setCommitter(committerIdent); } if (matchAuthorToCommitterDate) { cb.setAuthor( diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java index 7a50bdde50..17d621205b 100644 --- a/java/com/google/gerrit/server/config/GerritGlobalModule.java +++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java @@ -177,9 +177,7 @@ import com.google.gerrit.server.notedb.ChangeDraftNotesUpdate; import com.google.gerrit.server.notedb.DeleteZombieCommentsRefs; import com.google.gerrit.server.notedb.NoteDbModule; import com.google.gerrit.server.notedb.StoreSubmitRequirementsOp; -import com.google.gerrit.server.patch.DiffFileSizeValidator; import com.google.gerrit.server.patch.DiffOperationsImpl; -import com.google.gerrit.server.patch.DiffValidator; import com.google.gerrit.server.patch.PatchListCacheImpl; import com.google.gerrit.server.patch.PatchScriptFactory; import com.google.gerrit.server.patch.PatchScriptFactoryForAutoFix; @@ -410,8 +408,6 @@ public class GerritGlobalModule extends FactoryModule { .to(SubmitRequirementConfigValidator.class); DynamicSet.bind(binder(), CommitValidationListener.class).to(PrologRulesWarningValidator.class); DynamicSet.setOf(binder(), CommentValidator.class); - DynamicSet.setOf(binder(), DiffValidator.class); - DynamicSet.bind(binder(), DiffValidator.class).to(DiffFileSizeValidator.class); DynamicSet.setOf(binder(), ChangeMessageModifier.class); DynamicSet.setOf(binder(), RefOperationValidationListener.class); DynamicSet.setOf(binder(), OnSubmitValidationListener.class); diff --git a/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/java/com/google/gerrit/server/edit/ChangeEditModifier.java index 4c15a7e1e3..68569f0be4 100644 --- a/java/com/google/gerrit/server/edit/ChangeEditModifier.java +++ b/java/com/google/gerrit/server/edit/ChangeEditModifier.java @@ -535,7 +535,7 @@ public class ChangeEditModifier { builder.setTreeId(tree); builder.setParentIds(basePatchsetCommit.getParents()); builder.setAuthor(basePatchsetCommit.getAuthorIdent()); - builder.setCommitter(getCommitterIdent(timestamp)); + builder.setCommitter(getCommitterIdent(basePatchsetCommit, timestamp)); builder.setMessage(commitMessage); ObjectId newCommitId = objectInserter.insert(builder); objectInserter.flush(); @@ -543,9 +543,14 @@ public class ChangeEditModifier { } } - private PersonIdent getCommitterIdent(Instant commitTimestamp) { + private PersonIdent getCommitterIdent(RevCommit basePatchsetCommit, Instant commitTimestamp) { IdentifiedUser user = currentUser.get().asIdentifiedUser(); - return user.newCommitterIdent(commitTimestamp, zoneId); + return Optional.ofNullable(basePatchsetCommit.getCommitterIdent()) + .map( + ident -> + user.newCommitterIdent(ident.getEmailAddress(), commitTimestamp, zoneId) + .orElseGet(() -> user.newCommitterIdent(commitTimestamp, zoneId))) + .orElseGet(() -> user.newCommitterIdent(commitTimestamp, zoneId)); } /** diff --git a/java/com/google/gerrit/server/git/LargeObjectException.java b/java/com/google/gerrit/server/git/LargeObjectException.java index 145b631645..04db42cd85 100644 --- a/java/com/google/gerrit/server/git/LargeObjectException.java +++ b/java/com/google/gerrit/server/git/LargeObjectException.java @@ -25,10 +25,6 @@ public class LargeObjectException extends Exception { private static final long serialVersionUID = 1L; - public LargeObjectException(String message) { - super(message); - } - public LargeObjectException(String message, org.eclipse.jgit.errors.LargeObjectException cause) { super(message, cause); } diff --git a/java/com/google/gerrit/server/git/TagSet.java b/java/com/google/gerrit/server/git/TagSet.java index a528c8fd60..bffc479c4f 100644 --- a/java/com/google/gerrit/server/git/TagSet.java +++ b/java/com/google/gerrit/server/git/TagSet.java @@ -14,6 +14,8 @@ package com.google.gerrit.server.git; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableSet; @@ -41,6 +43,7 @@ import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefDatabase; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevFlag; import org.eclipse.jgit.revwalk.RevSort; import org.eclipse.jgit.revwalk.RevWalk; import org.roaringbitmap.RoaringBitmap; @@ -209,6 +212,7 @@ class TagSet { try (TagWalk rw = new TagWalk(git)) { rw.setRetainBody(false); + RevFlag isTag = rw.newFlag("tag"); for (Ref ref : git.getRefDatabase() .getRefsByPrefixWithExclusions(RefDatabase.ALL, SKIPPABLE_REF_PREFIXES)) { @@ -218,9 +222,9 @@ class TagSet { } else if (isTag(ref)) { // For a tag, remember where it points to. try { - addTag(rw, git.getRefDatabase().peel(ref)); + addTag(rw, git.getRefDatabase().peel(ref), isTag); } catch (IOException e) { - addTag(rw, ref); + addTag(rw, ref, isTag); } } else { @@ -229,17 +233,10 @@ class TagSet { } } - // Traverse the complete history. Copy any flags from a commit to - // all of its ancestors. This automatically updates any Tag object - // as the TagCommit and the stored Tag object share the same - // underlying bit set. + // Traverse the complete history and propagate reachability to parents. TagCommit c; while ((c = (TagCommit) rw.next()) != null) { - RoaringBitmap mine = c.refFlags; - int pCnt = c.getParentCount(); - for (int pIdx = 0; pIdx < pCnt; pIdx++) { - ((TagCommit) c.getParent(pIdx)).refFlags.or(mine); - } + c.propagateReachabilityToParents(isTag); } } catch (IOException e) { logger.atWarning().withCause(e).log("Error building tags for repository %s", projectName); @@ -356,9 +353,7 @@ class TagSet { refs.putAll(old.refs); for (Tag srcTag : old.tags) { - RoaringBitmap mine = new RoaringBitmap(); - mine.or(srcTag.refFlags); - tags.add(new Tag(srcTag, mine)); + tags.add(new Tag(srcTag)); } for (TagMatcher.LostRef lost : m.lostRefs) { @@ -369,7 +364,7 @@ class TagSet { } } - private void addTag(TagWalk rw, Ref ref) { + private void addTag(TagWalk rw, Ref ref, RevFlag isTag) { ObjectId id = ref.getPeeledObjectId(); if (id == null) { id = ref.getObjectId(); @@ -378,7 +373,12 @@ class TagSet { if (!tags.contains(id)) { RoaringBitmap flags; try { - flags = ((TagCommit) rw.parseCommit(id)).refFlags; + TagCommit commit = ((TagCommit) rw.parseCommit(id)); + commit.add(isTag); + if (commit.refFlags == null) { + commit.refFlags = new RoaringBitmap(); + } + flags = commit.refFlags; } catch (IncorrectObjectTypeException notCommit) { flags = new RoaringBitmap(); } catch (IOException e) { @@ -395,6 +395,9 @@ class TagSet { rw.markStart(commit); int flag = refs.size(); + if (commit.refFlags == null) { + commit.refFlags = new RoaringBitmap(); + } commit.refFlags.add(flag); refs.put(ref.getName(), new CachedRef(ref, flag)); } catch (IncorrectObjectTypeException notCommit) { @@ -432,8 +435,13 @@ class TagSet { // RoaringBitmap in TagCommit.refFlags. @VisibleForTesting final RoaringBitmap refFlags; + Tag(Tag src) { + this(src, src.refFlags.clone()); + } + Tag(AnyObjectId id, RoaringBitmap flags) { super(id); + checkNotNull(flags); this.refFlags = flags; } @@ -485,13 +493,50 @@ class TagSet { } // TODO(hanwen): this would be better named as CommitWithReachability, as it also holds non-tags. + // However, non-tags will have a null refFlags field. private static final class TagCommit extends RevCommit { /** CachedRef.flag => isVisible, indicating if this commit is reachable from the ref. */ - final RoaringBitmap refFlags; + RoaringBitmap refFlags; TagCommit(AnyObjectId id) { super(id); - refFlags = new RoaringBitmap(); + } + + /** + * Copy any flags from this commit to all of its ancestors. + * + * <p>Do not maintain a reference to the flags on non-tag commits after copying their flags to + * their ancestors. The flag copying automatically updates any Tag object as the TagCommit and + * the stored Tag object share the same underlying RoaringBitmap. + * + * @param isTag {@code RevFlag} indicating if this TagCommit is a tag + */ + void propagateReachabilityToParents(RevFlag isTag) { + RoaringBitmap mine = refFlags; + if (mine != null) { + boolean canMoveBitmap = false; + if (!has(isTag)) { + refFlags = null; + canMoveBitmap = true; + } + int pCnt = getParentCount(); + for (int pIdx = 0; pIdx < pCnt; pIdx++) { + TagCommit commit = (TagCommit) getParent(pIdx); + RoaringBitmap parentFlags = commit.refFlags; + if (parentFlags == null) { + if (canMoveBitmap) { + // This commit is not itself a Tag, so in order to reduce cloning overhead, migrate + // its refFlags object to its first parent with null refFlags + commit.refFlags = mine; + canMoveBitmap = false; + } else { + commit.refFlags = mine.clone(); + } + } else { + parentFlags.or(mine); + } + } + } } } } diff --git a/java/com/google/gerrit/server/patch/DiffFileSizeValidator.java b/java/com/google/gerrit/server/patch/DiffFileSizeValidator.java deleted file mode 100644 index 14a0f7bd50..0000000000 --- a/java/com/google/gerrit/server/patch/DiffFileSizeValidator.java +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright (C) 2023 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.server.patch; - -import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_CORE_SECTION; -import static org.eclipse.jgit.lib.ConfigConstants.CONFIG_KEY_BIGFILE_THRESHOLD; -import static org.eclipse.jgit.storage.pack.PackConfig.DEFAULT_BIG_FILE_THRESHOLD; - -import com.google.gerrit.server.config.GerritServerConfig; -import com.google.gerrit.server.git.LargeObjectException; -import com.google.gerrit.server.patch.filediff.FileDiffOutput; -import com.google.inject.Inject; -import org.eclipse.jgit.lib.Config; - -public class DiffFileSizeValidator implements DiffValidator { - static final int DEFAULT_MAX_FILE_SIZE = DEFAULT_BIG_FILE_THRESHOLD; - private static final String ERROR_MESSAGE = - "File size for file %s exceeded the max file size threshold. Threshold = %d bytes, Actual size = %d bytes"; - - final int maxFileSize; - - @Inject - public DiffFileSizeValidator(@GerritServerConfig Config cfg) { - this.maxFileSize = - cfg.getInt(CONFIG_CORE_SECTION, CONFIG_KEY_BIGFILE_THRESHOLD, DEFAULT_MAX_FILE_SIZE); - } - - @Override - public void validate(FileDiffOutput fileDiff) throws LargeObjectException { - if (fileDiff.size() > maxFileSize) { - throw new LargeObjectException( - String.format(ERROR_MESSAGE, fileDiff.getDefaultPath(), maxFileSize, fileDiff.size())); - } - if (fileDiff.sizeDelta() > maxFileSize) { - throw new LargeObjectException( - String.format( - ERROR_MESSAGE, fileDiff.getDefaultPath(), maxFileSize, fileDiff.sizeDelta())); - } - } -} diff --git a/java/com/google/gerrit/server/patch/DiffValidator.java b/java/com/google/gerrit/server/patch/DiffValidator.java deleted file mode 100644 index aee3c8bc04..0000000000 --- a/java/com/google/gerrit/server/patch/DiffValidator.java +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (C) 2023 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.server.patch; - -import com.google.gerrit.extensions.annotations.ExtensionPoint; -import com.google.gerrit.server.git.LargeObjectException; -import com.google.gerrit.server.patch.filediff.FileDiffOutput; - -/** Interface to validate diff outputs. */ -@ExtensionPoint -public interface DiffValidator { - void validate(FileDiffOutput fileDiffOutput) - throws LargeObjectException, DiffNotAvailableException; -} diff --git a/java/com/google/gerrit/server/patch/DiffValidators.java b/java/com/google/gerrit/server/patch/DiffValidators.java deleted file mode 100644 index 964353dc57..0000000000 --- a/java/com/google/gerrit/server/patch/DiffValidators.java +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright (C) 2023 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.server.patch; - -import com.google.gerrit.extensions.registration.DynamicSet; -import com.google.gerrit.server.git.LargeObjectException; -import com.google.gerrit.server.patch.filediff.FileDiffOutput; -import com.google.inject.Inject; - -/** Validates {@link FileDiffOutput}(s) after they are computed by the {@link DiffOperations}. */ -public class DiffValidators { - DynamicSet<DiffValidator> diffValidators; - - @Inject - public DiffValidators(DynamicSet<DiffValidator> diffValidators) { - this.diffValidators = diffValidators; - } - - public void validate(FileDiffOutput fileDiffOutput) - throws LargeObjectException, DiffNotAvailableException { - for (DiffValidator validator : diffValidators) { - validator.validate(fileDiffOutput); - } - } -} diff --git a/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/java/com/google/gerrit/server/patch/PatchScriptFactory.java index 5015c76807..3baa3b1535 100644 --- a/java/com/google/gerrit/server/patch/PatchScriptFactory.java +++ b/java/com/google/gerrit/server/patch/PatchScriptFactory.java @@ -94,7 +94,6 @@ public class PatchScriptFactory implements Callable<PatchScript> { private final PermissionBackend permissionBackend; private final ProjectCache projectCache; private final DiffOperations diffOperations; - private final DiffValidators diffValidators; private final Change.Id changeId; @@ -110,7 +109,6 @@ public class PatchScriptFactory implements Callable<PatchScript> { PermissionBackend permissionBackend, ProjectCache projectCache, DiffOperations diffOperations, - DiffValidators diffValidators, @Assisted ChangeNotes notes, @Assisted String fileName, @Assisted("patchSetA") @Nullable PatchSet.Id patchSetA, @@ -126,7 +124,6 @@ public class PatchScriptFactory implements Callable<PatchScript> { this.permissionBackend = permissionBackend; this.projectCache = projectCache; this.diffOperations = diffOperations; - this.diffValidators = diffValidators; this.fileName = fileName; this.psa = patchSetA; @@ -147,7 +144,6 @@ public class PatchScriptFactory implements Callable<PatchScript> { PermissionBackend permissionBackend, ProjectCache projectCache, DiffOperations diffOperations, - DiffValidators diffValidators, @Assisted ChangeNotes notes, @Assisted String fileName, @Assisted int parentNum, @@ -163,7 +159,6 @@ public class PatchScriptFactory implements Callable<PatchScript> { this.permissionBackend = permissionBackend; this.projectCache = projectCache; this.diffOperations = diffOperations; - this.diffValidators = diffValidators; this.fileName = fileName; this.psa = null; @@ -225,14 +220,13 @@ public class PatchScriptFactory implements Callable<PatchScript> { } private PatchScript getPatchScript(Repository git, ObjectId aId, ObjectId bId) - throws IOException, DiffNotAvailableException, LargeObjectException { + throws IOException, DiffNotAvailableException { FileDiffOutput fileDiffOutput = aId == null ? diffOperations.getModifiedFileAgainstParent( notes.getProjectName(), bId, parentNum, fileName, diffPrefs.ignoreWhitespace) : diffOperations.getModifiedFile( notes.getProjectName(), aId, bId, fileName, diffPrefs.ignoreWhitespace); - diffValidators.validate(fileDiffOutput); return newBuilder().toPatchScript(git, fileDiffOutput); } diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java b/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java index 9107dded38..9286f47939 100644 --- a/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java +++ b/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java @@ -64,10 +64,6 @@ public abstract class FileDiffOutput implements Serializable { */ public abstract Optional<String> newPath(); - public String getDefaultPath() { - return oldPath().isPresent() ? oldPath().get() : newPath().get(); - } - /** * The file mode of the old file at the old git tree diff identified by {@link #oldCommitId()} * ()}. diff --git a/java/com/google/gerrit/server/query/change/ChangePredicates.java b/java/com/google/gerrit/server/query/change/ChangePredicates.java index 6d4d74dadd..14e40bdf85 100644 --- a/java/com/google/gerrit/server/query/change/ChangePredicates.java +++ b/java/com/google/gerrit/server/query/change/ChangePredicates.java @@ -23,10 +23,12 @@ import com.google.gerrit.entities.PatchSet; import com.google.gerrit.entities.Project; import com.google.gerrit.git.ObjectIds; import com.google.gerrit.index.query.Predicate; +import com.google.gerrit.index.query.QueryParseException; import com.google.gerrit.server.DraftCommentsReader; import com.google.gerrit.server.StarredChangesUtil; import com.google.gerrit.server.change.HashtagsUtil; import com.google.gerrit.server.index.change.ChangeField; +import com.google.inject.ImplementedBy; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -61,12 +63,25 @@ public class ChangePredicates { return new ChangeIndexPredicate(ChangeField.COMMENTBY_SPEC, id.toString()); } + @ImplementedBy(IndexEditByPredicateProvider.class) + public interface EditByPredicateProvider { + + /** + * Returns a predicate that matches changes where the provided {@link + * com.google.gerrit.entities.Account.Id} has a pending change edit. + */ + Predicate<ChangeData> editBy(Account.Id id) throws QueryParseException; + } + /** - * Returns a predicate that matches changes where the provided {@link - * com.google.gerrit.entities.Account.Id} has a pending change edit. + * A default implementation of {@link EditByPredicateProvider}, based on th {@link + * ChangeField#EDITBY_SPEC} index field. */ - public static Predicate<ChangeData> editBy(Account.Id id) { - return new ChangeIndexPredicate(ChangeField.EDITBY_SPEC, id.toString()); + public static class IndexEditByPredicateProvider implements EditByPredicateProvider { + @Override + public Predicate<ChangeData> editBy(Account.Id id) { + return new ChangeIndexPredicate(ChangeField.EDITBY_SPEC, id.toString()); + } } /** diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index b3fa087f18..f8a4a9909e 100644 --- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -89,6 +89,7 @@ import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.plugincontext.PluginSetContext; import com.google.gerrit.server.project.ChildProjects; import com.google.gerrit.server.project.ProjectCache; +import com.google.gerrit.server.query.change.ChangePredicates.EditByPredicateProvider; import com.google.gerrit.server.query.change.PredicateArgs.ValOp; import com.google.gerrit.server.rules.SubmitRule; import com.google.gerrit.server.submit.SubmitDryRun; @@ -284,6 +285,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil private final Provider<CurrentUser> self; + private final EditByPredicateProvider editByPredicateProvider; + @Inject @VisibleForTesting public Arguments( @@ -318,7 +321,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil ExperimentFeatures experimentFeatures, HasOperandAliasConfig hasOperandAliasConfig, ChangeIsVisibleToPredicate.Factory changeIsVisbleToPredicateFactory, - PluginSetContext<SubmitRule> submitRules) { + PluginSetContext<SubmitRule> submitRules, + EditByPredicateProvider editByPredicateProvider) { this( queryProvider, rewriter, @@ -352,7 +356,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil experimentFeatures, hasOperandAliasConfig, changeIsVisbleToPredicateFactory, - submitRules); + submitRules, + editByPredicateProvider); } private Arguments( @@ -388,7 +393,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil ExperimentFeatures experimentFeatures, HasOperandAliasConfig hasOperandAliasConfig, ChangeIsVisibleToPredicate.Factory changeIsVisbleToPredicateFactory, - PluginSetContext<SubmitRule> submitRules) { + PluginSetContext<SubmitRule> submitRules, + EditByPredicateProvider editByPredicateProvider) { this.queryProvider = queryProvider; this.rewriter = rewriter; this.opFactories = opFactories; @@ -422,6 +428,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil this.experimentFeatures = experimentFeatures; this.hasOperandAliasConfig = hasOperandAliasConfig; this.submitRules = submitRules; + this.editByPredicateProvider = editByPredicateProvider; } public Arguments asUser(CurrentUser otherUser) { @@ -458,7 +465,8 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil experimentFeatures, hasOperandAliasConfig, changeIsVisbleToPredicateFactory, - submitRules); + submitRules, + editByPredicateProvider); } Arguments asUser(Account.Id otherId) { @@ -646,7 +654,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil } if ("edit".equalsIgnoreCase(value)) { - return ChangePredicates.editBy(self()); + return this.args.editByPredicateProvider.editBy(self()); } if ("attention".equalsIgnoreCase(value)) { @@ -1847,7 +1855,7 @@ public class ChangeQueryBuilder extends QueryBuilder<ChangeData, ChangeQueryBuil } /** Returns {@link com.google.gerrit.entities.Account.Id} of the identified calling user. */ - public Account.Id self() throws QueryParseException { + private Account.Id self() throws QueryParseException { return args.getIdentifiedUser().getAccountId(); } diff --git a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java index 3e471fbcc0..5993c76914 100644 --- a/java/com/google/gerrit/server/query/change/InternalChangeQuery.java +++ b/java/com/google/gerrit/server/query/change/InternalChangeQuery.java @@ -18,6 +18,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.gerrit.index.query.Predicate.and; import static com.google.gerrit.index.query.Predicate.not; import static com.google.gerrit.index.query.Predicate.or; +import static com.google.gerrit.server.query.change.ChangePredicates.EditByPredicateProvider; import static com.google.gerrit.server.query.change.ChangeStatusPredicate.open; import com.google.common.annotations.VisibleForTesting; @@ -35,6 +36,7 @@ import com.google.gerrit.entities.RefNames; import com.google.gerrit.index.IndexConfig; import com.google.gerrit.index.query.InternalQuery; import com.google.gerrit.index.query.Predicate; +import com.google.gerrit.index.query.QueryParseException; import com.google.gerrit.server.index.change.ChangeField; import com.google.gerrit.server.index.change.ChangeIndexCollection; import com.google.gerrit.server.notedb.ChangeNotes; @@ -75,8 +77,8 @@ public class InternalChangeQuery extends InternalQuery<ChangeData, InternalChang return ChangeStatusPredicate.forStatus(status); } - private static Predicate<ChangeData> editBy(Account.Id accountId) { - return ChangePredicates.editBy(accountId); + private Predicate<ChangeData> editBy(Account.Id accountId) throws QueryParseException { + return editByPredicateProvider.editBy(accountId); } private static Predicate<ChangeData> commit(String id) { @@ -85,6 +87,7 @@ public class InternalChangeQuery extends InternalQuery<ChangeData, InternalChang private final ChangeData.Factory changeDataFactory; private final ChangeNotes.Factory notesFactory; + private final EditByPredicateProvider editByPredicateProvider; @Inject InternalChangeQuery( @@ -92,10 +95,12 @@ public class InternalChangeQuery extends InternalQuery<ChangeData, InternalChang ChangeIndexCollection indexes, IndexConfig indexConfig, ChangeData.Factory changeDataFactory, - ChangeNotes.Factory notesFactory) { + ChangeNotes.Factory notesFactory, + EditByPredicateProvider editByPredicateProvider) { super(queryProcessor, indexes, indexConfig); this.changeDataFactory = changeDataFactory; this.notesFactory = notesFactory; + this.editByPredicateProvider = editByPredicateProvider; } public List<ChangeData> byKey(Change.Key key) { @@ -221,7 +226,7 @@ public class InternalChangeQuery extends InternalQuery<ChangeData, InternalChang return query(and(ChangePredicates.exactTopic(topic), open())); } - public List<ChangeData> byOpenEditByUser(Account.Id accountId) { + public List<ChangeData> byOpenEditByUser(Account.Id accountId) throws QueryParseException { return query(editBy(accountId)); } diff --git a/java/com/google/gerrit/server/restapi/account/DeleteAccount.java b/java/com/google/gerrit/server/restapi/account/DeleteAccount.java index 9b4c0a6657..0831ff99b3 100644 --- a/java/com/google/gerrit/server/restapi/account/DeleteAccount.java +++ b/java/com/google/gerrit/server/restapi/account/DeleteAccount.java @@ -26,6 +26,7 @@ import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.gpg.PublicKeyStoreUtil; +import com.google.gerrit.index.query.QueryParseException; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.IdentifiedUser; @@ -166,7 +167,7 @@ public class DeleteAccount implements RestModifyView<AccountResource, Input> { } } - private void deleteChangeEdits(Account.Id accountId) throws IOException { + private void deleteChangeEdits(Account.Id accountId) throws IOException, QueryParseException { // Note: in case of a stale index, the results of this query might be incomplete. List<ChangeData> changesWithEdits = queryProvider.get().byOpenEditByUser(accountId); diff --git a/java/com/google/gerrit/server/restapi/change/ApplyPatch.java b/java/com/google/gerrit/server/restapi/change/ApplyPatch.java index 75eaacf7eb..6adde99309 100644 --- a/java/com/google/gerrit/server/restapi/change/ApplyPatch.java +++ b/java/com/google/gerrit/server/restapi/change/ApplyPatch.java @@ -65,6 +65,7 @@ import java.io.IOException; import java.time.Instant; import java.time.ZoneId; import java.util.List; +import java.util.Optional; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.lib.ObjectId; @@ -188,7 +189,14 @@ public class ApplyPatch implements RestModifyView<ChangeResource, ApplyPatchPatc ObjectId treeId = applyResult.getTreeId(); Instant now = TimeUtil.now(); - PersonIdent committerIdent = user.get().newCommitterIdent(now, serverZoneId); + PersonIdent committerIdent = + Optional.ofNullable(latestPatchset.getCommitterIdent()) + .map( + ident -> + user.get() + .newCommitterIdent(ident.getEmailAddress(), now, serverZoneId) + .orElseGet(() -> user.get().newCommitterIdent(now, serverZoneId))) + .orElseGet(() -> user.get().newCommitterIdent(now, serverZoneId)); PersonIdent authorIdent = input.author == null ? committerIdent diff --git a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java index 1bfb6bd623..677279e663 100644 --- a/java/com/google/gerrit/server/restapi/change/CherryPickChange.java +++ b/java/com/google/gerrit/server/restapi/change/CherryPickChange.java @@ -74,6 +74,7 @@ import java.time.Instant; import java.time.ZoneId; import java.util.HashSet; import java.util.List; +import java.util.Optional; import java.util.Set; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.ObjectId; @@ -174,7 +175,8 @@ public class CherryPickChange { null, null, null, - null); + null, + Optional.empty()); } /** @@ -205,7 +207,17 @@ public class CherryPickChange { throws IOException, InvalidChangeOperationException, UpdateException, RestApiException, ConfigInvalidException, NoSuchProjectException { return cherryPick( - sourceChange, project, sourceCommit, input, dest, TimeUtil.now(), null, null, null, null); + sourceChange, + project, + sourceCommit, + input, + dest, + TimeUtil.now(), + null, + null, + null, + null, + Optional.empty()); } /** @@ -227,6 +239,9 @@ public class CherryPickChange { * @param idForNewChange The ID that the new change of the cherry pick will have. If provided and * the cherry-pick doesn't result in creating a new change, then * InvalidChangeOperationException is thrown. + * @param verifiedBaseCommit - base commit for the cherry-pick, which is guaranteed to be + * associated with exactly one change and belong to a {@code dest} branch. This is currently + * only used when this base commit was created in the same API call. * @return Result object that describes the cherry pick. * @throws IOException Unable to open repository or read from the database. * @throws InvalidChangeOperationException Parent or branch don't exist, or two changes with same @@ -247,7 +262,8 @@ public class CherryPickChange { @Nullable Change.Id revertedChange, @Nullable ObjectId changeIdForNewChange, @Nullable Change.Id idForNewChange, - @Nullable Boolean workInProgress) + @Nullable Boolean workInProgress, + Optional<RevCommit> verifiedBaseCommit) throws IOException, InvalidChangeOperationException, UpdateException, RestApiException, ConfigInvalidException, NoSuchProjectException { IdentifiedUser identifiedUser = user.get(); @@ -265,8 +281,9 @@ public class CherryPickChange { } RevCommit baseCommit = - CommitUtil.getBaseCommit( - project.get(), queryProvider.get(), revWalk, destRef, input.base); + verifiedBaseCommit.orElse( + CommitUtil.getBaseCommit( + project.get(), queryProvider.get(), revWalk, destRef, input.base)); CodeReviewCommit commitToCherryPick = revWalk.parseCommit(sourceCommit); @@ -306,8 +323,15 @@ public class CherryPickChange { CodeReviewCommit cherryPickCommit; ProjectState projectState = projectCache.get(dest.project()).orElseThrow(noSuchProject(dest.project())); - PersonIdent committerIdent = identifiedUser.newCommitterIdent(timestamp, serverZoneId); - + PersonIdent committerIdent = + Optional.ofNullable(commitToCherryPick.getCommitterIdent()) + .map( + ident -> + identifiedUser + .newCommitterIdent(ident.getEmailAddress(), timestamp, serverZoneId) + .orElseGet( + () -> identifiedUser.newCommitterIdent(timestamp, serverZoneId))) + .orElseGet(() -> identifiedUser.newCommitterIdent(timestamp, serverZoneId)); try { MergeUtil mergeUtil; if (input.allowConflicts) { diff --git a/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java b/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java index ff21916248..989dc7a943 100644 --- a/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java +++ b/java/com/google/gerrit/server/restapi/change/CreateMergePatchSet.java @@ -74,6 +74,7 @@ import java.io.IOException; import java.time.Instant; import java.time.ZoneId; import java.util.List; +import java.util.Optional; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectReader; @@ -182,11 +183,18 @@ public class CreateMergePatchSet implements RestModifyView<ChangeResource, Merge Instant now = TimeUtil.now(); IdentifiedUser me = user.get().asIdentifiedUser(); - PersonIdent committer = me.newCommitterIdent(now, serverZoneId); PersonIdent author = in.author == null - ? committer + ? me.newCommitterIdent(now, serverZoneId) : new PersonIdent(in.author.name, in.author.email, now, serverZoneId); + RevCommit commit = rw.parseCommit(ps.commitId()); + PersonIdent committer = + Optional.ofNullable(commit.getCommitterIdent()) + .map( + ident -> + me.newCommitterIdent(ident.getEmailAddress(), now, serverZoneId) + .orElseGet(() -> me.newCommitterIdent(now, serverZoneId))) + .orElseGet(() -> me.newCommitterIdent(now, serverZoneId)); CodeReviewCommit newCommit = createMergeCommit( in, diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java index 4eca1f3b4b..3717e02f1d 100644 --- a/java/com/google/gerrit/server/restapi/change/PutMessage.java +++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java @@ -30,6 +30,7 @@ import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.GerritPersonIdent; +import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.change.NotifyResolver; @@ -51,6 +52,7 @@ import com.google.inject.Singleton; import java.io.IOException; import java.time.Instant; import java.time.ZoneId; +import java.util.Optional; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.ObjectId; @@ -175,8 +177,15 @@ public class PutMessage implements RestModifyView<ChangeResource, CommitMessageI builder.setTreeId(basePatchSetCommit.getTree()); builder.setParentIds(basePatchSetCommit.getParents()); builder.setAuthor(basePatchSetCommit.getAuthorIdent()); - builder.setCommitter( - userProvider.get().asIdentifiedUser().newCommitterIdent(timestamp, zoneId)); + IdentifiedUser user = userProvider.get().asIdentifiedUser(); + PersonIdent committer = + Optional.ofNullable(basePatchSetCommit.getCommitterIdent()) + .map( + ident -> + user.newCommitterIdent(ident.getEmailAddress(), timestamp, zoneId) + .orElseGet(() -> user.newCommitterIdent(timestamp, zoneId))) + .orElseGet(() -> user.newCommitterIdent(timestamp, zoneId)); + builder.setCommitter(committer); builder.setMessage(commitMessage); ObjectId newCommitId = objectInserter.insert(builder); objectInserter.flush(); diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java index 691fc7588a..3851e826da 100644 --- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java +++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java @@ -86,6 +86,7 @@ import java.util.Comparator; import java.util.Iterator; import java.util.List; import java.util.Locale; +import java.util.Optional; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -310,6 +311,13 @@ public class RevertSubmission cherryPickInput.message = revertInput.message; ObjectId generatedChangeId = CommitMessageUtil.generateChangeId(); Change.Id cherryPickRevertChangeId = Change.id(seq.nextChangeId()); + RevCommit baseCommit = null; + if (cherryPickInput.base != null) { + try (Repository git = repoManager.openRepository(changeNotes.getProjectName()); + RevWalk revWalk = new RevWalk(git.newObjectReader())) { + baseCommit = revWalk.parseCommit(ObjectId.fromString(cherryPickInput.base)); + } + } try (RefUpdateContext ctx = RefUpdateContext.open(CHANGE_MODIFICATION)) { try (BatchUpdate bu = updateFactory.create(project, user.get(), TimeUtil.now())) { bu.setNotify( @@ -323,7 +331,8 @@ public class RevertSubmission generatedChangeId, cherryPickRevertChangeId, timestamp, - revertInput.workInProgress)); + revertInput.workInProgress, + baseCommit)); if (!revertInput.workInProgress) { commitUtil.addChangeRevertedNotificationOps( bu, changeNotes.getChangeId(), cherryPickRevertChangeId, generatedChangeId.name()); @@ -548,18 +557,21 @@ public class RevertSubmission private final Change.Id cherryPickRevertChangeId; private final Instant timestamp; private final boolean workInProgress; + private final RevCommit baseCommit; CreateCherryPickOp( ObjectId revCommitId, ObjectId computedChangeId, Change.Id cherryPickRevertChangeId, Instant timestamp, - Boolean workInProgress) { + Boolean workInProgress, + RevCommit baseCommit) { this.revCommitId = revCommitId; this.computedChangeId = computedChangeId; this.cherryPickRevertChangeId = cherryPickRevertChangeId; this.timestamp = timestamp; this.workInProgress = workInProgress; + this.baseCommit = baseCommit; } @Override @@ -577,7 +589,8 @@ public class RevertSubmission change.getId(), computedChangeId, cherryPickRevertChangeId, - workInProgress); + workInProgress, + Optional.ofNullable(baseCommit)); // save the commit as base for next cherryPick of that branch cherryPickInput.base = changeNotesFactory diff --git a/java/com/google/gerrit/server/submit/CherryPick.java b/java/com/google/gerrit/server/submit/CherryPick.java index 0471b67fdd..7fe5e69631 100644 --- a/java/com/google/gerrit/server/submit/CherryPick.java +++ b/java/com/google/gerrit/server/submit/CherryPick.java @@ -34,6 +34,7 @@ import com.google.gerrit.server.update.RepoContext; import java.io.IOException; import java.util.Collection; import java.util.List; +import java.util.Optional; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.revwalk.RevCommit; @@ -102,8 +103,10 @@ public class CherryPick extends SubmitStrategy { RevCommit mergeTip = args.mergeTip.getCurrentTip(); args.rw.parseBody(mergeTip); String cherryPickCmtMsg = args.mergeUtil.createCommitMessageOnSubmit(toMerge, mergeTip); - - PersonIdent committer = ctx.newCommitterIdent(args.caller); + PersonIdent committer = + Optional.ofNullable(toMerge.getCommitterIdent()) + .map(ident -> ctx.newCommitterIdent(ident.getEmailAddress(), args.caller)) + .orElseGet(() -> ctx.newCommitterIdent(args.caller)); try { newCommit = args.mergeUtil.createCherryPickFromCommit( diff --git a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java index 5f58a744fe..87de810f2f 100644 --- a/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java +++ b/java/com/google/gerrit/server/submit/RebaseSubmitStrategy.java @@ -41,6 +41,7 @@ import com.google.gerrit.server.update.RepoContext; import java.io.IOException; import java.util.Collection; import java.util.List; +import java.util.Optional; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; @@ -167,7 +168,10 @@ public class RebaseSubmitStrategy extends SubmitStrategy { RevCommit mergeTip = args.mergeTip.getCurrentTip(); args.rw.parseBody(mergeTip); String cherryPickCmtMsg = args.mergeUtil.createCommitMessageOnSubmit(toMerge, mergeTip); - PersonIdent committer = ctx.newCommitterIdent(args.caller); + PersonIdent committer = + Optional.ofNullable(toMerge.getCommitterIdent()) + .map(ident -> ctx.newCommitterIdent(ident.getEmailAddress(), args.caller)) + .orElseGet(() -> ctx.newCommitterIdent(args.caller)); try { newCommit = args.mergeUtil.createCherryPickFromCommit( diff --git a/java/com/google/gerrit/server/update/Context.java b/java/com/google/gerrit/server/update/Context.java index aa41d90dc9..4e5d73fe60 100644 --- a/java/com/google/gerrit/server/update/Context.java +++ b/java/com/google/gerrit/server/update/Context.java @@ -164,4 +164,16 @@ public interface Context { default PersonIdent newCommitterIdent(IdentifiedUser user) { return user.newCommitterIdent(getWhen(), getZoneId()); } + + /** + * Creates a committer {@link PersonIdent} for the given user. The identity will be created with + * the given email if the user is allowed to use it, otherwise fallback to preferred email. + * + * @param user user for which a committer {@link PersonIdent} should be created + * @param email committer email of the source commit + * @return the created committer {@link PersonIdent} + */ + default PersonIdent newCommitterIdent(String email, IdentifiedUser user) { + return user.newCommitterIdent(email, getWhen(), getZoneId()).orElseGet(this::newCommitterIdent); + } } diff --git a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java index b1cc8668ac..d8c2aae573 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java @@ -113,7 +113,7 @@ public class ApplyPatchIT extends AbstractDaemonTest { in.responseFormatOptions = ImmutableList.of(CURRENT_REVISION, CURRENT_COMMIT); ChangeInfo result = gApi.changes().id(change.get()).applyPatch(in); - assertThat(result.getCurrentRevision().commit.committer.email).isEqualTo(emailTwo); + assertThat(result.getCurrentRevision().commit.committer.email).isEqualTo(emailOne); } private static final String MODIFIED_FILE_NAME = "modified_file.txt"; diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java index 3b0d240de6..3a835b78b4 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -3920,7 +3920,7 @@ public class ChangeIT extends AbstractDaemonTest { gApi.changes().id(change.get()).setMessage(msg); assertThat(gApi.changes().id(change.get()).get().getCurrentRevision().commit.committer.email) - .isEqualTo(emailTwo); + .isEqualTo(emailOne); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/change/CreateMergePatchSetIT.java b/javatests/com/google/gerrit/acceptance/api/change/CreateMergePatchSetIT.java index 771935a6b7..502f286688 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/CreateMergePatchSetIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/CreateMergePatchSetIT.java @@ -188,7 +188,7 @@ public class CreateMergePatchSetIT extends AbstractDaemonTest { gApi.changes().id(change.get()).createMergePatchSet(in); assertThat(gApi.changes().id(change.get()).get().getCurrentRevision().commit.committer.email) - .isEqualTo(emailTwo); + .isEqualTo(emailOne); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java index 7fe79e4ac8..7d1ddfc4f1 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseChainOnBehalfOfUploaderIT.java @@ -257,7 +257,7 @@ public class RebaseChainOnBehalfOfUploaderIT extends AbstractDaemonTest { .commit .committer .email) - .isEqualTo(uploaderEmailTwo); + .isEqualTo(uploaderEmailOne); assertThat( gApi.changes() .id(changeToBeRebased2.get()) @@ -266,7 +266,7 @@ public class RebaseChainOnBehalfOfUploaderIT extends AbstractDaemonTest { .commit .committer .email) - .isEqualTo(uploaderEmailTwo); + .isEqualTo(uploaderEmailOne); assertThat( gApi.changes() .id(changeToBeRebased3.get()) @@ -275,7 +275,7 @@ public class RebaseChainOnBehalfOfUploaderIT extends AbstractDaemonTest { .commit .committer .email) - .isEqualTo(uploaderEmailTwo); + .isEqualTo(uploaderEmailOne); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java index 152d9dd006..d9b079ae9d 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseIT.java @@ -237,7 +237,7 @@ public class RebaseIT { // Rebase the second change gApi.changes().id(c2.get()).rebase(); assertThat(gApi.changes().id(c2.get()).get().getCurrentRevision().commit.committer.email) - .isEqualTo(emailTwo); + .isEqualTo(emailOne); } @Test @@ -461,6 +461,12 @@ public class RebaseIT { String changeId = r2.getChangeId(); requestScopeOperations.setApiUser(user.id()); rebaseCall.call(changeId); + + // Verify that the committer has been updated + GitPerson committer = + gApi.changes().id(r2.getChangeId()).get().getCurrentRevision().commit.committer; + assertThat(committer.name).isEqualTo(user.fullName()); + assertThat(committer.email).isEqualTo(user.email()); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java b/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java index 345601285f..968c1f7fb2 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/RebaseOnBehalfOfUploaderIT.java @@ -282,7 +282,7 @@ public class RebaseOnBehalfOfUploaderIT extends AbstractDaemonTest { .commit .committer .email) - .isEqualTo(uploaderEmailTwo); + .isEqualTo(uploaderEmailOne); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java b/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java index e4dc0e8305..84a4a40612 100644 --- a/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java +++ b/javatests/com/google/gerrit/acceptance/api/project/CommitIT.java @@ -228,7 +228,7 @@ public class CommitIT extends AbstractDaemonTest { input.message = "cherry-pick to foo branch"; ChangeInfo cherryPickResult = gApi.projects().name(project.get()).commit(commit).cherryPick(input).get(); - assertThat(cherryPickResult.getCurrentRevision().commit.committer.email).isEqualTo(emailTwo); + assertThat(cherryPickResult.getCurrentRevision().commit.committer.email).isEqualTo(emailOne); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java b/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java index fa4f568f08..b9ef0bf48e 100644 --- a/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java +++ b/javatests/com/google/gerrit/acceptance/api/revision/ApplyProvidedFixIT.java @@ -128,7 +128,7 @@ public class ApplyProvidedFixIT extends AbstractDaemonTest { gApi.changes().id(change.get()).current().applyFix(applyProvidedFixInput); EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow(); - assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo); + assertThat(editInfo.commit.committer.email).isEqualTo(emailOne); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java index cdcd04443c..b3db99f518 100644 --- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java @@ -385,7 +385,7 @@ public class RevisionIT extends AbstractDaemonTest { input.message = "cherry-pick to foo branch"; ChangeInfo changeInfo = gApi.changes().id(changeId.get()).revision(commit).cherryPick(input).get(); - assertThat(changeInfo.getCurrentRevision().commit.committer.email).isEqualTo(emailTwo); + assertThat(changeInfo.getCurrentRevision().commit.committer.email).isEqualTo(emailOne); } @Test diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java index 1ab74fbef6..b31d35ca78 100644 --- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java +++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java @@ -796,7 +796,7 @@ public class RobotCommentsIT extends AbstractDaemonTest { gApi.changes().id(change.get()).current().applyFix(fixId); EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow(); - assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo); + assertThat(editInfo.commit.committer.email).isEqualTo(emailOne); } @Test diff --git a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java index 6bff0be956..9c691ae088 100644 --- a/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java +++ b/javatests/com/google/gerrit/acceptance/edit/ChangeEditIT.java @@ -291,7 +291,7 @@ public class ChangeEditIT extends AbstractDaemonTest { gApi.changes().id(change.get()).edit().rebase(); EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow(); - assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo); + assertThat(editInfo.commit.committer.email).isEqualTo(emailOne); } @Test @@ -365,7 +365,7 @@ public class ChangeEditIT extends AbstractDaemonTest { gApi.changes().id(change.get()).edit().modifyFile(FILE_NAME, RawInputUtil.create(CONTENT_NEW)); EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow(); - assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo); + assertThat(editInfo.commit.committer.email).isEqualTo(emailOne); } @Test @@ -530,7 +530,7 @@ public class ChangeEditIT extends AbstractDaemonTest { gApi.changes().id(change.get()).edit().modifyCommitMessage(msg); EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow(); - assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo); + assertThat(editInfo.commit.committer.email).isEqualTo(emailOne); } @Test @@ -703,7 +703,7 @@ public class ChangeEditIT extends AbstractDaemonTest { gApi.changes().id(change.get()).edit().deleteFile(FILE_NAME); EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow(); - assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo); + assertThat(editInfo.commit.committer.email).isEqualTo(emailOne); } @Test @@ -738,7 +738,7 @@ public class ChangeEditIT extends AbstractDaemonTest { gApi.changes().id(change.get()).edit().renameFile(FILE_NAME, FILE_NAME3); EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow(); - assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo); + assertThat(editInfo.commit.committer.email).isEqualTo(emailOne); } @Test @@ -800,7 +800,7 @@ public class ChangeEditIT extends AbstractDaemonTest { gApi.changes().id(change.get()).edit().restoreFile(FILE_NAME); EditInfo editInfo = gApi.changes().id(change.get()).edit().get().orElseThrow(); - assertThat(editInfo.commit.committer.email).isEqualTo(emailTwo); + assertThat(editInfo.commit.committer.email).isEqualTo(emailOne); } @Test diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java index 9e85d8ce73..c0531e513b 100644 --- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java +++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java @@ -875,6 +875,47 @@ public class RefAdvertisementIT extends AbstractDaemonTest { "refs/tags/new-tag"); } + // rcMaster (c1 master master-tag) <- rcBranch (c2 branch branch-tag) <- rcBranch (c2 branch) <- + // newcommit1 <- newcommit2 (new-branch) + @Test + public void uploadPackReachableTagVisibleFromLeafBranch() throws Exception { + try (Repository repo = repoManager.openRepository(project)) { + // rcBranch (c2 branch) <- newcommit1 (new-branch) + PushOneCommit.Result r = + pushFactory + .create(admin.newIdent(), testRepo) + .setParent(rcBranch) + .to("refs/heads/new-branch"); + r.assertOkStatus(); + RevCommit branchRc = r.getCommit(); + + // rcBranch (c2) <- newcommit1 <- newcommit2 (new-branch) + r = + pushFactory + .create(admin.newIdent(), testRepo) + .setParent(branchRc) + .to("refs/heads/new-branch"); + r.assertOkStatus(); + } + + projectOperations + .project(project) + .forUpdate() + .add(deny(Permission.READ).ref("refs/heads/master").group(REGISTERED_USERS)) + .add(deny(Permission.READ).ref("refs/heads/branch").group(REGISTERED_USERS)) + .add(allow(Permission.READ).ref("refs/heads/new-branch").group(REGISTERED_USERS)) + .update(); + + requestScopeOperations.setApiUser(user.id()); + assertUploadPackRefs( + "refs/heads/new-branch", + // 'master' and 'branch' branches are not visible but 'master-tag' and 'branch-tag' are + // reachable from new-branch (since PushOneCommit always bases changes on each other). + "refs/tags/branch-tag", + "refs/tags/master-tag"); + // tree-tag not visible. See comment in subsetOfBranchesVisibleIncludingHead. + } + // first ls-remote: rcBranch (c2 branch) <- newcommit1 (updated-tag) // second ls-remote: rcBranch (c2 branch updated-tag) @Test diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java index 73e0d173be..37684de8c7 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByCherryPickIT.java @@ -144,7 +144,7 @@ public class SubmitByCherryPickIT extends AbstractSubmit { revision.review(ReviewInput.approve()); revision.submit(); assertThat(gApi.changes().id(changeId.get()).get().getCurrentRevision().commit.committer.email) - .isEqualTo(emailTwo); + .isEqualTo(emailOne); } @Test diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java index a03a5b5e45..80fbe99bab 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/SubmitByRebaseAlwaysIT.java @@ -146,7 +146,7 @@ public class SubmitByRebaseAlwaysIT extends AbstractSubmitByRebase { revision.review(ReviewInput.approve()); revision.submit(); assertThat(gApi.changes().id(changeId.get()).get().getCurrentRevision().commit.committer.email) - .isEqualTo(emailTwo); + .isEqualTo(emailOne); } @Test diff --git a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java index ea8e0a74b3..90a9b9d631 100644 --- a/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java +++ b/javatests/com/google/gerrit/server/index/change/FakeQueryBuilder.java @@ -59,6 +59,7 @@ public class FakeQueryBuilder extends ChangeQueryBuilder { null, null, null, + null, null)); } diff --git a/javatests/com/google/gerrit/server/patch/DiffValidatorsTest.java b/javatests/com/google/gerrit/server/patch/DiffValidatorsTest.java deleted file mode 100644 index fa1d09ed30..0000000000 --- a/javatests/com/google/gerrit/server/patch/DiffValidatorsTest.java +++ /dev/null @@ -1,74 +0,0 @@ -// Copyright (C) 2023 The Android Open Source Project -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package com.google.gerrit.server.patch; - -import static com.google.common.truth.Truth.assertThat; -import static com.google.gerrit.testing.GerritJUnit.assertThrows; - -import com.google.common.collect.ImmutableList; -import com.google.gerrit.entities.Patch.ChangeType; -import com.google.gerrit.entities.Patch.FileMode; -import com.google.gerrit.entities.Patch.PatchType; -import com.google.gerrit.server.git.LargeObjectException; -import com.google.gerrit.server.patch.filediff.FileDiffOutput; -import com.google.gerrit.testing.InMemoryModule; -import com.google.inject.Guice; -import com.google.inject.Inject; -import com.google.inject.Injector; -import java.util.Optional; -import org.eclipse.jgit.lib.ObjectId; -import org.junit.Before; -import org.junit.Test; - -/** Test class for {@link DiffValidators}. */ -public class DiffValidatorsTest { - @Inject private DiffValidators diffValidators; - - @Before - public void setUpInjector() throws Exception { - Injector injector = Guice.createInjector(new InMemoryModule()); - injector.injectMembers(this); - } - - @Test - public void fileSizeExceeded() { - int largeSize = 100000000; - FileDiffOutput fileDiff = - FileDiffOutput.builder() - .oldCommitId(ObjectId.zeroId()) - .newCommitId(ObjectId.zeroId()) - .comparisonType(ComparisonType.againstRoot()) - .changeType(ChangeType.ADDED) - .patchType(Optional.of(PatchType.UNIFIED)) - .oldPath(Optional.empty()) - .newPath(Optional.of("f.txt")) - .oldMode(Optional.empty()) - .newMode(Optional.of(FileMode.REGULAR_FILE)) - .headerLines(ImmutableList.of()) - .edits(ImmutableList.of()) - .size(largeSize) - .sizeDelta(largeSize) - .build(); - Exception thrown = - assertThrows(LargeObjectException.class, () -> diffValidators.validate(fileDiff)); - assertThat(thrown) - .hasMessageThat() - .isEqualTo( - String.format( - "File size for file f.txt exceeded the max file size threshold." - + " Threshold = %d bytes, Actual size = %d bytes", - DiffFileSizeValidator.DEFAULT_MAX_FILE_SIZE, largeSize)); - } -} diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts index 57bb8757f4..aa569adb62 100644 --- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts +++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.ts @@ -1025,23 +1025,17 @@ export class GrChangeActions } private actionsChanged() { - this.hidden = - Object.keys(this.actions).length === 0 && - Object.keys(this.revisionActions).length === 0 && - this.additionalActions.length === 0; this.actionLoadingMessage = ''; this.disabledMenuActions = []; - if (Object.keys(this.revisionActions).length !== 0) { - if (!this.revisionActions.download) { - this.revisionActions = { - ...this.revisionActions, - download: DOWNLOAD_ACTION, - }; - fire(this, 'revision-actions-changed', { - value: this.revisionActions, - }); - } + if (!this.revisionActions.download) { + this.revisionActions = { + ...this.revisionActions, + download: DOWNLOAD_ACTION, + }; + fire(this, 'revision-actions-changed', { + value: this.revisionActions, + }); } if ( !this.actions.includedIn && diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts index ae6044912d..f0836093f1 100644 --- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts +++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.ts @@ -390,7 +390,7 @@ suite('gr-change-view tests', () => { </gr-copy-clipboard> </div> <div class="commitActions"> - <gr-change-actions hidden="" id="actions"> </gr-change-actions> + <gr-change-actions id="actions"> </gr-change-actions> </div> </div> <h2 class="assistive-tech-only">Change metadata</h2> diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts index d053928d26..97b8de3913 100644 --- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts +++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.ts @@ -835,10 +835,14 @@ export class GrFileList extends LitElement { if (changedProperties.has('files')) { this.filesChanged(); this.numFilesShown = Math.min(this.files.length, DEFAULT_NUM_FILES_SHOWN); + fire(this, 'files-shown-changed', {length: this.numFilesShown}); } if (changedProperties.has('expandedFiles')) { this.expandedFilesChanged(changedProperties.get('expandedFiles')); } + if (changedProperties.has('numFilesShown')) { + fire(this, 'files-shown-changed', {length: this.numFilesShown}); + } } override connectedCallback() { diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts index bd81828907..5fcafc3ff8 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.ts @@ -395,7 +395,6 @@ export class GrSettingsView extends LitElement { this.serverConfig?.auth.use_contributor_agreements, () => html`<li><a href="#Agreements">Agreements</a></li>` )} - <li><a href="#MailFilters">Mail Filters</a></li> <gr-endpoint-decorator name="settings-menu-item"> </gr-endpoint-decorator> </ul> @@ -680,91 +679,6 @@ export class GrSettingsView extends LitElement { <gr-agreements-list id="agreementsList"></gr-agreements-list> </fieldset>` )} - <h2 id="MailFilters">Mail Filters</h2> - <fieldset class="filters"> - <p> - Gerrit emails include metadata about the change to support writing - mail filters. - </p> - <p> - Here are some example Gmail queries that can be used for filters - or for searching through archived messages. View the - <a - href=${this.getFilterDocsLink(this.docsBaseUrl)} - target="_blank" - rel="noopener noreferrer" - >Gerrit documentation</a - > - for the complete set of footers. - </p> - <table> - <tbody> - <tr> - <th>Name</th> - <th>Query</th> - </tr> - <tr> - <td>Changes requesting my review</td> - <td> - <code class="queryExample"> - "Gerrit-Reviewer: <em>Your Name</em> - <<em>your.email@example.com</em>>" - </code> - </td> - </tr> - <tr> - <td>Changes requesting my attention</td> - <td> - <code class="queryExample"> - "Gerrit-Attention: <em>Your Name</em> - <<em>your.email@example.com</em>>" - </code> - </td> - </tr> - <tr> - <td>Changes from a specific owner</td> - <td> - <code class="queryExample"> - "Gerrit-Owner: <em>Owner name</em> - <<em>owner.email@example.com</em>>" - </code> - </td> - </tr> - <tr> - <td>Changes targeting a specific branch</td> - <td> - <code class="queryExample"> - "Gerrit-Branch: <em>branch-name</em>" - </code> - </td> - </tr> - <tr> - <td>Changes in a specific project</td> - <td> - <code class="queryExample"> - "Gerrit-Project: <em>project-name</em>" - </code> - </td> - </tr> - <tr> - <td>Messages related to a specific Change ID</td> - <td> - <code class="queryExample"> - "Gerrit-Change-Id: <em>Change ID</em>" - </code> - </td> - </tr> - <tr> - <td>Messages related to a specific change number</td> - <td> - <code class="queryExample"> - "Gerrit-Change-Number: <em>change number</em>" - </code> - </td> - </tr> - </tbody> - </table> - </fieldset> <gr-endpoint-decorator name="settings-screen"> </gr-endpoint-decorator> </div> diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts index d77d35cda2..30a2922922 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.ts @@ -148,7 +148,6 @@ suite('gr-settings-view tests', () => { <li><a href="#EmailAddresses"> Email Addresses </a></li> <li><a href="#Groups"> Groups </a></li> <li><a href="#Identities"> Identities </a></li> - <li><a href="#MailFilters"> Mail Filters </a></li> <gr-endpoint-decorator name="settings-menu-item"> </gr-endpoint-decorator> </ul> @@ -454,92 +453,6 @@ suite('gr-settings-view tests', () => { <fieldset> <gr-identities id="identities"> </gr-identities> </fieldset> - <h2 id="MailFilters">Mail Filters</h2> - <fieldset class="filters"> - <p> - Gerrit emails include metadata about the change to support writing - mail filters. - </p> - <p> - Here are some example Gmail queries that can be used for filters - or for searching through archived messages. View the - <a - href="https://test.com/user-notify.html" - rel="noopener noreferrer" - target="_blank" - > - Gerrit documentation - </a> - for the complete set of footers. - </p> - <table> - <tbody> - <tr> - <th>Name</th> - <th>Query</th> - </tr> - <tr> - <td>Changes requesting my review</td> - <td> - <code class="queryExample"> - "Gerrit-Reviewer: <em> Your Name </em> < - <em> your.email@example.com </em> >" - </code> - </td> - </tr> - <tr> - <td>Changes requesting my attention</td> - <td> - <code class="queryExample"> - "Gerrit-Attention: <em> Your Name </em> < - <em> your.email@example.com </em> >" - </code> - </td> - </tr> - <tr> - <td>Changes from a specific owner</td> - <td> - <code class="queryExample"> - "Gerrit-Owner: <em> Owner name </em> < - <em> owner.email@example.com </em> >" - </code> - </td> - </tr> - <tr> - <td>Changes targeting a specific branch</td> - <td> - <code class="queryExample"> - "Gerrit-Branch: <em> branch-name </em> " - </code> - </td> - </tr> - <tr> - <td>Changes in a specific project</td> - <td> - <code class="queryExample"> - "Gerrit-Project: <em> project-name </em> " - </code> - </td> - </tr> - <tr> - <td>Messages related to a specific Change ID</td> - <td> - <code class="queryExample"> - "Gerrit-Change-Id: <em> Change ID </em> " - </code> - </td> - </tr> - <tr> - <td>Messages related to a specific change number</td> - <td> - <code class="queryExample"> - "Gerrit-Change-Number: <em> change number </em> " - </code> - </td> - </tr> - </tbody> - </table> - </fieldset> <gr-endpoint-decorator name="settings-screen"> </gr-endpoint-decorator> </div> diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts index d27be9fc5f..e2229b5ce7 100644 --- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts +++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts @@ -1187,7 +1187,10 @@ export class GrComment extends LitElement { async createSuggestEdit(e: MouseEvent) { e.stopPropagation(); const line = await this.getCommentedCode(); - this.messageText += `${USER_SUGGESTION_START_PATTERN}${line}${'\n```'}`; + const addNewLine = this.messageText.length !== 0; + this.messageText += `${ + addNewLine ? '\n' : '' + }${USER_SUGGESTION_START_PATTERN}${line}${'\n```'}`; } async getCommentedCode() { diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts index 70e653afb2..e557ca8bea 100644 --- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts +++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-change-actions-js-api_test.ts @@ -147,20 +147,17 @@ suite('gr-change-actions-js-api-interface tests', () => { test('move action button to overflow', async () => { const key = changeActions.add(ActionType.REVISION, 'Bork!'); await element.updateComplete; - assert.isTrue(queryAndAssert<GrDropdown>(element, '#moreActions').hidden); - assert.isOk( - queryAndAssert<GrButton>(element, `[data-action-key="${key}"]`) - ); + + let items = queryAndAssert<GrDropdown>(element, '#moreActions').items; + assert.isFalse(items?.some(item => item.name === 'Bork!')); + assert.isOk(query<GrButton>(element, `[data-action-key="${key}"]`)); + changeActions.setActionOverflow(ActionType.REVISION, key, true); await element.updateComplete; + + items = queryAndAssert<GrDropdown>(element, '#moreActions').items; + assert.isTrue(items?.some(item => item.name === 'Bork!')); assert.isNotOk(query<GrButton>(element, `[data-action-key="${key}"]`)); - assert.isFalse( - queryAndAssert<GrDropdown>(element, '#moreActions').hidden - ); - assert.strictEqual( - queryAndAssert<GrDropdown>(element, '#moreActions').items![0].name, - 'Bork!' - ); }); test('change actions priority', async () => { diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts index c29417e0c2..4afdf00742 100644 --- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts +++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea.ts @@ -122,9 +122,11 @@ export class GrTextarea extends LitElement { private changeNum?: NumericChangeId; + // Represents the current location of the ':' or '@' that triggered a drop-down. // private but used in tests specialCharIndex = -1; + // Represents the current search string being used to query either emoji or mention suggestions. // private but used in tests currentSearchString?: string; @@ -280,8 +282,7 @@ export class GrTextarea extends LitElement { this.fireChangedEvents(); // Add to updated because we want this.textarea.selectionStart and // this.textarea is null in the willUpdate lifecycle - this.computeSpecialCharIndex(); - this.computeCurrentSearchString(); + this.computeIndexAndSearchString(); this.handleTextChanged(); } } @@ -373,6 +374,13 @@ export class GrTextarea extends LitElement { return; } + const selection = this.getVisibleDropdown().getCurrentText(); + if (selection === '') { + // Nothing was selected, so treat this like a newline and reset the dropdown. + this.indent(e); + this.resetDropdown(); + return; + } e.preventDefault(); e.stopPropagation(); this.setValue(this.getVisibleDropdown().getCurrentText()); @@ -402,17 +410,17 @@ export class GrTextarea extends LitElement { // below needs to happen after iron-autogrow-textarea has set the // incorrect value. await this.updateComplete; - this.textarea!.selectionStart = specialCharIndex + 1; - this.textarea!.selectionEnd = specialCharIndex + 1; + this.textarea!.selectionStart = specialCharIndex + text.length + 1; + this.textarea!.selectionEnd = specialCharIndex + text.length + 1; this.resetDropdown(); } private addValueToText(value: string) { if (!this.text) return ''; return ( - this.text.substr(0, this.specialCharIndex ?? 0) + + this.text.substring(0, this.specialCharIndex ?? 0) + value + - this.text.substr(this.textarea!.selectionStart) + this.text.substring(this.textarea!.selectionStart) ); } @@ -425,7 +433,7 @@ export class GrTextarea extends LitElement { */ updateCaratPosition() { if (typeof this.textarea!.value === 'string') { - this.hiddenText!.textContent = this.textarea!.value.substr( + this.hiddenText!.textContent = this.textarea!.value.substring( 0, this.textarea!.selectionStart ); @@ -436,12 +444,7 @@ export class GrTextarea extends LitElement { return caratSpan; } - private shouldResetDropdown( - text: string, - charIndex: number, - suggestions?: Item[], - char?: string - ) { + private shouldResetDropdown(text: string, charIndex: number, char?: string) { // Under any of the following conditions, close and reset the dropdown: // - The cursor is no longer at the end of the current search string // - The search string is an space or new line @@ -452,32 +455,10 @@ export class GrTextarea extends LitElement { (this.currentSearchString ?? '').length + charIndex + 1 || this.currentSearchString === ' ' || this.currentSearchString === '\n' || - !(text[charIndex] === char) || - !suggestions || - !suggestions.length + !(text[charIndex] === char) ); } - // When special char is detected, set index. We are interested only on - // special char after space or in beginning of textarea - // In case of mentions we are interested if previous char is '\n' as well - private getSpecialCharIndex(text: string) { - const charAtCursor = text[this.textarea!.selectionStart - 1]; - if ( - this.textarea!.selectionStart < 2 || - text[this.textarea!.selectionStart - 2] === ' ' - ) { - return this.textarea!.selectionStart - 1; - } - if ( - charAtCursor === '@' && - text[this.textarea!.selectionStart - 2] === '\n' - ) { - return this.textarea!.selectionStart - 1; - } - return -1; - } - private async computeSuggestions() { this.suggestions = []; if (this.currentSearchString === undefined) { @@ -513,7 +494,6 @@ export class GrTextarea extends LitElement { this.shouldResetDropdown( this.text, this.specialCharIndex, - this.suggestions, this.text[this.specialCharIndex] ) ) { @@ -539,26 +519,20 @@ export class GrTextarea extends LitElement { ); } - private computeSpecialCharIndex() { - const charAtCursor = this.text[this.textarea!.selectionStart - 1]; - - if (charAtCursor === '@' && this.specialCharIndex === -1) { - this.specialCharIndex = this.getSpecialCharIndex(this.text); - } - if (charAtCursor === ':' && this.specialCharIndex === -1) { - this.specialCharIndex = this.getSpecialCharIndex(this.text); - } - } - - private computeCurrentSearchString() { - if (this.specialCharIndex === -1) { + private computeIndexAndSearchString() { + const currentCarat = this.textarea?.selectionStart ?? this.text.length; + const m = this.text + .substring(0, currentCarat) + .match(/(?:^|\s)([:@][\S]*)$/); + if (!m) { + this.specialCharIndex = -1; this.currentSearchString = undefined; return; } - this.currentSearchString = this.text.substr( - this.specialCharIndex + 1, - this.textarea!.selectionStart - this.specialCharIndex - 1 - ); + this.currentSearchString = m[1].substring(1); + if (this.specialCharIndex !== -1) return; + + this.specialCharIndex = currentCarat - m[1].length; } // Private but used in tests. @@ -645,7 +619,7 @@ export class GrTextarea extends LitElement { // When nothing is selected, selectionStart is the caret position. We want // the indentation level of the current line, not the end of the text which // may be different. - const currentLine = this.textarea!.textarea.value.substr( + const currentLine = this.textarea!.textarea.value.substring( 0, this.textarea!.selectionStart ) diff --git a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts index 4dcaa80b2d..aa32f7d668 100644 --- a/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts +++ b/polygerrit-ui/app/elements/shared/gr-textarea/gr-textarea_test.ts @@ -220,20 +220,6 @@ suite('gr-textarea tests', () => { ]); }); - test('emoji selector does not open when previous char is \n', async () => { - element.textarea!.focus(); - await waitUntil(() => element.textarea!.focused === true); - - element.textarea!.selectionStart = 1; - element.textarea!.selectionEnd = 1; - element.text = '\n:'; - - await element.updateComplete; - - assert.isTrue(element.emojiSuggestions!.isHidden); - assert.isTrue(element.mentionsSuggestions!.isHidden); - }); - test('selecting mentions from dropdown', async () => { stubRestApi('getSuggestedAccounts').returns( Promise.resolve([ @@ -300,19 +286,19 @@ suite('gr-textarea tests', () => { assert.isTrue(element.emojiSuggestions!.isHidden); assert.isFalse(element.mentionsSuggestions!.isHidden); - element.text = '@h '; + element.text = '@h'; await waitUntil(() => element.suggestions.length > 0); await element.updateComplete; assert.isTrue(element.emojiSuggestions!.isHidden); assert.isFalse(element.mentionsSuggestions!.isHidden); - element.text = '@h :'; + element.text = '@h:'; await waitUntil(() => element.suggestions.length > 0); await element.updateComplete; assert.isTrue(element.emojiSuggestions!.isHidden); assert.isFalse(element.mentionsSuggestions!.isHidden); - element.text = '@h :D'; + element.text = '@h:D'; await waitUntil(() => element.suggestions.length > 0); await element.updateComplete; assert.isTrue(element.emojiSuggestions!.isHidden); @@ -347,11 +333,16 @@ suite('gr-textarea tests', () => { element.text = ':D@'; await element.updateComplete; // emoji dropdown hidden since we have no more suggestions - assert.isTrue(element.emojiSuggestions!.isHidden); + assert.isFalse(element.emojiSuggestions!.isHidden); assert.isTrue(element.mentionsSuggestions!.isHidden); element.text = ':D@b'; await element.updateComplete; + assert.isFalse(element.emojiSuggestions!.isHidden); + assert.isTrue(element.mentionsSuggestions!.isHidden); + + element.text = ':D@b '; + await element.updateComplete; assert.isTrue(element.emojiSuggestions!.isHidden); assert.isTrue(element.mentionsSuggestions!.isHidden); }); diff --git a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts index 5d23da12fb..b3c7dad688 100644 --- a/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts +++ b/polygerrit-ui/app/embed/diff/gr-diff/gr-diff.ts @@ -319,9 +319,14 @@ export class GrDiff extends LitElement implements GrDiffApi { if (this.diffElement) { this.highlights.init(this.diffElement, this); } + this.observeNodes(); } override disconnectedCallback() { + if (this.nodeObserver) { + this.nodeObserver.disconnect(); + this.nodeObserver = undefined; + } this.removeSelectionListeners(); this.diffSelection.cleanup(); this.highlights.cleanup(); @@ -422,7 +427,6 @@ export class GrDiff extends LitElement implements GrDiffApi { if (changedProperties.has('groups')) { if (this.groups?.length > 0) { this.loading = false; - this.observeNodes(); } } } diff --git a/tools/bzl/classpath.bzl b/tools/bzl/classpath.bzl index 3be7a12507..3c80fc38dc 100644 --- a/tools/bzl/classpath.bzl +++ b/tools/bzl/classpath.bzl @@ -2,7 +2,7 @@ def _classpath_collector(ctx): all = [] for d in ctx.attr.deps: if JavaInfo in d: - all.append(d[JavaInfo].transitive_runtime_deps) + all.append(d[JavaInfo].transitive_runtime_jars) if hasattr(d[JavaInfo].compilation_info, "runtime_classpath"): all.append(d[JavaInfo].compilation_info.runtime_classpath) elif hasattr(d, "files"): diff --git a/tools/bzl/javadoc.bzl b/tools/bzl/javadoc.bzl index 3add0258ce..a18110433c 100644 --- a/tools/bzl/javadoc.bzl +++ b/tools/bzl/javadoc.bzl @@ -17,7 +17,7 @@ def _impl(ctx): zip_output = ctx.outputs.zip - transitive_jars = depset(transitive = [j[JavaInfo].transitive_deps for j in ctx.attr.libs]) + transitive_jars = depset(transitive = [j[JavaInfo].transitive_compile_time_jars for j in ctx.attr.libs]) # TODO(davido): Remove list to depset conversion on source_jars, when this issue is fixed: # https://github.com/bazelbuild/bazel/issues/4221 diff --git a/tools/bzl/pkg_war.bzl b/tools/bzl/pkg_war.bzl index e2be145b68..4792de2f2a 100644 --- a/tools/bzl/pkg_war.bzl +++ b/tools/bzl/pkg_war.bzl @@ -88,7 +88,7 @@ def _war_impl(ctx): transitive_libs = [] for j in ctx.attr.libs: if JavaInfo in j: - transitive_libs.append(j[JavaInfo].transitive_runtime_deps) + transitive_libs.append(j[JavaInfo].transitive_runtime_jars) elif hasattr(j, "files"): transitive_libs.append(j.files) @@ -102,7 +102,7 @@ def _war_impl(ctx): # Add pgm lib transitive_pgmlibs = [] for j in ctx.attr.pgmlibs: - transitive_pgmlibs.append(j[JavaInfo].transitive_runtime_deps) + transitive_pgmlibs.append(j[JavaInfo].transitive_runtime_jars) transitive_pgmlib_deps = depset(transitive = transitive_pgmlibs) for dep in transitive_pgmlib_deps.to_list(): @@ -117,7 +117,7 @@ def _war_impl(ctx): if ctx.attr.context: for jar in ctx.attr.context: if JavaInfo in jar: - transitive_context_libs.append(jar[JavaInfo].transitive_runtime_deps) + transitive_context_libs.append(jar[JavaInfo].transitive_runtime_jars) elif hasattr(jar, "files"): transitive_context_libs.append(jar.files) |