Merge "RebaseUtil: Support base revisions which are part of the dest branch"
diff --git a/Documentation/config-project-config.txt b/Documentation/config-project-config.txt
index a6babb9..1ad0270 100644
--- a/Documentation/config-project-config.txt
+++ b/Documentation/config-project-config.txt
@@ -228,7 +228,7 @@
[NOTE]
Project owners can be allowed to change the parent of projects that they own
-(see config-gerrit.html#receive.allowProjectOwnersToChangeParent[
+(see link:config-gerrit.html#receive.allowProjectOwnersToChangeParent[
receive.allowProjectOwnersToChangeParent] setting which is `false` by default).
In this case project owners may escape the settings that are enforced by their
parent project by choosing a different parent project.
diff --git a/Documentation/intro-project-owner.txt b/Documentation/intro-project-owner.txt
index 97b58af..15f27db 100644
--- a/Documentation/intro-project-owner.txt
+++ b/Documentation/intro-project-owner.txt
@@ -44,7 +44,7 @@
To see the access rights of your project
- go to the Gerrit Web UI
-- click on the `Projects` > `List` menu entry
+- click on the `BROWSE` > `Repositories` menu entry
- find your project in the project list and click on it
- click on the `Access` menu entry
@@ -150,7 +150,7 @@
group backends.
The Gerrit internal groups can be seen in the Gerrit Web UI by clicking
-on the `Groups` > `List` menu entry. By clicking on a group you can
+on the `BROWSE` > `Groups` menu entry. By clicking on a group you can
edit the group members (`Members` tab) and the group options
(`General` tab).
@@ -168,8 +168,9 @@
`Make group visible to all registered users.`, which defines whether
non-members can see who is member of the group.
-New internal Gerrit groups can be created under `Groups` >
-`Create New Group`. This menu is only available if you have the global
+New internal Gerrit groups can be created under `BROWSE` > `Groups`
+and then clicking on the `CREATE NEW` button in the upper right corner.
+The `CREATE NEW` button is only available if you have the global
capability link:access-control.html#capability_createGroup[Create Group]
assigned.
@@ -238,7 +239,7 @@
To see the options of your project:
. Go to the Gerrit Web UI.
-. Click on the `Projects` > `List` menu entry.
+. Click on the `BROWSE` > `Repositories` menu entry.
. Find your project in the project list and click it.
. Click the `General` menu entry.
@@ -431,7 +432,7 @@
== Branch Administration
As project owner you can administrate the branches of your project in
-the Gerrit Web UI under `Projects` > `List` > <your project> >
+the Gerrit Web UI under `BROWSE` > `Repositories` > <your project> >
`Branches`. In the Web UI link:project-configuration.html#branch-creation[
branch creation] is allowed if you have
link:access-control.html#category_create[Create Reference] access right and
@@ -450,7 +451,7 @@
With Gerrit individual users control their own email subscriptions. By
editing the link:user-notify.html#user[watched projects] in the Web UI
-under `Settings` > `Watched Projects` users can decide which events to
+under `Settings` > `Notifications` users can decide which events to
be informed about by email. The change notifications can be filtered by
link:user-search.html[change search expressions].
@@ -476,8 +477,8 @@
link:user-dashboards.html#project-dashboards[project level]. This way
you can define a view of the changes that are relevant for your
project and share this dashboard with all users. The project dashboards
-can be seen in the Web UI under `Projects` > `List` > <your project> >
-`Dashboards`.
+can be seen in the Web UI under `BROWSE` > `Repositories` > <your project>
+> `Dashboards`.
[[issue-tracker-integration]]
== Issue Tracker Integration
@@ -509,7 +510,7 @@
to Gerrit changes to the issues in the issue tracker system or to
automatically close an issue if the corresponding change is merged.
If installed, project owners may enable/disable the issue tracker
-integration from the Gerrit Web UI under `Projects` > `Lists` >
+integration from the Gerrit Web UI under `BROWSE` > `Repositories` >
<your project> > `General`.
[[comment-links]]
@@ -554,7 +555,7 @@
With the link:https://gerrit-review.googlesource.com/admin/repos/plugins/reviewers[
reviewers,role=external,window=_blank] plugin it is possible to configure default reviewers who
will be automatically added to each change. The default reviewers can
-be configured in the Gerrit Web UI under `Projects` > `List` >
+be configured in the Gerrit Web UI under `BROWSE` > `Repositories` >
<your project> > `General` in the `reviewers Plugin` section.
The link:https://gerrit-review.googlesource.com/admin/repos/plugins/reviewers-by-blame[
@@ -565,7 +566,7 @@
touched by the change, since these users should be familiar with the
code and can most likely review the change. How many reviewers the
plugin will add to a change at most can be configured in the Gerrit
-Web UI under `Projects` > `List` > <your project> > `General` in the
+Web UI under `BROWSE` > `Repositories` > <your project> > `General` in the
`reviewers-by-blame Plugin` section.
[[download-commands]]
@@ -650,9 +651,10 @@
[[project-creation]]
=== Project Creation
-New projects can be created in the Gerrit Web UI under `Projects` >
-`Create Project`. The `Create Project` menu entry is only available if
-you have the link:access-control.html#capability_createProject[
+New projects can be created in the Gerrit Web UI under `BROWSE` >
+`Repositories` and then clicking on the `CREATE NEW` button in the
+upper right corner. The `CREATE NEW` button is only available if you
+have the link:access-control.html#capability_createProject[
Create Project] global capability assigned.
Projects can also be created via REST or SSH as described in the
@@ -736,7 +738,7 @@
If the link:https://gerrit-review.googlesource.com/admin/repos/plugins/delete-project[
delete-project,role=external,window=_blank] plugin is installed, projects can be deleted from the
-Gerrit Web UI under `Projects` > `List` > <project> > `General` by
+Gerrit Web UI under `BROWSE` > `Repositories` > <project> > `General` by
clicking on the `Delete` command under `Project Commands`. The `Delete`
command is only available if you have the `Delete Projects` global
capability assigned, or if you own the project and you have the
diff --git a/Documentation/intro-user.txt b/Documentation/intro-user.txt
index 4c61073..108022a9 100644
--- a/Documentation/intro-user.txt
+++ b/Documentation/intro-user.txt
@@ -31,7 +31,7 @@
* link:https://github.com/uwolfer/gerrit-intellij-plugin[Gerrit
IntelliJ Plugin,role=external,window=_blank]: Gerrit integration with the
link:http://www.jetbrains.com/idea/[IntelliJ Platform,role=external,window=_blank]
-* link:https://opendev.org/ttygroup/gertty]: Console-based interface for Gerrit
+* link:https://opendev.org/ttygroup/gertty[gertty]: Console-based interface for Gerrit
[[clone]]
== Clone Gerrit Project
diff --git a/java/com/google/gerrit/server/CommentsUtil.java b/java/com/google/gerrit/server/CommentsUtil.java
index 55321d3..e290002 100644
--- a/java/com/google/gerrit/server/CommentsUtil.java
+++ b/java/com/google/gerrit/server/CommentsUtil.java
@@ -246,6 +246,7 @@
.collect(toCollection(ArrayList::new));
int cmItr = 0;
+ int lastMatch = 0;
for (CommentInfo comment : sortedCommentInfos) {
// Keep advancing the change message pointer until we associate the comment to the next change
// message in timestamp
@@ -256,6 +257,7 @@
|| (skipAutoGeneratedMessages && isAutoGenerated(cm))) {
cmItr += 1;
} else {
+ lastMatch = cmItr;
break;
}
}
@@ -265,11 +267,11 @@
// In case of no match "cmItr" will never be less than "changeMessages" size, hence the
// changeMessageId won't be set for any comment.
//
- // Reset the search from the beginning, since we can't assume there will always be a match
- // between change messages and comments. This could be the case of imported changes.
+ // Reset the search to the last succesful match, since we can't assume there will always be
+ // a match between change messages and comments. This could be the case of imported changes.
//
// More details here: https://issues.gerritcodereview.com/issues/318079520
- cmItr = 0;
+ cmItr = lastMatch;
}
}
}
diff --git a/java/com/google/gerrit/server/change/IncludedIn.java b/java/com/google/gerrit/server/change/IncludedIn.java
index 1faf121..f104a57 100644
--- a/java/com/google/gerrit/server/change/IncludedIn.java
+++ b/java/com/google/gerrit/server/change/IncludedIn.java
@@ -39,7 +39,6 @@
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collection;
-import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
@@ -107,10 +106,7 @@
Stream<String> filteredTagsStream =
sortedShortNames(
filterReadableRefs(project, getMatchingRefNames(allMatchingTagsAndBranches, tags)));
- for (Iterator<PluginSetEntryContext<FilterIncludedIn>> pluginFilterIt =
- filterIncludedIn.iterator();
- pluginFilterIt.hasNext(); ) {
- PluginSetEntryContext<FilterIncludedIn> pluginFilter = pluginFilterIt.next();
+ for (PluginSetEntryContext<FilterIncludedIn> pluginFilter : filterIncludedIn) {
filteredBranchesStream =
filteredBranchesStream.filter(pluginFilter.get().getBranchFilter(project, rev));
filteredTagsStream =
diff --git a/java/com/google/gerrit/server/change/RebaseChangeOp.java b/java/com/google/gerrit/server/change/RebaseChangeOp.java
index e94f197..5daac75 100644
--- a/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -45,6 +45,7 @@
import com.google.gerrit.server.git.GroupCollector;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.MergeUtilFactory;
+import com.google.gerrit.server.logging.CallerFinder;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.DiffNotAvailableException;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -98,6 +99,7 @@
private final RebaseUtil rebaseUtil;
private final ChangeResource.Factory changeResourceFactory;
private final ChangeNotes.Factory notesFactory;
+ private final CallerFinder callerFinder;
private final ChangeNotes notes;
private final PatchSet originalPatchSet;
@@ -199,6 +201,7 @@
this.notes = notes;
this.projectName = notes.getProjectName();
this.originalPatchSet = originalPatchSet;
+ this.callerFinder = CallerFinder.builder().addTarget(RebaseChangeOp.class).build();
}
@CanIgnoreReturnValue
@@ -378,6 +381,8 @@
}
}
+ logger.atFine().log(
+ "flushing inserter %s", ctx.getRevWalk().getObjectReader().getCreatedFromInserter());
ctx.getRevWalk().getObjectReader().getCreatedFromInserter().flush();
patchSetInserter.updateRepo(ctx);
}
@@ -494,6 +499,9 @@
if (success) {
filesWithGitConflicts = null;
tree = merger.getResultTreeId();
+ logger.atFine().log(
+ "tree of rebased commit: %s (no conflicts, inserter: %s, caller: %s)",
+ tree.name(), merger.getObjectInserter(), callerFinder.findCallerLazy());
} else {
List<String> conflicts = ImmutableList.of();
Map<String, ResolveMerger.MergeFailureReason> failed = ImmutableMap.of();
@@ -551,6 +559,9 @@
"BASE",
ctx.getRevWalk().parseCommit(base),
mergeResults);
+ logger.atFine().log(
+ "tree of rebased commit: %s (with conflicts, inserter: %s, caller: %s)",
+ tree.name(), ctx.getInserter(), callerFinder.findCallerLazy());
}
List<ObjectId> parents = new ArrayList<>();
@@ -581,11 +592,10 @@
new PersonIdent(
cb.getAuthor(), cb.getCommitter().getWhen(), cb.getCommitter().getTimeZone()));
}
- logger.atFine().log(
- "tree of rebased commit: %s (inserter: %s)", tree.name(), ctx.getInserter());
ObjectId objectId = ctx.getInserter().insert(cb);
CodeReviewCommit commit = ((CodeReviewRevWalk) ctx.getRevWalk()).parseCommit(objectId);
commit.setFilesWithGitConflicts(filesWithGitConflicts);
+ logger.atFine().log("rebased commit=%s", commit.name());
return commit;
}
}
diff --git a/java/com/google/gerrit/server/change/WalkSorter.java b/java/com/google/gerrit/server/change/WalkSorter.java
index 6c903b8..cc87ea8 100644
--- a/java/com/google/gerrit/server/change/WalkSorter.java
+++ b/java/com/google/gerrit/server/change/WalkSorter.java
@@ -23,6 +23,7 @@
import com.google.common.collect.ListMultimap;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.Ordering;
+import com.google.common.collect.SetMultimap;
import com.google.common.flogger.FluentLogger;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.gerrit.entities.PatchSet;
@@ -146,8 +147,8 @@
Set<RevCommit> commits = byCommit.keySet();
ListMultimap<RevCommit, RevCommit> children = collectChildren(commits);
- ListMultimap<RevCommit, RevCommit> pending =
- MultimapBuilder.hashKeys().arrayListValues().build();
+ SetMultimap<RevCommit, RevCommit> pending =
+ MultimapBuilder.hashKeys().hashSetValues().build();
Deque<RevCommit> todo = new ArrayDeque<>();
RevFlag done = rw.newFlag("done");
@@ -156,6 +157,7 @@
int found = 0;
RevCommit c;
List<PatchSetData> result = new ArrayList<>(expected);
+ int maxPopSize = commits.size() * commits.size();
while (found < expected && (c = rw.next()) != null) {
if (!commits.contains(c)) {
continue;
@@ -164,9 +166,15 @@
todo.add(c);
int i = 0;
while (!todo.isEmpty()) {
- // Sanity check: we can't pop more than N pending commits, otherwise
- // we have an infinite loop due to programmer error or something.
- checkState(++i <= commits.size(), "Too many pending steps while sorting %s", commits);
+ // Sanity check: in the worst case scenario, each commit can add all previous commits in
+ // the todo queue. This can lead to (n-1) + (n-2) + ... +1 iterations of the algorithm.
+ // So, in the worst case we can't pop more than N^2 pending commits, otherwise
+ // we have an infinite loop due to programmer error or something. (actually, it is
+ // (N-1) + (N-2) + (N-3) + (1) + 1 = N/2*(N-1)+1, but N^2 is enough for us.)
+ checkState(
+ ++i <= maxPopSize,
+ "Too many pending steps while sorting %s - can be a problem in the algorithm.",
+ commits);
RevCommit t = todo.removeFirst();
if (t.has(done)) {
continue;
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java
index 6364fc9..1adbb67 100644
--- a/java/com/google/gerrit/server/git/MergeUtil.java
+++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -1055,6 +1055,12 @@
@Override
public void close() {}
+
+ @Override
+ public String toString() {
+ return String.format(
+ "%s (wrapped inserter: %s)", super.toString(), inserter.toString());
+ }
},
repoConfig);
}
diff --git a/java/com/google/gerrit/server/git/WorkQueue.java b/java/com/google/gerrit/server/git/WorkQueue.java
index 444b5fe..c5e2338 100644
--- a/java/com/google/gerrit/server/git/WorkQueue.java
+++ b/java/com/google/gerrit/server/git/WorkQueue.java
@@ -91,8 +91,8 @@
private final WorkQueue workQueue;
@Inject
- Lifecycle(WorkQueue workQeueue) {
- this.workQueue = workQeueue;
+ Lifecycle(WorkQueue workQueue) {
+ this.workQueue = workQueue;
}
@Override
diff --git a/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java b/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
index 55639e9..5a439f8 100644
--- a/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
+++ b/java/com/google/gerrit/server/mail/send/SmtpEmailSender.java
@@ -163,7 +163,7 @@
if (denyrcpt.contains(address)
|| denyrcpt.contains(domain)
|| denyrcpt.contains("@" + domain)) {
- logger.atWarning().log("Not emailing %s (prohibited by sendemail.denyrcpt)", address);
+ logger.atInfo().log("Not emailing %s (prohibited by sendemail.denyrcpt)", address);
return true;
}
diff --git a/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java b/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
index 185a6ac..56b3842 100644
--- a/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
+++ b/java/com/google/gerrit/server/restapi/change/ApplyPatchUtil.java
@@ -215,8 +215,15 @@
}
private static String decodeIfNecessary(String patch) {
- if (Base64.isBase64(patch)) {
- return new String(org.eclipse.jgit.util.Base64.decode(patch), UTF_8);
+ if (Base64.isBase64(patch.getBytes(UTF_8))) {
+ try {
+ return new String(org.eclipse.jgit.util.Base64.decode(patch), UTF_8);
+ } catch (IllegalArgumentException e) {
+ // It's possible that all the chars in the patch are valid Base64 chars, but the full string
+ // is not a valid Base64 string as expected by jGit. In this case, we assume the patch is
+ // already unencoded.
+ return patch;
+ }
}
return patch;
}
diff --git a/java/com/google/gerrit/server/update/RetryHelper.java b/java/com/google/gerrit/server/update/RetryHelper.java
index 713417d..d9af527 100644
--- a/java/com/google/gerrit/server/update/RetryHelper.java
+++ b/java/com/google/gerrit/server/update/RetryHelper.java
@@ -476,17 +476,24 @@
actionType,
opts,
t -> {
+ String actionName = opts.actionName().orElse("N/A");
+ String cause = formatCause(t);
+
// exceptionPredicate checks for temporary errors for which the operation should be
// retried (e.g. LockFailure). The retry has good chances to succeed.
if (exceptionPredicate.test(t)) {
+ logger.atFine().withCause(t).log(
+ "Retry: %s failed with possibly temporary error (cause = %s)",
+ actionName, cause);
return true;
}
- String actionName = opts.actionName().orElse("N/A");
-
// Exception hooks may identify additional exceptions for retry.
if (exceptionHooks.stream()
.anyMatch(h -> h.shouldRetry(actionType, actionName, t))) {
+ logger.atFine().withCause(t).log(
+ "Retry: %s failed with possibly temporary error (cause = %s)",
+ actionName, cause);
return true;
}
@@ -502,7 +509,6 @@
return false;
}
- String cause = formatCause(t);
if (!TraceContext.isTracing()) {
String traceId = "retry-on-failure-" + new RequestId();
traceContext.addTag(RequestId.Type.TRACE_ID, traceId).forceLogging();
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
index c558a70..78361a1 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ApplyPatchIT.java
@@ -220,6 +220,25 @@
}
@Test
+ public void applyDecodedPatchConsistsOfBase64CharsOnly_success() throws Exception {
+ final String deletedFileName = "file_to_be_deleted";
+ final String deletedFileOriginalContent =
+ "The deletion patch of this file only contain valid Base64 chars.\n"
+ + "However, the patch is not Base64-encoded.\n";
+ final String deletedFileDiff =
+ "diff --git a/file_to_be_deleted b/file_to_be_deleted\n"
+ + "--- a/file_to_be_deleted\n"
+ + "+++ /dev/null\n";
+ initBaseWithFile(deletedFileName, deletedFileOriginalContent);
+ ApplyPatchPatchSetInput in = buildInput(deletedFileDiff);
+
+ ChangeInfo result = applyPatch(in);
+
+ DiffInfo diff = fetchDiffForFile(result, deletedFileName);
+ assertDiffForDeletedFile(diff, deletedFileName, deletedFileOriginalContent);
+ }
+
+ @Test
public void applyGerritBasedPatchWithSingleFile_success() throws Exception {
String head = getHead(repo(), HEAD).name();
createBranchWithRevision(BranchNameKey.create(project, "branch"), head);
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
index 5bdf91f..44fc4bb 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
@@ -22,7 +22,6 @@
import static com.google.gerrit.extensions.restapi.testing.BinaryResultSubject.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.google.gerrit.truth.MapSubject.assertThatMap;
-import static com.google.gerrit.truth.OptionalSubject.assertThat;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -49,6 +48,7 @@
import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.truth.NullAwareCorrespondence;
+import com.google.gerrit.truth.OptionalSubject;
import com.google.inject.Inject;
import java.util.Map;
import org.eclipse.jgit.lib.ObjectId;
@@ -1629,7 +1629,7 @@
TestHumanComment comment = changeOperations.change(changeId).comment(childCommentUuid).get();
- assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
+ OptionalSubject.assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
}
@Test
@@ -1640,7 +1640,7 @@
TestHumanComment comment = changeOperations.change(changeId).comment(childCommentUuid).get();
- assertThat(comment.tag()).value().isEqualTo("tag");
+ OptionalSubject.assertThat(comment.tag()).value().isEqualTo("tag");
}
@Test
@@ -1700,7 +1700,7 @@
TestHumanComment comment =
changeOperations.change(changeId).draftComment(childCommentUuid).get();
- assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
+ OptionalSubject.assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
}
@Test
@@ -1730,7 +1730,7 @@
TestRobotComment comment =
changeOperations.change(changeId).robotComment(childCommentUuid).get();
- assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
+ OptionalSubject.assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
}
private ChangeInfo getChangeFromServer(Change.Id changeId) throws RestApiException {
diff --git a/javatests/com/google/gerrit/server/change/WalkSorterTest.java b/javatests/com/google/gerrit/server/change/WalkSorterTest.java
index 7775452..0d000a5 100644
--- a/javatests/com/google/gerrit/server/change/WalkSorterTest.java
+++ b/javatests/com/google/gerrit/server/change/WalkSorterTest.java
@@ -86,6 +86,114 @@
}
@Test
+ public void seriesOfMergeChangesInSpecialOrder() throws Exception {
+ TestRepository<Repo> p = newRepo("p");
+ // For this test, the RevWalk in the sortProject must return commits in the following order:
+ // c3, c1, c2, c4.
+ // To achieve this order, the commit timestamps must be set in a specific order - RevWalk
+ // returns them sorted by timestamp, starting from the newest one.
+
+ // timestamp: base + 3
+ RevCommit c1 = p.commit().tick(3).create();
+ // timestamp: base + 2
+ RevCommit c2 = p.commit().tick(-1).parent(c1).create();
+ // timestamp: base + 4
+ RevCommit c3 = p.commit().tick(2).parent(c1).parent(c2).create();
+ // timestamp: base + 1
+ RevCommit c4 = p.commit().tick(-3).parent(c3).create();
+
+ ChangeData cd1 = newChange(p, c1);
+ ChangeData cd2 = newChange(p, c2);
+ ChangeData cd3 = newChange(p, c3);
+ ChangeData cd4 = newChange(p, c4);
+
+ List<ChangeData> changes = ImmutableList.of(cd1, cd2, cd3, cd4);
+ WalkSorter sorter = new WalkSorter(repoManager);
+
+ assertSorted(
+ sorter,
+ changes,
+ ImmutableList.of(
+ patchSetData(cd4, c4),
+ patchSetData(cd3, c3),
+ patchSetData(cd2, c2),
+ patchSetData(cd1, c1)));
+ }
+
+ @Test
+ public void seriesOfMergeChangesWorstCaseForTopoSorting() throws Exception {
+ TestRepository<Repo> p = newRepo("p");
+ // For this test, the RevWalk in the sortProject must return commits in the following order:
+ // c1, c2, c3, c4, c5, c6, c7.
+ // To achieve this order, the commit timestamps must be set in a specific order - RevWalk
+ // returns them sorted by timestamp, starting from the newest one.
+
+ // timestamp: base + 8
+ RevCommit c1 = p.commit().tick(8).create();
+ // timestamp: base + 7
+ RevCommit c2 = p.commit().tick(-1).parent(c1).create();
+ // timestamp: base + 6
+ RevCommit c3 = p.commit().tick(-1).parent(c1).parent(c2).create();
+ // timestamp: base + 5
+ RevCommit c4 = p.commit().tick(-1).parent(c1).parent(c2).parent(c3).create();
+ // timestamp: base + 4
+ RevCommit c5 = p.commit().tick(-1).parent(c1).parent(c2).parent(c3).parent(c4).create();
+ // timestamp: base + 3
+ RevCommit c6 =
+ p.commit().tick(-1).parent(c1).parent(c2).parent(c3).parent(c4).parent(c5).create();
+ // timestamp: base + 2
+ RevCommit c7 =
+ p.commit()
+ .tick(-1)
+ .parent(c1)
+ .parent(c2)
+ .parent(c3)
+ .parent(c4)
+ .parent(c5)
+ .parent(c6)
+ .create();
+ // timestamp: base + 1
+ RevCommit c8 =
+ p.commit()
+ .tick(-1)
+ .parent(c1)
+ .parent(c2)
+ .parent(c3)
+ .parent(c4)
+ .parent(c5)
+ .parent(c6)
+ .parent(c7)
+ .create();
+
+ ChangeData cd1 = newChange(p, c1);
+ ChangeData cd2 = newChange(p, c2);
+ ChangeData cd3 = newChange(p, c3);
+ ChangeData cd4 = newChange(p, c4);
+ ChangeData cd5 = newChange(p, c5);
+ ChangeData cd6 = newChange(p, c6);
+ ChangeData cd7 = newChange(p, c7);
+ ChangeData cd8 = newChange(p, c8);
+
+ List<ChangeData> changes = ImmutableList.of(cd1, cd2, cd3, cd4, cd5, cd6, cd7, cd8);
+ WalkSorter sorter = new WalkSorter(repoManager);
+
+ // Do not use assertSorted because it tests all permutations. We don't need it for this test
+ // and total number of permutations is quite big.
+ assertThat(sorter.sort(changes))
+ .containsExactlyElementsIn(
+ ImmutableList.of(
+ patchSetData(cd8, c8),
+ patchSetData(cd7, c7),
+ patchSetData(cd6, c6),
+ patchSetData(cd5, c5),
+ patchSetData(cd4, c4),
+ patchSetData(cd3, c3),
+ patchSetData(cd2, c2),
+ patchSetData(cd1, c1)))
+ .inOrder();
+ }
+
+ @Test
public void subsetOfSeriesOfChanges() throws Exception {
TestRepository<Repo> p = newRepo("p");
RevCommit c1_1 = p.commit().create();
diff --git a/lib/errorprone/BUILD b/lib/errorprone/BUILD
index 456860a..f95a430 100644
--- a/lib/errorprone/BUILD
+++ b/lib/errorprone/BUILD
@@ -3,6 +3,7 @@
java_library(
name = "annotations",
data = ["//lib:LICENSE-Apache2.0"],
+ neverlink = 1,
visibility = ["//visibility:public"],
exports = ["@error-prone-annotations//jar"],
)
diff --git a/polygerrit-ui/app/constants/constants.ts b/polygerrit-ui/app/constants/constants.ts
index 24fb5c0..0fa58f4 100644
--- a/polygerrit-ui/app/constants/constants.ts
+++ b/polygerrit-ui/app/constants/constants.ts
@@ -263,6 +263,7 @@
email_strategy: EmailStrategy.ATTENTION_SET_ONLY,
default_base_for_merges: DefaultBase.AUTO_MERGE,
allow_browser_notifications: false,
+ allow_suggest_code_while_commenting: true,
diff_page_sidebar: 'NONE',
};
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 405f476..e12f93f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -107,6 +107,7 @@
import {isImageDiff} from '../../../utils/diff-util';
import {formStyles} from '../../../styles/form-styles';
import {NormalizedFileInfo} from '../../change/gr-file-list/gr-file-list';
+import {configModelToken} from '../../../models/config/config-model';
const LOADING_BLAME = 'Loading blame...';
const LOADED_BLAME = 'Blame loaded';
@@ -208,6 +209,10 @@
// Private but used in tests.
@state() isActiveChildView = false;
+ // Whether to allow the "Show Blame button"
+ @state()
+ allowBlame = false;
+
// Private but used in tests.
@state()
loggedIn = false;
@@ -261,6 +266,8 @@
private readonly getViewModel = resolve(this, changeViewModelToken);
+ private readonly getConfigModel = resolve(this, configModelToken);
+
private throttledToggleFileReviewed?: (e: KeyboardEvent) => void;
@state()
@@ -457,6 +464,13 @@
}
);
+ subscribe(
+ this,
+ () => this.getConfigModel().serverConfig$,
+ serverConfig =>
+ (this.allowBlame = serverConfig?.change.allow_blame ?? false)
+ );
+
// When user initially loads the diff view, we want to automatically mark
// the file as reviewed if they have it enabled. We can't observe these
// properties since the method will be called anytime a property updates
@@ -1054,26 +1068,9 @@
}
private renderRightControls() {
- const blameLoaderClass =
- !isMagicPath(this.path) && !isImageDiff(this.diff) ? 'show' : '';
- const blameToggleLabel =
- this.isBlameLoaded && !this.isBlameLoading ? 'Hide blame' : 'Show blame';
const diffModeSelectorClass = !this.diff || this.diff.binary ? 'hide' : '';
return html` <div class="rightControls">
- ${this.renderSidebarTriggers()}
- <span class="blameLoader ${blameLoaderClass}">
- <gr-button
- link=""
- id="toggleBlame"
- title=${this.createTitle(
- Shortcut.TOGGLE_BLAME,
- ShortcutSection.DIFFS
- )}
- ?disabled=${this.isBlameLoading}
- @click=${this.toggleBlame}
- >${blameToggleLabel}</gr-button
- >
- </span>
+ ${this.renderSidebarTriggers()} ${this.renderBlameButton()}
${when(
this.computeCanEdit(),
() => html`
@@ -1143,6 +1140,24 @@
</div>`;
}
+ private renderBlameButton() {
+ if (!this.allowBlame) return;
+ const blameLoaderClass =
+ !isMagicPath(this.path) && !isImageDiff(this.diff) ? 'show' : '';
+ const blameToggleLabel =
+ this.isBlameLoaded && !this.isBlameLoading ? 'Hide blame' : 'Show blame';
+ return html` <span class="blameLoader ${blameLoaderClass}">
+ <gr-button
+ link=""
+ id="toggleBlame"
+ title=${this.createTitle(Shortcut.TOGGLE_BLAME, ShortcutSection.DIFFS)}
+ ?disabled=${this.isBlameLoading}
+ @click=${this.toggleBlame}
+ >${blameToggleLabel}</gr-button
+ >
+ </span>`;
+ }
+
private renderDialogs() {
return html`
<gr-apply-fix-dialog id="applyFixDialog"></gr-apply-fix-dialog>
@@ -1771,12 +1786,13 @@
* Otherwise hide it.
*/
private toggleBlame() {
+ if (!this.allowBlame) return;
assertIsDefined(this.diffHost, 'diffHost');
if (this.isBlameLoaded) {
this.diffHost.clearBlame();
- return;
+ } else {
+ this.loadBlame();
}
- this.loadBlame();
}
private handleToggleHideAllCommentThreads() {
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 4d4834e..29ece75 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
@@ -71,6 +71,8 @@
import {navigationToken} from '../../core/gr-navigation/gr-navigation';
import {getDocUrl, rootUrl} from '../../../utils/url-util';
import {configModelToken} from '../../../models/config/config-model';
+import {SuggestionsProvider} from '../../../api/suggestions';
+import {pluginLoaderToken} from '../../shared/gr-js-api-interface/gr-plugin-loader';
const HTTP_AUTH = ['HTTP', 'HTTP_LDAP'];
@@ -118,6 +120,9 @@
@query('#allowBrowserNotifications')
allowBrowserNotifications?: HTMLInputElement;
+ @query('#allowSuggestCodeWhileCommenting')
+ allowSuggestCodeWhileCommenting?: HTMLInputElement;
+
@query('#disableKeyboardShortcuts')
disableKeyboardShortcuts!: HTMLInputElement;
@@ -196,6 +201,9 @@
@state() private docsBaseUrl = '';
+ @state()
+ suggestionsProvider?: SuggestionsProvider;
+
// private but used in test
public _testOnly_loadingPromise?: Promise<void>;
@@ -212,6 +220,8 @@
private readonly getConfigModel = resolve(this, configModelToken);
+ private readonly getPluginLoader = resolve(this, pluginLoaderToken);
+
constructor() {
super();
subscribe(
@@ -265,6 +275,14 @@
// we need to manually calling scrollIntoView when hash changed
document.addEventListener('location-change', this.handleLocationChange);
fireTitleChange('Settings');
+ this.getPluginLoader()
+ .awaitPluginsLoaded()
+ .then(() => {
+ const suggestionsPlugins =
+ this.getPluginLoader().pluginsModel.getState().suggestionsPlugins;
+ // We currently support results from only 1 provider.
+ this.suggestionsProvider = suggestionsPlugins?.[0]?.provider;
+ });
}
override firstUpdated() {
@@ -451,6 +469,7 @@
${this.renderTheme()} ${this.renderChangesPerPages()}
${this.renderDateTimeFormat()} ${this.renderEmailNotification()}
${this.renderEmailFormat()} ${this.renderBrowserNotifications()}
+ ${this.renderGenerateSuggestionWhenCommenting()}
${this.renderDefaultBaseForMerges()}
${this.renderRelativeDateInChangeTable()} ${this.renderDiffView()}
${this.renderShowSizeBarsInFileList()}
@@ -867,6 +886,45 @@
`;
}
+ private renderGenerateSuggestionWhenCommenting() {
+ if (
+ !this.flagsService.isEnabled(KnownExperimentId.ML_SUGGESTED_EDIT) ||
+ !this.suggestionsProvider
+ )
+ return nothing;
+ return html`
+ <section id="allowSuggestCodeWhileCommentingSection">
+ <div class="title">
+ <label for="allowSuggestCodeWhileCommenting"
+ >Allow generating suggestions while commenting</label
+ >
+ <a
+ href=${getDocUrl(
+ this.docsBaseUrl,
+ 'user-suggest-edits.html#_generate_suggestion'
+ )}
+ target="_blank"
+ rel="noopener noreferrer"
+ >
+ <gr-icon icon="help" title="read documentation"></gr-icon>
+ </a>
+ </div>
+ <span class="value">
+ <input
+ id="allowSuggestCodeWhileCommenting"
+ type="checkbox"
+ ?checked=${this.localPrefs.allow_suggest_code_while_commenting}
+ @change=${() => {
+ this.localPrefs.allow_suggest_code_while_commenting =
+ this.allowSuggestCodeWhileCommenting!.checked;
+ this.prefsChanged = true;
+ }}
+ />
+ </span>
+ </section>
+ `;
+ }
+
private renderDefaultBaseForMerges() {
if (!this.localPrefs.default_base_for_merges) return nothing;
return nothing;
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 3e341b8..9b83f3e 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.ts
@@ -377,6 +377,19 @@
this.generateSuggestEdit();
}
);
+ subscribe(
+ this,
+ () => this.getUserModel().preferences$,
+ prefs => {
+ if (
+ this.generateSuggestion !==
+ !!prefs.allow_suggest_code_while_commenting
+ ) {
+ this.generateSuggestion =
+ !!prefs.allow_suggest_code_while_commenting;
+ }
+ }
+ );
}
}
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index b66eb3c..cba3a05 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -462,6 +462,7 @@
export function createChangeConfig(): ChangeConfigInfo {
return {
+ allow_blame: true,
large_change: 500,
// The default update_delay is 5 minutes, but we don't want to accidentally
// start polling in tests
@@ -708,6 +709,7 @@
changes_per_page: 10,
email_strategy: EmailStrategy.ENABLED,
allow_browser_notifications: true,
+ allow_suggest_code_while_commenting: true,
};
}
diff --git a/polygerrit-ui/app/types/common.ts b/polygerrit-ui/app/types/common.ts
index 9992f8b..4f23dbd 100644
--- a/polygerrit-ui/app/types/common.ts
+++ b/polygerrit-ui/app/types/common.ts
@@ -1335,6 +1335,7 @@
// The email_format doesn't mentioned in doc, but exists in Java class GeneralPreferencesInfo
email_format?: EmailFormat;
allow_browser_notifications?: boolean;
+ allow_suggest_code_while_commenting?: boolean;
diff_page_sidebar?: DiffPageSidebar;
}
diff --git a/polygerrit-ui/app/utils/change-util.ts b/polygerrit-ui/app/utils/change-util.ts
index 5079abd..a78254d 100644
--- a/polygerrit-ui/app/utils/change-util.ts
+++ b/polygerrit-ui/app/utils/change-util.ts
@@ -87,10 +87,8 @@
change: ChangeInfo,
options?: ChangeStatusesOptions
): ChangeStates[] {
- const states = [];
- if (change.revert_of) {
- states.push(ChangeStates.REVERT);
- }
+ const states: ChangeStates[] = [];
+
if (change.status === ChangeStatus.MERGED) {
if (options?.revertingChangeStatus === ChangeStatus.MERGED) {
return [ChangeStates.MERGED, ChangeStates.REVERT_SUBMITTED];
@@ -103,6 +101,10 @@
if (change.status === ChangeStatus.ABANDONED) {
return [ChangeStates.ABANDONED];
}
+
+ if (change.revert_of) {
+ states.push(ChangeStates.REVERT);
+ }
if (change.mergeable === false || (options && options.mergeable === false)) {
// 'mergeable' prop may not always exist (@see Issue 6819)
states.push(ChangeStates.MERGE_CONFLICT);
@@ -116,15 +118,29 @@
states.push(ChangeStates.PRIVATE);
}
- // If there are any pre-defined statuses, only return those. Otherwise,
- // will determine the derived status.
- if (states.length || !options) {
+ // The gr-change-list table does not want READY TO SUBMIT or ACTIVE and it
+ // does not pass options.
+ if (!options) {
+ return states;
+ }
+
+ // The change is not submittable if there are conflicts or is WIP/private even
+ // if the submit requirements are ok.
+ if (
+ [
+ ChangeStates.MERGE_CONFLICT,
+ ChangeStates.GIT_CONFLICT,
+ ChangeStates.WIP,
+ ChangeStates.PRIVATE,
+ ].some(unsubmittableState => states.includes(unsubmittableState))
+ ) {
return states;
}
if (change.submittable) {
states.push(ChangeStates.READY_TO_SUBMIT);
- } else {
+ }
+ if (states.length === 0) {
states.push(ChangeStates.ACTIVE);
}
return states;
diff --git a/polygerrit-ui/app/utils/change-util_test.ts b/polygerrit-ui/app/utils/change-util_test.ts
index 6e53c16..51e935e 100644
--- a/polygerrit-ui/app/utils/change-util_test.ts
+++ b/polygerrit-ui/app/utils/change-util_test.ts
@@ -176,6 +176,18 @@
]);
});
+ test('Revert that is submittable', () => {
+ const change = {
+ ...createChange(),
+ revert_of: 123 as NumericChangeId,
+ submittable: true,
+ };
+ assert.deepEqual(changeStatuses(change, {mergeable: true}), [
+ ChangeStates.REVERT,
+ ChangeStates.READY_TO_SUBMIT,
+ ]);
+ });
+
test('Open status with private and wip', () => {
const change = {
...createChange(),
diff --git a/tools/deps.bzl b/tools/deps.bzl
index a0d8715..0a262e0 100644
--- a/tools/deps.bzl
+++ b/tools/deps.bzl
@@ -13,7 +13,7 @@
OW2_VERS = "9.2"
AUTO_COMMON_VERSION = "1.2.1"
AUTO_FACTORY_VERSION = "1.0.1"
-AUTO_VALUE_VERSION = "1.7.4"
+AUTO_VALUE_VERSION = "1.10.4"
AUTO_VALUE_GSON_VERSION = "1.3.1"
PROLOG_VERS = "1.4.4"
PROLOG_REPO = GERRIT
@@ -303,13 +303,13 @@
maven_jar(
name = "auto-value",
artifact = "com.google.auto.value:auto-value:" + AUTO_VALUE_VERSION,
- sha1 = "6b126cb218af768339e4d6e95a9b0ae41f74e73d",
+ sha1 = "90f9629eaa123f88551cc26a64bc386967ee24cc",
)
maven_jar(
name = "auto-value-annotations",
artifact = "com.google.auto.value:auto-value-annotations:" + AUTO_VALUE_VERSION,
- sha1 = "eff48ed53995db2dadf0456426cc1f8700136f86",
+ sha1 = "9679de8286eb0a151db6538ba297a8951c4a1224",
)
maven_jar(
@@ -464,8 +464,8 @@
maven_jar(
name = "httpcore",
- artifact = "org.apache.httpcomponents:httpcore:4.4.4",
- sha1 = "b31526a230871fbe285fbcbe2813f9c0839ae9b0",
+ artifact = "org.apache.httpcomponents:httpcore:4.4.16",
+ sha1 = "51cf043c87253c9f58b539c9f7e44c8894223850",
)
# Test-only dependencies below.
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index 4482751..d685fd3 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -61,8 +61,8 @@
maven_jar(
name = "log4j",
- artifact = "ch.qos.reload4j:reload4j:1.2.19",
- sha1 = "4eae9978468c5e885a6fb44df7e2bbc07a20e6ce",
+ artifact = "ch.qos.reload4j:reload4j:1.2.25",
+ sha1 = "45921e383a1001c2a599fc4c6cf59af80cdd1cf1",
)
SLF4J_VERS = "1.7.36"
