diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 662bbfed85..da3130eeaa 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -135,11 +135,10 @@ Query for open changes of watched projects: If the `n` query parameter is supplied and additional changes exist that match the query beyond the end, the last change object has a -`_more_changes: true` JSON field set. Callers can resume a query with -the `N` query parameter, supplying the last change's `_sortkey` field -as the value. When going in the reverse direction with the `P` query -parameter a `_more_changes: true` is put in the first change object if -there are results *before* the first change returned. +`_more_changes: true` JSON field set. + +The `S` or `start` query parameter can be supplied to skip a number +of changes from the list. Clients are allowed to specify more than one query by setting the `q` parameter multiple times. In this case the result is an array of @@ -3627,7 +3626,7 @@ In addition `ReviewerInfo` has the following fields: |Field Name |Description |`approvals` | The approvals of the reviewer as a map that maps the label names to the -approval values ("`-2`", "`-1`", " `0`", "`+1`", "`+2`"). +approval values ("`-2`", "`-1`", "`0`", "`+1`", "`+2`"). |========================== [[reviewer-input]] diff --git a/ReleaseNotes/ReleaseNotes-2.9.txt b/ReleaseNotes/ReleaseNotes-2.9.txt index e54f15a9bd..2a5705468d 100644 --- a/ReleaseNotes/ReleaseNotes-2.9.txt +++ b/ReleaseNotes/ReleaseNotes-2.9.txt @@ -33,6 +33,9 @@ Important Notes *WARNING:* Support for query via the SQL index is removed. The usage of a secondary index is now mandatory. +*WARNING:* The `sortkey` and `sortkey_prev` options on the query changes +REST endpoint are link:#sortkey-deprecation[deprecated]. + *WARNING:* The new change screen only displays download commands if the `download-commands` core plugin or any other plugin providing download commands is installed. The `download-commands` plugin provides the @@ -299,6 +302,22 @@ REST API ~~~~~~~~ +Changes +^^^^^^^ + + +[[sortkey-deprecation]] +* Results returned by the +link:https://gerrit-documentation.storage.googleapis.com/Documentation/2.9/rest-api-changes.html#list-changes[ +query changes] endpoint are now paginated using offsets instead of sortkeys. ++ +The `sortkey` and `sortkey_prev` parameters on the endpoint are deprecated. The +results are now paginated using the `--limit` (`-n`) option to limit the number +of results, and the `-S` option to set the start point. ++ +Queries with sortkeys are still supported against old index versions, to enable +online reindexing while clients have an older JS version. + Projects ^^^^^^^^ diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java index 388e1cf83f..bb9a40b115 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java @@ -133,13 +133,13 @@ public class Abandon implements RestModifyView, } catch (Exception e) { log.error("Cannot email update for change " + change.getChangeId(), e); } + indexFuture.checkedGet(); hooks.doChangeAbandonedHook(change, caller.getAccount(), db.patchSets().get(change.currentPatchSetId()), Strings.emptyToNull(input.message), db); ChangeInfo result = json.format(change); - indexFuture.checkedGet(); return result; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index be78b06740..d0c2b98107 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -200,12 +200,6 @@ public class ChangeInserter { if(!messageIsForChange()) { commitMessageNotForChange(); } - gitRefUpdated.fire(change.getProject(), patchSet.getRefName(), - ObjectId.zeroId(), commit); - - if (runHooks) { - hooks.doPatchsetCreatedHook(change, patchSet, db); - } if (sendMail) { try { @@ -221,6 +215,14 @@ public class ChangeInserter { } } f.checkedGet(); + + gitRefUpdated.fire(change.getProject(), patchSet.getRefName(), + ObjectId.zeroId(), commit); + + if (runHooks) { + hooks.doPatchsetCreatedHook(change, patchSet, db); + } + return change; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java index 21f5a690f4..ab9244c43d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java @@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.collect.SetMultimap; -import com.google.common.util.concurrent.CheckedFuture; import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; @@ -318,14 +317,13 @@ public class PatchSetInserter { } finally { db.rollback(); } - CheckedFuture f = mergeabilityChecker.newCheck() + mergeabilityChecker.newCheck() .addChange(updatedChange) .reindex() - .runAsync(); + .run(); if (runHooks) { hooks.doPatchsetCreatedHook(updatedChange, patchSet, db); } - f.checkedGet(); return updatedChange; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java index f4d008270d..610e5df72d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java @@ -173,22 +173,22 @@ public class PostReview implements RestModifyView } else { indexWrite = Futures. immediateCheckedFuture(null); } - if (message != null) { - if (input.notify.compareTo(NotifyHandling.NONE) > 0) { - email.create( - input.notify, - change, - revision.getPatchSet(), - revision.getAccountId(), - message, - comments).sendAsync(); - } - fireCommentAddedHook(revision); + if (message != null && input.notify.compareTo(NotifyHandling.NONE) > 0) { + email.create( + input.notify, + change, + revision.getPatchSet(), + revision.getAccountId(), + message, + comments).sendAsync(); } Output output = new Output(); output.labels = input.labels; indexWrite.checkedGet(); + if (message != null) { + fireCommentAddedHook(revision); + } return output; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java index 92e8af388e..48b8855899 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java @@ -249,24 +249,23 @@ public class PostReviewers implements RestModifyView added) + private void emailReviewers(Change change, List added) throws OrmException, EmailException { if (added.isEmpty()) { return; } - // Execute hook for added reviewers - // - PatchSet patchSet = dbProvider.get().patchSets().get(change.currentPatchSetId()); - for (PatchSetApproval psa : added) { - Account account = accountCache.get(psa.getAccountId()).getAccount(); - hooks.doReviewerAddedHook(change, account, patchSet, dbProvider.get()); - } - // Email the reviewers // // The user knows they added themselves, don't bother emailing them. diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java index cd8b89fc1e..88da0949f6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Publish.java @@ -96,12 +96,13 @@ public class Publish implements RestModifyView, || updatedChange.getStatus() == Change.Status.NEW) { CheckedFuture indexFuture = indexer.indexAsync(updatedChange.getId()); - hooks.doDraftPublishedHook(updatedChange, updatedPatchSet, dbProvider.get()); sender.send(rsrc.getNotes(), update, rsrc.getChange().getStatus() == Change.Status.DRAFT, rsrc.getUser(), updatedChange, updatedPatchSet, rsrc.getControl().getLabelTypes()); indexFuture.checkedGet(); + hooks.doDraftPublishedHook(updatedChange, updatedPatchSet, + dbProvider.get()); } } catch (PatchSetInfoNotAvailableException e) { throw new ResourceNotFoundException(e.getMessage()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java index 3e014444b8..47bb485dd6 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java @@ -15,7 +15,6 @@ package com.google.gerrit.server.change; import com.google.common.base.Strings; -import com.google.common.util.concurrent.CheckedFuture; import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.DefaultInput; @@ -123,11 +122,9 @@ public class PutTopic implements RestModifyView, db.rollback(); } update.commit(); - CheckedFuture indexFuture = - indexer.indexAsync(change.getId()); + indexer.index(db, change); hooks.doTopicChangedHook(change, currentUser.getAccount(), oldTopicName, db); - indexFuture.checkedGet(); } return Strings.isNullOrEmpty(newTopicName) ? Response.none() diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java index 7a31ebec86..88e2137371 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java @@ -134,13 +134,13 @@ public class Restore implements RestModifyView, } catch (Exception e) { log.error("Cannot email update for change " + change.getChangeId(), e); } + f.checkedGet(); hooks.doChangeRestoredHook(change, caller.getAccount(), db.patchSets().get(change.currentPatchSetId()), Strings.emptyToNull(input.message), dbProvider.get()); ChangeInfo result = json.format(change); - f.checkedGet(); return result; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java index c49ed30c8c..1b2a023d2d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java @@ -859,6 +859,7 @@ public class MergeOp { db.commit(); sendMergedEmail(c, submitter); + indexer.index(db, c); if (submitter != null) { try { hooks.doChangeMergedHook(c, @@ -1085,6 +1086,14 @@ public class MergeOp { } })); + if (indexFuture != null) { + try { + indexFuture.checkedGet(); + } catch (IOException e) { + log.error("Failed to index new change message", e); + } + } + if (submitter != null) { try { hooks.doMergeFailedHook(c, @@ -1094,13 +1103,6 @@ public class MergeOp { log.error("Cannot run hook for merge failed " + c.getId(), ex); } } - if (indexFuture != null) { - try { - indexFuture.checkedGet(); - } catch (IOException e) { - log.error("Failed to index new change message", e); - } - } } private void abandonAllOpenChanges() throws NoSuchChangeException { @@ -1166,4 +1168,4 @@ public class MergeOp { } update.commit(); } -} \ No newline at end of file +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 548d89f786..938c9e6234 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -2100,13 +2100,6 @@ public class ReceiveCommits { .addChange(change) .reindex() .runAsync(); - gitRefUpdated.fire(project.getNameKey(), newPatchSet.getRefName(), - ObjectId.zeroId(), newCommit); - hooks.doPatchsetCreatedHook(change, newPatchSet, db); - if (mergedIntoRef != null) { - hooks.doChangeMergedHook( - change, currentUser.getAccount(), newPatchSet, db); - } workQueue.getDefaultQueue() .submit(requestScopePropagator.wrap(new Runnable() { @Override @@ -2135,6 +2128,14 @@ public class ReceiveCommits { })); f.checkedGet(); + gitRefUpdated.fire(project.getNameKey(), newPatchSet.getRefName(), + ObjectId.zeroId(), newCommit); + hooks.doPatchsetCreatedHook(change, newPatchSet, db); + if (mergedIntoRef != null) { + hooks.doChangeMergedHook( + change, currentUser.getAccount(), newPatchSet, db); + } + if (magicBranch != null && magicBranch.isSubmit()) { submit(changeCtl, newPatchSet); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java index 30702c62c5..7bc457acdd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java @@ -114,7 +114,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook { TagMatcher tags = tagCache.get(projectName).matcher( tagCache, db, - filterTagsSeperately ? filter(refs).values() : result.values()); + filterTagsSeperately ? filter(db.getAllRefs()).values() : result.values()); for (Ref tag : deferredTags) { if (tags.isReachable(tag)) { result.put(tag.getName(), tag); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java index 91a0ec5551..09986021e5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java @@ -90,10 +90,10 @@ public class ListGroups implements RestReadView { @Option(name = "--limit", aliases = {"-n"}, metaVar = "CNT", usage = "maximum number of groups to list") private int limit; - @Option(name = "-S", metaVar = "CNT", usage = "number of groups to skip") + @Option(name = "--start", aliases = {"-S"}, metaVar = "CNT", usage = "number of groups to skip") private int start; - @Option(name = "-m", metaVar = "MATCH", usage = "match group substring") + @Option(name = "--match", aliases = {"-m"}, metaVar = "MATCH", usage = "match group substring") private String matchSubstring; @Option(name = "-o", usage = "Output options per group") diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/AbstractPreloadedPluginScanner.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/AbstractPreloadedPluginScanner.java index 727a43c01c..e23965426d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/AbstractPreloadedPluginScanner.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/AbstractPreloadedPluginScanner.java @@ -70,9 +70,9 @@ public abstract class AbstractPreloadedPluginScanner implements + "Implementation-Version: " + pluginVersion + "\n" + "Gerrit-ReloadMode: restart\n" + "Gerrit-ApiType: " + apiType + "\n"); - appendIfNotNull(manifestString, "Gerrit-Module: ", sshModuleClass); + appendIfNotNull(manifestString, "Gerrit-SshModule: ", sshModuleClass); appendIfNotNull(manifestString, "Gerrit-HttpModule: ", httpModuleClass); - appendIfNotNull(manifestString, "Gerrit-SshModule: ", sysModuleClass); + appendIfNotNull(manifestString, "Gerrit-Module: ", sysModuleClass); return new Manifest(new ByteArrayInputStream(manifestString.toString() .getBytes())); } @@ -115,8 +115,14 @@ public abstract class AbstractPreloadedPluginScanner implements Class.forName("com.google.inject.servlet.ServletModule"); Class sshModuleBaseClass = Class.forName("com.google.gerrit.sshd.CommandModule"); + sshModuleClass = null; + httpModuleClass = null; + sysModuleClass = null; for (Class clazz : classes) { + if (clazz.isLocalClass()) { + continue; + } if (sshModuleBaseClass.isAssignableFrom(clazz)) { sshModuleClass = diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java index 8b895f22d5..ad9636ea69 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ListProjects.java @@ -151,17 +151,17 @@ public class ListProjects implements RestReadView { this.limit = limit; } - @Option(name = "-S", metaVar = "CNT", usage = "number of projects to skip") + @Option(name = "--start", aliases = {"-S"}, metaVar = "CNT", usage = "number of projects to skip") public void setStart(int start) { this.start = start; } - @Option(name = "-p", metaVar = "PREFIX", usage = "match project prefix") + @Option(name = "--prefix", aliases = {"-p"}, metaVar = "PREFIX", usage = "match project prefix") public void setMatchPrefix(String matchPrefix) { this.matchPrefix = matchPrefix; } - @Option(name = "-m", metaVar = "MATCH", usage = "match project substring") + @Option(name = "--match", aliases = {"-m"}, metaVar = "MATCH", usage = "match project substring") public void setMatchSubstring(String matchSubstring) { this.matchSubstring = matchSubstring; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java index fb55b49527..4808b47f3d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryChanges.java @@ -80,7 +80,7 @@ public class QueryChanges implements RestReadView { imp.setSortkeyBefore(key); } - @Option(name = "-S", metaVar = "CNT", usage = "Number of changes to skip") + @Option(name = "--start", aliases = {"-S"}, metaVar = "CNT", usage = "Number of changes to skip") public void setStart(int start) { imp.setStart(start); } diff --git a/tools/pack_war.py b/tools/pack_war.py index ee87b510cf..ba39856ebf 100755 --- a/tools/pack_war.py +++ b/tools/pack_war.py @@ -35,7 +35,11 @@ def link_jars(libs, directory): makedirs(directory) while not path.isfile('.buckconfig'): chdir('..') - cp = check_output(['buck', 'audit', 'classpath'] + libs) + try: + cp = check_output(['buck', 'audit', 'classpath'] + libs) + except Exception as e: + print('call to buck audit failed: %s' % e, file=sys.stderr) + exit(1) for j in cp.strip().splitlines(): if j not in jars: jars.add(j) diff --git a/tools/plugin_archetype_deploy.sh b/tools/plugin_archetype_deploy.sh index 929e1fd59d..b5d1a6658a 100755 --- a/tools/plugin_archetype_deploy.sh +++ b/tools/plugin_archetype_deploy.sh @@ -13,7 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -help() +function help { cat <<'eof' Usage: plugin_archetype_deploy [option] @@ -21,8 +21,9 @@ Usage: plugin_archetype_deploy [option] Deploys Gerrit plugin Maven archetypes to Maven Central Valid options: - --help show this message - --dry-run don't execute commands, just print them + --help show this message. + --dry-run don't execute commands, just print them. + eof exit } @@ -62,22 +63,39 @@ function build_and_deploy -Dfile=target/$module-$ver.jar } -while [ $# -gt 0 ]; do - test "$1" == --dry-run && dryRun=true - test "$1" == --help && help - shift -done +function confirm +{ + read -n1 -p "Are you sure you want to deploy? [N/y]: " ready + if [[ ! $ready == [Yy] ]]; then + if [[ $ready == [Nn] || -z $ready ]]; then + echo; exit + else + echo; confirm + fi + fi +} -root=$(instroot) -cd "$root" -ver=$(getver GERRIT_VERSION) -[[ $ver == *-SNAPSHOT ]] \ - && url="https://oss.sonatype.org/content/repositories/snapshots" \ - || url="https://oss.sonatype.org/service/local/staging/deploy/maven2" +function run +{ + test ${dryRun:-'false'} == 'false' && confirm + root=$(instroot) + cd "$root" + ver=$(getver GERRIT_VERSION) + [[ $ver == *-SNAPSHOT ]] \ + && url="https://oss.sonatype.org/content/repositories/snapshots" \ + || url="https://oss.sonatype.org/service/local/staging/deploy/maven2" -for d in gerrit-plugin-archetype \ - gerrit-plugin-js-archetype \ - gerrit-plugin-gwt-archetype ; do - (cd "$d"; build_and_deploy) -done + for d in gerrit-plugin-archetype \ + gerrit-plugin-js-archetype \ + gerrit-plugin-gwt-archetype ; do + (cd "$d"; build_and_deploy) + done +} +if [ "$1" == "--dry-run" ]; then + dryRun=true && run +elif [ -z "$1" ]; then + run +else + help +fi