From c14217757fcc95091ae4a23dabed709bf9ae2ce8 Mon Sep 17 00:00:00 2001 From: Youssef Elghareeb Date: Fri, 17 Jan 2020 15:34:14 +0100 Subject: [PATCH 1/6] Add a pgm to delete dead draft comments refs This cleanup is needed after we fixed a bug related to the deletion of draft comments refs (e.g. after being published or deleted). More context in I6f7b65828b. This bug caused some draft comments refs to remain in the git "All-Users" repo. These refs point to an empty tree which has a unique SHA1 in git. This also kept the history of commits pointed to by these refs through the parent pointers because updates to these refs were made using fast-forward transactions. The empty tree SHA1 was used to detect and delete these zombie refs. Change-Id: I6aaf4ac292aca2da52325dffd9dbce74c0510390 --- .../google/gerrit/pgm/DeleteZombieDrafts.java | 79 ++++++ .../notedb/DeleteZombieCommentsRefs.java | 130 +++++++++ .../git/DeleteZombieCommentsRefsTest.java | 247 ++++++++++++++++++ 3 files changed, 456 insertions(+) create mode 100644 java/com/google/gerrit/pgm/DeleteZombieDrafts.java create mode 100644 java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java create mode 100644 javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java diff --git a/java/com/google/gerrit/pgm/DeleteZombieDrafts.java b/java/com/google/gerrit/pgm/DeleteZombieDrafts.java new file mode 100644 index 0000000000..82c260674a --- /dev/null +++ b/java/com/google/gerrit/pgm/DeleteZombieDrafts.java @@ -0,0 +1,79 @@ +// Copyright (C) 2020 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.pgm; + +import com.google.gerrit.pgm.init.api.ConsoleUI; +import com.google.gerrit.pgm.util.SiteProgram; +import com.google.gerrit.server.config.GerritServerConfigModule; +import com.google.gerrit.server.config.SitePath; +import com.google.gerrit.server.notedb.DeleteZombieCommentsRefs; +import com.google.gerrit.server.notedb.DeleteZombieCommentsRefs.Factory; +import com.google.gerrit.server.schema.SchemaModule; +import com.google.gerrit.server.securestore.SecureStoreClassName; +import com.google.gwtorm.server.OrmException; +import com.google.inject.AbstractModule; +import com.google.inject.Guice; +import com.google.inject.Injector; +import com.google.inject.Module; +import com.google.inject.assistedinject.FactoryModuleBuilder; +import com.google.inject.util.Providers; +import java.io.IOException; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import org.kohsuke.args4j.Option; + +/** + * A pgm which can be used to clean zombie draft comments refs More context in + * https://gerrit-review.googlesource.com/c/gerrit/+/246233 + * + *

The implementation is in {@link DeleteZombieCommentsRefs} + */ +public class DeleteZombieDrafts extends SiteProgram { + @Option( + name = "--cleanup-percentage", + aliases = {"-c"}, + usage = "Clean a % of zombie drafts (default is 100%)") + private Integer cleanupPercentage = 100; + + @Override + public int run() throws IOException, OrmException { + mustHaveValidSite(); + Injector sysInjector = getSysInjector(); + DeleteZombieCommentsRefs cleanup = + sysInjector.getInstance(Factory.class).create(cleanupPercentage); + cleanup.execute(); + return 0; + } + + private Injector getSysInjector() { + List modules = new ArrayList<>(); + modules.add( + new AbstractModule() { + @Override + protected void configure() { + bind(Path.class).annotatedWith(SitePath.class).toInstance(getSitePath()); + bind(ConsoleUI.class).toInstance(ConsoleUI.getInstance(false)); + bind(String.class) + .annotatedWith(SecureStoreClassName.class) + .toProvider(Providers.of(getConfiguredSecureStoreClass())); + install(new FactoryModuleBuilder().build(DeleteZombieCommentsRefs.Factory.class)); + } + }); + modules.add(new GerritServerConfigModule()); + modules.add(new SchemaModule()); + return Guice.createInjector(modules); + } +} diff --git a/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java b/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java new file mode 100644 index 0000000000..eabc67b06b --- /dev/null +++ b/java/com/google/gerrit/server/notedb/DeleteZombieCommentsRefs.java @@ -0,0 +1,130 @@ +// Copyright (C) 2020 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.notedb; + +import static com.google.common.collect.ImmutableList.toImmutableList; + +import com.google.common.collect.Iterables; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.server.config.AllUsersName; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.update.RefUpdateUtil; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import org.eclipse.jgit.lib.BatchRefUpdate; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.transport.ReceiveCommand; + +/** + * This class can be used to clean zombie draft comments refs. More context in + * https://gerrit-review.googlesource.com/c/gerrit/+/246233 + * + *

An earlier bug in the deletion of draft comments {@code + * refs/draft-comments/$change_id_short/$change_id/$user_id} caused some draft refs to remain in Git + * and not get deleted. These refs point to an empty tree. + */ +public class DeleteZombieCommentsRefs { + private final String EMPTY_TREE_ID = "4b825dc642cb6eb9a060e54bf8d69288fbee4904"; + private final String DRAFT_REFS_PREFIX = "refs/draft-comments"; + private final int CHUNK_SIZE = 100; // log progress after deleting every CHUNK_SIZE refs + private final GitRepositoryManager repoManager; + private final AllUsersName allUsers; + private final int cleanupPercentage; + private Repository allUsersRepo; + + public interface Factory { + DeleteZombieCommentsRefs create(int cleanupPercentage); + } + + @Inject + public DeleteZombieCommentsRefs( + AllUsersName allUsers, + GitRepositoryManager repoManager, + @Assisted Integer cleanupPercentage) { + this.allUsers = allUsers; + this.repoManager = repoManager; + this.cleanupPercentage = (cleanupPercentage == null) ? 100 : cleanupPercentage; + } + + public void execute() throws IOException { + allUsersRepo = repoManager.openRepository(allUsers); + + List draftRefs = allUsersRepo.getRefDatabase().getRefsByPrefix(DRAFT_REFS_PREFIX); + List zombieRefs = filterZombieRefs(draftRefs); + + System.out.println( + String.format( + "Found a total of %d zombie draft refs in %s repo.", + zombieRefs.size(), allUsers.get())); + + System.out.println(String.format("Cleanup percentage = %d", cleanupPercentage)); + zombieRefs = + zombieRefs.stream() + .filter(ref -> Change.Id.fromAllUsersRef(ref.getName()).get() % 100 < cleanupPercentage) + .collect(toImmutableList()); + System.out.println( + String.format("Number of zombie refs to be cleaned = %d", zombieRefs.size())); + + long zombieRefsCnt = zombieRefs.size(); + long deletedRefsCnt = 0; + long startTime = System.currentTimeMillis(); + + for (List refsBatch : Iterables.partition(zombieRefs, CHUNK_SIZE)) { + deleteBatchZombieRefs(refsBatch); + long elapsed = (System.currentTimeMillis() - startTime) / 1000; + deletedRefsCnt += refsBatch.size(); + logProgress(deletedRefsCnt, zombieRefsCnt, elapsed); + } + } + + private void deleteBatchZombieRefs(List refsBatch) throws IOException { + List deleteCommands = + refsBatch.stream() + .map( + zombieRef -> + new ReceiveCommand( + zombieRef.getObjectId(), ObjectId.zeroId(), zombieRef.getName())) + .collect(toImmutableList()); + BatchRefUpdate bru = allUsersRepo.getRefDatabase().newBatchUpdate(); + bru.setAtomic(true); + bru.addCommand(deleteCommands); + RefUpdateUtil.executeChecked(bru, allUsersRepo); + } + + private List filterZombieRefs(List allDraftRefs) throws IOException { + List zombieRefs = new ArrayList<>((int) (allDraftRefs.size() * 0.5)); + for (Ref ref : allDraftRefs) { + if (isZombieRef(ref)) { + zombieRefs.add(ref); + } + } + return zombieRefs; + } + + private boolean isZombieRef(Ref ref) throws IOException { + return allUsersRepo.parseCommit(ref.getObjectId()).getTree().getName().equals(EMPTY_TREE_ID); + } + + private void logProgress(long deletedRefsCount, long allRefsCount, long elapsed) { + System.out.format( + "Deleted %d/%d zombie draft refs (%d seconds)\n", deletedRefsCount, allRefsCount, elapsed); + } +} diff --git a/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java b/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java new file mode 100644 index 0000000000..6d0556f7b6 --- /dev/null +++ b/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java @@ -0,0 +1,247 @@ +// Copyright (C) 2020 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.git; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.reviewdb.client.RefNames; +import com.google.gerrit.server.config.AllUsersName; +import com.google.gerrit.server.notedb.DeleteZombieCommentsRefs; +import com.google.gerrit.server.update.RefUpdateUtil; +import com.google.gerrit.server.util.time.TimeUtil; +import com.google.gerrit.testing.InMemoryRepositoryManager; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import org.eclipse.jgit.lib.BatchRefUpdate; +import org.eclipse.jgit.lib.CommitBuilder; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectInserter; +import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.RefUpdate; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.lib.TreeFormatter; +import org.eclipse.jgit.revwalk.RevBlob; +import org.eclipse.jgit.revwalk.RevWalk; +import org.eclipse.jgit.transport.ReceiveCommand; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class DeleteZombieCommentsRefsTest { + private InMemoryRepositoryManager repoManager = new InMemoryRepositoryManager(); + private Project.NameKey allUsersProject = new Project.NameKey("All-Users"); + + @Test + public void cleanZombieDraftsSmall() throws Exception { + try (Repository usersRepo = repoManager.createRepository(allUsersProject)) { + Ref ref1 = createRefWithNonEmptyTreeCommit(usersRepo, 1, 1000001); + Ref ref2 = createRefWithEmptyTreeCommit(usersRepo, 1, 1000002); + + DeleteZombieCommentsRefs clean = + new DeleteZombieCommentsRefs(new AllUsersName("All-Users"), repoManager, null); + clean.execute(); + + /* Check that ref1 still exists, and ref2 is deleted */ + assertThat(usersRepo.exactRef(ref1.getName())).isNotNull(); + assertThat(usersRepo.exactRef(ref2.getName())).isNull(); + } + } + + @Test + public void cleanZombieDraftsWithPercentage() throws Exception { + try (Repository usersRepo = repoManager.createRepository(allUsersProject)) { + Ref ref1 = createRefWithNonEmptyTreeCommit(usersRepo, 1005, 1000001); + Ref ref2 = createRefWithEmptyTreeCommit(usersRepo, 1006, 1000002); + Ref ref3 = createRefWithEmptyTreeCommit(usersRepo, 1060, 1000002); + + assertThat(usersRepo.getRefDatabase().getRefs()).hasSize(3); + + int cleanupPercentage = 50; + DeleteZombieCommentsRefs clean = + new DeleteZombieCommentsRefs( + new AllUsersName("All-Users"), repoManager, cleanupPercentage); + clean.execute(); + + /* ref1 not deleted, ref2 deleted, ref3 not deleted because of the clean percentage */ + assertThat(usersRepo.getRefDatabase().getRefs()).hasSize(2); + assertThat(usersRepo.exactRef(ref1.getName())).isNotNull(); + assertThat(usersRepo.exactRef(ref2.getName())).isNull(); + assertThat(usersRepo.exactRef(ref3.getName())).isNotNull(); + + /* Re-execute the cleanup and make sure nothing's changed */ + clean.execute(); + assertThat(usersRepo.getRefDatabase().getRefs()).hasSize(2); + assertThat(usersRepo.exactRef(ref1.getName())).isNotNull(); + assertThat(usersRepo.exactRef(ref2.getName())).isNull(); + assertThat(usersRepo.exactRef(ref3.getName())).isNotNull(); + + /* Increase the cleanup percentage */ + cleanupPercentage = 70; + clean = + new DeleteZombieCommentsRefs( + new AllUsersName("All-Users"), repoManager, cleanupPercentage); + + clean.execute(); + + /* Now ref3 is deleted */ + assertThat(usersRepo.getRefDatabase().getRefs()).hasSize(1); + assertThat(usersRepo.exactRef(ref1.getName())).isNotNull(); + assertThat(usersRepo.exactRef(ref2.getName())).isNull(); + assertThat(usersRepo.exactRef(ref3.getName())).isNull(); + } + } + + @Test + public void cleanZombieDraftsLarge() throws Exception { + try (Repository usersRepo = repoManager.createRepository(allUsersProject)) { + int goodRefsCnt = 5000; + int zombieRefsCnt = 5000; + int userIdGoodRefs = 1000001; + int userIdBadRefs = 1000002; + + Ref nonEmptyBaseRef = createRefWithNonEmptyTreeCommit(usersRepo, 1, userIdGoodRefs); + Ref emptyBaseRef = createRefWithEmptyTreeCommit(usersRepo, 1, userIdBadRefs); + + List goodRefs = + createNRefsOnCommit( + usersRepo, nonEmptyBaseRef.getObjectId(), goodRefsCnt, userIdGoodRefs); + List badRefs = + createNRefsOnCommit(usersRepo, emptyBaseRef.getObjectId(), zombieRefsCnt, userIdBadRefs); + + goodRefs.add(0, nonEmptyBaseRef.getName()); + badRefs.add(0, emptyBaseRef.getName()); + + assertThat(usersRepo.getRefDatabase().getRefs().size()) + .isEqualTo(goodRefs.size() + badRefs.size()); + + DeleteZombieCommentsRefs clean = + new DeleteZombieCommentsRefs(new AllUsersName("All-Users"), repoManager, null); + clean.execute(); + + assertThat( + usersRepo.getRefDatabase().getRefs().stream() + .map(Ref::getName) + .collect(toImmutableList())) + .containsExactlyElementsIn(goodRefs); + + assertThat( + usersRepo.getRefDatabase().getRefs().stream() + .map(Ref::getName) + .collect(toImmutableList())) + .containsNoneIn(badRefs); + } + } + + private static List createNRefsOnCommit( + Repository usersRepo, ObjectId commitId, int n, int uuid) throws IOException { + List refNames = new ArrayList<>(); + BatchRefUpdate bru = usersRepo.getRefDatabase().newBatchUpdate(); + bru.setAtomic(true); + for (int i = 2; i <= n + 1; i++) { + String refName = getRefName(i, uuid); + bru.addCommand( + new ReceiveCommand(ObjectId.zeroId(), commitId, refName, ReceiveCommand.Type.CREATE)); + refNames.add(refName); + } + RefUpdateUtil.executeChecked(bru, usersRepo); + return refNames; + } + + private static String getRefName(int changeId, int userId) { + Change.Id cId = new Change.Id(changeId); + Account.Id aId = new Account.Id(userId); + return RefNames.refsDraftComments(cId, aId); + } + + private static Ref createRefWithNonEmptyTreeCommit(Repository usersRepo, int changeId, int userId) + throws IOException { + RevWalk rw = new RevWalk(usersRepo); + ObjectId fileObj = createBlob(usersRepo, String.format("file %d content", changeId)); + ObjectId treeObj = + createTree(usersRepo, rw.lookupBlob(fileObj), String.format("file%d.txt", changeId)); + ObjectId commitObj = createCommit(usersRepo, treeObj, null); + Ref refObj = createRef(usersRepo, commitObj, getRefName(changeId, userId)); + return refObj; + } + + private static Ref createRefWithEmptyTreeCommit(Repository usersRepo, int changeId, int userId) + throws IOException { + ObjectId treeEmpty = createTree(usersRepo, null, ""); + ObjectId commitObj = createCommit(usersRepo, treeEmpty, null); + Ref refObj = createRef(usersRepo, commitObj, getRefName(changeId, userId)); + return refObj; + } + + private static Ref createRef(Repository repo, ObjectId commitId, String refName) + throws IOException { + RefUpdate update = repo.updateRef(refName); + update.setNewObjectId(commitId); + update.setForceUpdate(true); + update.update(); + return repo.exactRef(refName); + } + + private static ObjectId createCommit(Repository repo, ObjectId treeId, ObjectId parentCommit) + throws IOException { + try (ObjectInserter oi = repo.newObjectInserter()) { + PersonIdent committer = + new PersonIdent(new PersonIdent("Foo Bar", "foo.bar@baz.com"), TimeUtil.nowTs()); + CommitBuilder cb = new CommitBuilder(); + cb.setTreeId(treeId); + cb.setCommitter(committer); + cb.setAuthor(committer); + cb.setMessage("Test commit"); + if (parentCommit != null) { + cb.setParentIds(parentCommit); + } + ObjectId commitId = oi.insert(cb); + oi.flush(); + oi.close(); + return commitId; + } + } + + private static ObjectId createTree(Repository repo, RevBlob blob, String blobName) + throws IOException { + try (ObjectInserter oi = repo.newObjectInserter()) { + TreeFormatter formatter = new TreeFormatter(); + if (blob != null) { + formatter.append(blobName, blob); + } + ObjectId treeId = oi.insert(formatter); + oi.flush(); + oi.close(); + return treeId; + } + } + + private static ObjectId createBlob(Repository repo, String content) throws IOException { + try (ObjectInserter oi = repo.newObjectInserter()) { + ObjectId blobId = oi.insert(Constants.OBJ_BLOB, content.getBytes(UTF_8)); + oi.flush(); + oi.close(); + return blobId; + } + } +} From 9bede91ce3e4546dc09c89363d3a8d70d1fad021 Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Mon, 27 Jan 2020 17:38:40 -0500 Subject: [PATCH 2/6] Documentation: Recommend Bazelisk to launch bazel Change-Id: Ic9dfd80fe7eadd1c52f3c0cac0d78e61654dd34c --- Documentation/dev-bazel.txt | 14 ++++++++++++-- Documentation/dev-readme.txt | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Documentation/dev-bazel.txt b/Documentation/dev-bazel.txt index b62a0a4d62..9d1f6a9512 100644 --- a/Documentation/dev-bazel.txt +++ b/Documentation/dev-bazel.txt @@ -9,12 +9,22 @@ To build Gerrit from source, you need: * A JDK for Java 8|9|10|11|... * Python 2 or 3 * Node.js -* link:https://www.bazel.io/versions/master/docs/install.html[Bazel] directly -or through link:https://github.com/bazelbuild/bazelisk[Bazelisk] +* link:https://bazel.build/[Bazel] -launched with +link:https://github.com/bazelbuild/bazelisk[Bazelisk] * Maven * zip, unzip * gcc +[[bazel]] +=== Bazel + +link:https://github.com/bazelbuild/bazelisk[Bazelisk] includes a +link:https://bazel.build/[Bazel] version check and downloads the correct +`bazel` version for the git project/repository. Bazelisk is the recommended +`bazel` launcher for Gerrit. Once Bazelisk is installed locally, a `bazel` +symlink can be created towards it. This is so that every `bazel` command +seamlessly uses Bazelisk, which then runs the proper `bazel` binary version. + [[java]] === Java diff --git a/Documentation/dev-readme.txt b/Documentation/dev-readme.txt index 92d080b0c7..fa0c184b40 100644 --- a/Documentation/dev-readme.txt +++ b/Documentation/dev-readme.txt @@ -1,7 +1,7 @@ = Gerrit Code Review: Developer Setup To build a developer instance, you'll need link:https://bazel.build/[Bazel] to -compile the code. +compile the code, preferably launched with link:https://github.com/bazelbuild/bazelisk[Bazelisk]. == Getting the Source From dbea43209b8449dee07c84b50831a6cab52af41c Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Fri, 24 Jan 2020 08:34:50 +0100 Subject: [PATCH 3/6] Bazel: Remove bazel version check Starting from Bazel 2.0.0 the version check is part of the Bazel's shell script wrapper: Bazel's Debian package and the binary installer now include an improved wrapper that understands `/.bazelversion` files and the `$USE_BAZEL_VERSION` environment variable. This is similar to what Bazelisk offers (https://github.com/bazelbuild/bazelisk#how-does-bazelisk-know-\ whch-bazel-version-to-run-and-where-to-get-it-from), except that it works offline and integrates with apt-get. Given that all supported Gerrit versions have been upgraded to Bazel 2.0.0, we can remove our own implementation of the version check. If a different Bazel version is used, bazel shell script would refuse to run and issue this error message: ERROR: The project you're trying to build requires Bazel 2.0.0 (specified in /home//projects/gerrit2/.bazelversion), but it wasn't found in /home//.bazel/bin. As a side effect of this change, we don't need to explicitly load now bazel_skylib library. But given that rules_closure internally depends on it, don't pass omit_bazel_skylib in rules_closure_dependencies() method. Change-Id: I3469145375aa78ed7f173f7c66acf1ddb7f7b745 --- WORKSPACE | 17 ----------------- tools/bzl/bazelisk_version.bzl | 16 ---------------- 2 files changed, 33 deletions(-) delete mode 100644 tools/bzl/bazelisk_version.bzl diff --git a/WORKSPACE b/WORKSPACE index 6495faac5a..0b40765515 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -23,13 +23,6 @@ load("@bazel_toolchains//rules:rbe_repo.bzl", "rbe_autoconfig") # otherwise refer to RBE docs. rbe_autoconfig(name = "rbe_default") -http_archive( - name = "bazel_skylib", - sha256 = "2ea8a5ed2b448baf4a6855d3ce049c4c452a6470b1efd1504fdb7c1c134d220a", - strip_prefix = "bazel-skylib-0.8.0", - urls = ["https://github.com/bazelbuild/bazel-skylib/archive/0.8.0.tar.gz"], -) - http_archive( name = "io_bazel_rules_closure", sha256 = "03c3b16f205085817fd89cfdcb2220a0138647ee7992be9cef291b069dd90301", @@ -47,15 +40,6 @@ http_file( urls = ["https://raw.githubusercontent.com/google/closure-compiler/35d2b3340ff23a69441f10fa3bc820691c2942f2/contrib/externs/polymer-1.0.js"], ) -# Check Bazel version when invoked by Bazel directly -load("//tools/bzl:bazelisk_version.bzl", "bazelisk_version") - -bazelisk_version(name = "bazelisk_version") - -load("@bazelisk_version//:check.bzl", "check_bazel_version") - -check_bazel_version() - load("@io_bazel_rules_closure//closure:repositories.bzl", "rules_closure_dependencies", "rules_closure_toolchains") # Prevent redundant loading of dependencies. @@ -64,7 +48,6 @@ load("@io_bazel_rules_closure//closure:repositories.bzl", "rules_closure_depende # https://github.com/google/closure-templates/pull/155 rules_closure_dependencies( omit_aopalliance = True, - omit_bazel_skylib = True, omit_javax_inject = True, omit_rules_cc = True, ) diff --git a/tools/bzl/bazelisk_version.bzl b/tools/bzl/bazelisk_version.bzl deleted file mode 100644 index d8b3d10982..0000000000 --- a/tools/bzl/bazelisk_version.bzl +++ /dev/null @@ -1,16 +0,0 @@ -_template = """ -load("@bazel_skylib//lib:versions.bzl", "versions") - -def check_bazel_version(): - versions.check(minimum_bazel_version = "{version}") -""".strip() - -def _impl(repository_ctx): - repository_ctx.symlink(Label("@//:.bazelversion"), ".bazelversion") - bazelversion = repository_ctx.read(".bazelversion").strip() - - repository_ctx.file("BUILD", executable = False) - - repository_ctx.file("check.bzl", executable = False, content = _template.format(version = bazelversion)) - -bazelisk_version = repository_rule(implementation = _impl) From db4a2c4841aa527adcd8c6a52cdf57d1e3ce277d Mon Sep 17 00:00:00 2001 From: Marco Miller Date: Mon, 27 Jan 2020 17:59:08 -0500 Subject: [PATCH 4/6] Documentation: Mention unused_deps with buildifier Change-Id: I9d0a0837e4ab3c8b6d7670d2495e711885a69e09 --- Documentation/dev-contributing.txt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/dev-contributing.txt b/Documentation/dev-contributing.txt index 42a7f1b1e6..e7a4284846 100644 --- a/Documentation/dev-contributing.txt +++ b/Documentation/dev-contributing.txt @@ -171,7 +171,10 @@ To format Java source code, Gerrit uses the link:https://github.com/google/google-java-format[`google-java-format`] tool (version 1.7), and to format Bazel BUILD, WORKSPACE and .bzl files the link:https://github.com/bazelbuild/buildtools/tree/master/buildifier[`buildifier`] -tool (version 0.29.0). +tool (version 0.29.0). Unused dependencies are found and removed using the +link:https://github.com/bazelbuild/buildtools/tree/master/unused_deps[`unused_deps`] +build tool, a sibling of `buildifier`. + These tools automatically apply format according to the style guides; this streamlines code review by reducing the need for time-consuming, tedious, and contentious discussions about trivial issues like whitespace. From c5d726515a1d19eaafb0ec556c11a10eb2f6ae25 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 28 Jan 2020 21:11:00 +0900 Subject: [PATCH 5/6] DeleteZombieCommentsRefsTest: Open RevWalk in try-with-resource Change-Id: Ib151acaeca1d7c5e7371ccdd88b6c6e221a4a119 --- .../server/git/DeleteZombieCommentsRefsTest.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java b/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java index 6d0556f7b6..59343ba798 100644 --- a/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java +++ b/javatests/com/google/gerrit/server/git/DeleteZombieCommentsRefsTest.java @@ -176,13 +176,14 @@ public class DeleteZombieCommentsRefsTest { private static Ref createRefWithNonEmptyTreeCommit(Repository usersRepo, int changeId, int userId) throws IOException { - RevWalk rw = new RevWalk(usersRepo); - ObjectId fileObj = createBlob(usersRepo, String.format("file %d content", changeId)); - ObjectId treeObj = - createTree(usersRepo, rw.lookupBlob(fileObj), String.format("file%d.txt", changeId)); - ObjectId commitObj = createCommit(usersRepo, treeObj, null); - Ref refObj = createRef(usersRepo, commitObj, getRefName(changeId, userId)); - return refObj; + try (RevWalk rw = new RevWalk(usersRepo)) { + ObjectId fileObj = createBlob(usersRepo, String.format("file %d content", changeId)); + ObjectId treeObj = + createTree(usersRepo, rw.lookupBlob(fileObj), String.format("file%d.txt", changeId)); + ObjectId commitObj = createCommit(usersRepo, treeObj, null); + Ref refObj = createRef(usersRepo, commitObj, getRefName(changeId, userId)); + return refObj; + } } private static Ref createRefWithEmptyTreeCommit(Repository usersRepo, int changeId, int userId) From 91d62c07765455136dfcc1bb996b1c34dc074181 Mon Sep 17 00:00:00 2001 From: Thomas Draebing Date: Mon, 11 Nov 2019 09:42:14 +0100 Subject: [PATCH 6/6] Use the environment variables provided by the GerritCodeReview-plugin So far the pipeline described in the Jenkinsfile queried Gerrit to get the change metadata. By now the GerritCodeReview provides this data as environment variables. This reduces the boilerplate code needed in the Jenkinsfile. Change-Id: Ibc18d7f1b350e6070e93164cea6a5ff3fd826ae3 --- Jenkinsfile | 44 +++++++------------------------------------- 1 file changed, 7 insertions(+), 37 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index f2d361e6f3..631efcdd54 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -25,15 +25,6 @@ class Globals { static final String gerritRepositoryNameSha1Suffix = "-a6a0e4682515f3521897c5f950d1394f4619d928" } -class Change { - static String sha1 = "" - static String number = "" - static String branch = "" - static String ref = "" - static String patchNum = "" - static String url = "" -} - class Build { String url String result @@ -83,36 +74,17 @@ def postCheck(check) { gerritCheck(checks: [ "${check.uuid}" : "${check.getCheckResultFromBuild()}" ], url: "${check.consoleUrl}") } -def queryChangedFiles(url, changeNum, sha1) { - def queryUrl = "${url}changes/${Change.number}/revisions/${Change.sha1}/files/" +def queryChangedFiles(url) { + def queryUrl = "${url}changes/${env.GERRIT_CHANGE_NUMBER}/revisions/${env.GERRIT_PATCHSET_REVISION}/files/" def response = httpRequest queryUrl def files = response.getContent().substring(5) def filesJson = new JsonSlurper().parseText(files) return filesJson.keySet().findAll { it != "/COMMIT_MSG" } } -def queryChange(){ - def requestedChangeId = env.BRANCH_NAME.split('/')[1] - def queryUrl = "${Globals.gerritUrl}changes/${requestedChangeId}/?pp=0&O=3" - def response = httpRequest queryUrl - def jsonSlurper = new JsonSlurper() - return jsonSlurper.parseText(response.getContent().substring(5)) -} - -def getChangeMetaData(){ - def changeJson = queryChange() - Change.sha1 = changeJson.current_revision - Change.number = changeJson._number - Change.branch = changeJson.branch - def revision = changeJson.revisions.get(Change.sha1) - Change.ref = revision.ref - Change.patchNum = revision._number - Change.url = Globals.gerritUrl + "#/c/" + Change.number + "/" + Change.patchNum -} - def collectBuildModes() { Builds.modes = ["reviewdb", "notedb"] - def changedFiles = queryChangedFiles(Globals.gerritUrl, Change.number, Change.sha1) + def changedFiles = queryChangedFiles(Globals.gerritUrl) def polygerritFiles = changedFiles.findAll { it.startsWith("polygerrit-ui") || it.startsWith("lib/js") } @@ -137,11 +109,11 @@ def prepareBuildsForMode(buildName, mode="reviewdb", retryTimes = 1) { for (int i = 1; i <= retryTimes; i++) { try { slaveBuild = build job: "${buildName}", parameters: [ - string(name: 'REFSPEC', value: Change.ref), - string(name: 'BRANCH', value: Change.sha1), - string(name: 'CHANGE_URL', value: Change.url), + string(name: 'REFSPEC', value: "refs/changes/${env.BRANCH_NAME}"), + string(name: 'BRANCH', value: env.GERRIT_PATCHSET_REVISION), + string(name: 'CHANGE_URL', value: "${Globals.gerritUrl}c/${env.GERRIT_PROJECT}/+/${env.GERRIT_CHANGE_NUMBER}"), string(name: 'MODE', value: mode), - string(name: 'TARGET_BRANCH', value: Change.branch) + string(name: 'TARGET_BRANCH', value: env.GERRIT_BRANCH) ], propagate: false } finally { if (buildName == "Gerrit-codestyle"){ @@ -243,8 +215,6 @@ node ('master') { if (hasChangeNumber()) { stage('Preparing'){ gerritReview labels: ['Verified': 0, 'Code-Style': 0] - - getChangeMetaData() collectBuildModes() } }