Merge branch stable-3.4 into stable-3.5 * stable-3.4: Fix LoopOverCharArray bug pattern flagged by error prone Fix FloggerStringConcatenation bug pattern flagged by error prone Detect DelegateRepository in GarbageCollection operation docs: fix its vs it's grammar MergeUtil: Fix invalid argument index specifier flagged by JDK 17 Bazel: Add no_rbe tag to //tools/bzl:always_pass_test Add possibility to override MultiBaseLocalDiskRepositoryManager Change-Id: I704f60603ec5a60501efde1b9100651c48e555d3
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 74dbb6f..22c7e9f 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt
@@ -2236,7 +2236,7 @@ or "http://example.com:8080/gerrit/" so Gerrit can output links that point back to itself. + -Setting this is highly recommended, as its necessary for the upload +Setting this is highly recommended, as it is necessary for the upload code invoked by "git push" or "repo upload" to output hyperlinks to the newly uploaded changes.
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt index 75ad051..c11a3fa 100644 --- a/Documentation/dev-plugins.txt +++ b/Documentation/dev-plugins.txt
@@ -2684,7 +2684,7 @@ Plugins may also decide not to vote on a given change by returning an `Optional.empty()` (ie: the plugin is not enabled for this repository). -If a plugin decides not to vote, it's name will not be displayed in the UI and +If a plugin decides not to vote, its name will not be displayed in the UI and it will not be recoded in the database. .Gerrit's Pre-submit handling with three plugins
diff --git a/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java b/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java index cf3562f..fcd16ae 100644 --- a/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java +++ b/java/com/google/gerrit/httpd/auth/openid/OpenIdServiceImpl.java
@@ -178,7 +178,7 @@ aReq.addExtension(pape); } } catch (MessageException | ConsumerException e) { - logger.atSevere().withCause(e).log("Cannot create OpenID redirect for %s" + openidIdentifier); + logger.atSevere().withCause(e).log("Cannot create OpenID redirect for %s", openidIdentifier); return new DiscoveryResult(DiscoveryResult.Status.ERROR); }
diff --git a/java/com/google/gerrit/mail/ParserUtil.java b/java/com/google/gerrit/mail/ParserUtil.java index 4b292f3..40c5a95 100644 --- a/java/com/google/gerrit/mail/ParserUtil.java +++ b/java/com/google/gerrit/mail/ParserUtil.java
@@ -115,7 +115,8 @@ int numConsecutiveDigits = 0; int maxConsecutiveDigits = 0; int numDigitGroups = 0; - for (char c : s.toCharArray()) { + for (int i = 0; i < s.length(); i++) { + char c = s.charAt(i); if (c >= '0' && c <= '9') { numConsecutiveDigits++; } else if (numConsecutiveDigits > 0) {
diff --git a/java/com/google/gerrit/server/git/DelegateRepository.java b/java/com/google/gerrit/server/git/DelegateRepository.java index ddfc115..9ead038 100644 --- a/java/com/google/gerrit/server/git/DelegateRepository.java +++ b/java/com/google/gerrit/server/git/DelegateRepository.java
@@ -65,6 +65,10 @@ this.delegate = delegate; } + Repository delegate() { + return delegate; + } + @Override public void create(boolean bare) throws IOException { delegate.create(bare);
diff --git a/java/com/google/gerrit/server/git/GarbageCollection.java b/java/com/google/gerrit/server/git/GarbageCollection.java index a2942fe..6ae4d62 100644 --- a/java/com/google/gerrit/server/git/GarbageCollection.java +++ b/java/com/google/gerrit/server/git/GarbageCollection.java
@@ -85,7 +85,12 @@ try (Repository repo = repoManager.openRepository(p)) { logGcConfiguration(p, repo, aggressive); print(writer, "collecting garbage for \"" + p + "\":\n"); - GarbageCollectCommand gc = Git.wrap(repo).gc(); + GarbageCollectCommand gc = + Git.wrap( + repo instanceof DelegateRepository + ? ((DelegateRepository) repo).delegate() + : repo) + .gc(); gc.setAggressive(aggressive); logGcInfo(p, "before:", gc.getStatistics()); gc.setProgressMonitor(
diff --git a/java/com/google/gerrit/server/git/GitRepositoryManagerModule.java b/java/com/google/gerrit/server/git/GitRepositoryManagerModule.java index 6266925..dfbe663 100644 --- a/java/com/google/gerrit/server/git/GitRepositoryManagerModule.java +++ b/java/com/google/gerrit/server/git/GitRepositoryManagerModule.java
@@ -15,6 +15,7 @@ package com.google.gerrit.server.git; import com.google.gerrit.lifecycle.LifecycleModule; +import com.google.gerrit.server.ModuleImpl; import com.google.gerrit.server.config.RepositoryConfig; import com.google.gerrit.server.git.LocalDiskRepositoryManager.LocalDiskRepositoryManagerModule; import com.google.gerrit.server.git.MultiBaseLocalDiskRepositoryManager.MultiBaseLocalDiskRepositoryManagerModule; @@ -24,7 +25,9 @@ * Module to install {@link MultiBaseLocalDiskRepositoryManager} rather than {@link * LocalDiskRepositoryManager} if needed. */ +@ModuleImpl(name = GitRepositoryManagerModule.MANAGER_MODULE) public class GitRepositoryManagerModule extends LifecycleModule { + public static final String MANAGER_MODULE = "git-manager"; private final RepositoryConfig repoConfig;
diff --git a/java/com/google/gerrit/server/git/MergeUtil.java b/java/com/google/gerrit/server/git/MergeUtil.java index 3a4d407..c1333cb 100644 --- a/java/com/google/gerrit/server/git/MergeUtil.java +++ b/java/com/google/gerrit/server/git/MergeUtil.java
@@ -306,13 +306,13 @@ int nameLength = Math.max(oursName.length(), theirsName.length()); String oursNameFormatted = String.format( - "%0$-" + nameLength + "s (%s %s)", + "%-" + nameLength + "s (%s %s)", oursName, abbreviateName(ours, NAME_ABBREV_LEN), oursMsg.substring(0, Math.min(oursMsg.length(), 60))); String theirsNameFormatted = String.format( - "%0$-" + nameLength + "s (%s %s)", + "%-" + nameLength + "s (%s %s)", theirsName, abbreviateName(theirs, NAME_ABBREV_LEN), theirsMsg.substring(0, Math.min(theirsMsg.length(), 60)));
diff --git a/java/com/google/gerrit/server/git/MultiProgressMonitor.java b/java/com/google/gerrit/server/git/MultiProgressMonitor.java index 5cd9bd0..4985288 100644 --- a/java/com/google/gerrit/server/git/MultiProgressMonitor.java +++ b/java/com/google/gerrit/server/git/MultiProgressMonitor.java
@@ -191,7 +191,7 @@ volatileTotal.addAndGet(workUnits); } else { logger.atWarning().log( - "Total work has been finalized on sub-task " + getName() + " and cannot be updated"); + "Total work has been finalized on sub-task %s and cannot be updated", getName()); } }
diff --git a/javatests/com/google/gerrit/mail/data/NonUTF8Message.java b/javatests/com/google/gerrit/mail/data/NonUTF8Message.java index 60368eb..86a0b56 100644 --- a/javatests/com/google/gerrit/mail/data/NonUTF8Message.java +++ b/javatests/com/google/gerrit/mail/data/NonUTF8Message.java
@@ -45,7 +45,8 @@ public int[] rawChars() { int[] arr = new int[raw.length()]; int i = 0; - for (char c : raw.toCharArray()) { + for (int j = 0; j < raw.length(); j++) { + char c = raw.charAt(j); arr[i++] = c; } return arr;
diff --git a/javatests/com/google/gerrit/server/config/GitwebConfigTest.java b/javatests/com/google/gerrit/server/config/GitwebConfigTest.java index cb6de34..7316074 100644 --- a/javatests/com/google/gerrit/server/config/GitwebConfigTest.java +++ b/javatests/com/google/gerrit/server/config/GitwebConfigTest.java
@@ -24,7 +24,8 @@ @Test public void validPathSeparator() { - for (char c : VALID_CHARACTERS.toCharArray()) { + for (int i = 0; i < VALID_CHARACTERS.length(); i++) { + char c = VALID_CHARACTERS.charAt(i); assertWithMessage("valid character rejected: " + c) .that(GitwebConfig.isValidPathSeparator(c)) .isTrue(); @@ -33,7 +34,8 @@ @Test public void inalidPathSeparator() { - for (char c : SOME_INVALID_CHARACTERS.toCharArray()) { + for (int i = 0; i < SOME_INVALID_CHARACTERS.length(); i++) { + char c = SOME_INVALID_CHARACTERS.charAt(i); assertWithMessage("invalid character accepted: " + c) .that(GitwebConfig.isValidPathSeparator(c)) .isFalse();
diff --git a/javatests/com/google/gerrit/server/git/GarbageCollectionTest.java b/javatests/com/google/gerrit/server/git/GarbageCollectionTest.java new file mode 100644 index 0000000..41b5d79 --- /dev/null +++ b/javatests/com/google/gerrit/server/git/GarbageCollectionTest.java
@@ -0,0 +1,101 @@ +// Copyright (C) 2022 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.truth.Truth.assertThat; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.common.collect.ImmutableList; +import com.google.gerrit.entities.Project; +import com.google.gerrit.entities.Project.NameKey; +import com.google.gerrit.extensions.registration.DynamicSet; +import com.google.gerrit.server.config.GcConfig; +import com.google.gerrit.server.config.SitePaths; +import com.google.gerrit.server.plugincontext.PluginContext.PluginMetrics; +import com.google.gerrit.server.plugincontext.PluginSetContext; +import java.io.IOException; +import org.eclipse.jgit.errors.RepositoryNotFoundException; +import org.eclipse.jgit.lib.Config; +import org.eclipse.jgit.lib.Repository; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; + +public class GarbageCollectionTest { + private static final Project.NameKey FOO = Project.nameKey("foo"); + + @Rule public final MockitoRule mockito = MockitoJUnit.rule(); + @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); + + @Mock private GcConfig gcConfig; + @Mock private DelegateRepository wrapper; + + private SitePaths site; + private Config cfg; + + @Before + public void setup() throws Exception { + site = new SitePaths(temporaryFolder.newFolder().toPath()); + site.resolve("git").toFile().mkdir(); + cfg = new Config(); + cfg.setString("gerrit", null, "basePath", "git"); + } + + @Test + public void shouldCallGcOnDelegatedRepositoryWhenDelegateRepositoryIsPassed() throws IOException { + // given + GarbageCollection objectUnderTest = prepareObjectForTesting(); + + // when + objectUnderTest.run(ImmutableList.of(FOO), false, null); + + // then + verify(wrapper).delegate(); + } + + private GarbageCollection prepareObjectForTesting() throws IOException { + LocalDiskRepositoryManager repoManager = new DelegatedRepositoryManager(site, cfg, wrapper); + try (Repository repo = repoManager.createRepository(FOO)) { + assertThat(repo).isNotNull(); + } + return new GarbageCollection( + repoManager, + new GarbageCollectionQueue(), + gcConfig, + new PluginSetContext<>(new DynamicSet<>(), PluginMetrics.DISABLED_INSTANCE)); + } + + private static final class DelegatedRepositoryManager extends LocalDiskRepositoryManager { + private final DelegateRepository wrapper; + + private DelegatedRepositoryManager(SitePaths site, Config cfg, DelegateRepository wrapper) { + super(site, cfg); + this.wrapper = wrapper; + } + + @Override + public Repository openRepository(NameKey name) throws RepositoryNotFoundException { + Repository opened = super.openRepository(name); + when(wrapper.delegate()).thenReturn(opened); + when(wrapper.getConfig()).thenReturn(opened.getConfig()); + return wrapper; + } + } +}
diff --git a/tools/bzl/BUILD b/tools/bzl/BUILD index 7febbac..8f63d08 100644 --- a/tools/bzl/BUILD +++ b/tools/bzl/BUILD
@@ -7,4 +7,5 @@ sh_test( name = "always_pass_test", srcs = ["always_pass_test.sh"], + tags = ["no_rbe"], )