Always use http to get commit-msg hook
Http should always be available from primary servers and using scp
is getting complicated since the implementation of scp varies between
different OpenSSH versions.
Bug: Issue 16705
Bug: Issue 15944
Change-Id: Ifb61fa66325d771cc606814f38af4d99d0d68c87
diff --git a/src/main/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHook.java b/src/main/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHook.java
index 135d9b2..ccdfeda 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHook.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHook.java
@@ -15,115 +15,98 @@
package com.googlesource.gerrit.plugins.download.command;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.config.DownloadScheme;
-import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.googlesource.gerrit.plugins.download.scheme.AnonymousHttpScheme;
import com.googlesource.gerrit.plugins.download.scheme.HttpScheme;
import com.googlesource.gerrit.plugins.download.scheme.SshScheme;
-import java.util.Optional;
import org.eclipse.jgit.lib.Config;
public class CloneWithCommitMsgHook extends CloneCommand {
@VisibleForTesting protected static final String HOOK_COMMAND_KEY = "installCommitMsgHookCommand";
@VisibleForTesting protected static final String EXTRA_COMMAND_KEY = "installCommitExtraCommand";
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private static final String HOOK = "hooks/commit-msg";
private static final String TARGET = " `git rev-parse --git-dir`/";
private final String configCommand;
private final String extraCommand;
- private final SshScheme sshScheme;
- private final Provider<CurrentUser> userProvider;
+ private final String canonicalWebUrl;
@Inject
CloneWithCommitMsgHook(
- @GerritServerConfig Config config, SshScheme sshScheme, Provider<CurrentUser> userProvider) {
- this.configCommand = config.getString("gerrit", null, HOOK_COMMAND_KEY);
- this.extraCommand = config.getString("gerrit", null, EXTRA_COMMAND_KEY);
- this.sshScheme = sshScheme;
- this.userProvider = userProvider;
+ @GerritServerConfig Config config, @CanonicalWebUrl @Nullable Provider<String> urlProvider) {
+ this.configCommand = config.getString("gerrit", null, "installCommitMsgHookCommand");
+ this.extraCommand = config.getString("gerrit", null, "installCommitExtraCommand");
+ this.canonicalWebUrl = urlProvider != null ? urlProvider.get() : null;
}
@Nullable
@Override
public String getCommand(DownloadScheme scheme, String project) {
- Optional<String> username = userProvider.get().getUserName();
- if (!username.isPresent()) {
- return null;
- }
String projectName = getBaseName(project);
+ StringBuilder command = null;
+
if (configCommand != null) {
- return new StringBuilder()
- .append(super.getCommand(scheme, project))
+ command =
+ new StringBuilder()
+ .append(super.getCommand(scheme, project))
+ .append(" && (cd ")
+ .append(QuoteUtil.quote(projectName))
+ .append(" && ")
+ .append(configCommand)
+ .append(")");
+ } else if (scheme instanceof HttpScheme
+ || scheme instanceof AnonymousHttpScheme
+ || scheme instanceof SshScheme) {
+ command =
+ new StringBuilder()
+ .append(super.getCommand(scheme, project))
+ .append(" && (cd ")
+ .append(QuoteUtil.quote(projectName))
+ .append(" && mkdir -p .git/hooks")
+ .append(" && curl -Lo")
+ .append(TARGET)
+ .append(HOOK)
+ .append(" ")
+ .append(getHookUrl())
+ .append("; chmod +x")
+ .append(TARGET)
+ .append(HOOK)
+ .append(")");
+ }
+
+ if (extraCommand != null && command != null) {
+ command
.append(" && (cd ")
.append(QuoteUtil.quote(projectName))
.append(" && ")
- .append(configCommand)
- .append(")")
- .toString();
+ .append(extraCommand)
+ .append(")");
}
-
- if (scheme instanceof SshScheme) {
- StringBuilder b =
- new StringBuilder().append(super.getCommand(scheme, project)).append(" && scp -p");
-
- if (sshScheme.getSshdPort() != 22) {
- b.append(" -P ").append(sshScheme.getSshdPort());
- }
-
- b.append(" ")
- .append(username.get())
- .append("@")
- .append(sshScheme.getSshdHost())
- .append(":")
- .append(HOOK)
- .append(" ")
- .append(QuoteUtil.quote(projectName + "/.git/hooks/"));
- if (extraCommand != null) {
- b.append(" && (cd ")
- .append(QuoteUtil.quote(projectName))
- .append(" && ")
- .append(extraCommand)
- .append(")");
- }
- return b.toString();
- }
-
- if (scheme instanceof HttpScheme || scheme instanceof AnonymousHttpScheme) {
- return new StringBuilder()
- .append(super.getCommand(scheme, project))
- .append(" && (cd ")
- .append(QuoteUtil.quote(projectName))
- .append(" && mkdir -p .git/hooks")
- .append(" && curl -Lo")
- .append(TARGET)
- .append(HOOK)
- .append(" ")
- .append(getHttpHost(scheme, project))
- .append("tools/")
- .append(HOOK)
- .append("; chmod +x")
- .append(TARGET)
- .append(HOOK)
- .append(")")
- .toString();
- }
- return null;
+ return command != null ? command.toString() : null;
}
- private String getHttpHost(DownloadScheme scheme, String project) {
- String host = scheme.getUrl(project);
- host = host.substring(0, host.lastIndexOf(project));
- int auth = host.lastIndexOf("/a/");
- if (auth > -1) {
- host = host.substring(0, auth + 1);
+ private StringBuilder getHookUrl() {
+ StringBuilder hookUrl = new StringBuilder();
+ if (canonicalWebUrl != null) {
+ hookUrl.append(canonicalWebUrl);
+ if (!canonicalWebUrl.endsWith("/")) {
+ hookUrl.append("/");
+ }
+ hookUrl.append("tools/").append(HOOK);
+ } else {
+ logger.atWarning().log(
+ "Cannot add commit-msg hook URL since gerrit.canonicalWebUrl isn't configured.");
}
- return host;
+ return hookUrl;
}
private static String getBaseName(String project) {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHookTest.java b/src/test/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHookTest.java
index 03c1ba0..3d1f810 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHookTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHookTest.java
@@ -29,12 +29,8 @@
assertThat(command)
.isEqualTo(
String.format(
- "git clone \"%s\" && scp -p -P %d %s@%s:hooks/commit-msg %s/.git/hooks/",
- sshScheme.getUrl(ENV.projectName),
- ENV.sshPort,
- ENV.userName,
- ENV.fqdn,
- baseName(ENV.projectName)));
+ "git clone \"%s\" && %s",
+ sshScheme.getUrl(ENV.projectName), getDefaultHookCommand()));
}
@Test
@@ -68,8 +64,12 @@
assertThat(command)
.isEqualTo(
String.format(
- "git clone \"%s\" && (cd %s && %s)",
- sshScheme.getUrl(ENV.projectName), baseName(ENV.projectName), hookCommand));
+ "git clone \"%s\" && (cd %s && %s) && (cd %s && %s)",
+ sshScheme.getUrl(ENV.projectName),
+ baseName(ENV.projectName),
+ hookCommand,
+ baseName(ENV.projectName),
+ extraCommand));
}
@Test
@@ -78,11 +78,8 @@
assertThat(command)
.isEqualTo(
String.format(
- "git clone \"%s\" && (cd %s && mkdir -p .git/hooks && curl -Lo `git rev-parse --git-dir`/hooks/commit-msg https://%s@%s/tools/hooks/commit-msg; chmod +x `git rev-parse --git-dir`/hooks/commit-msg)",
- httpScheme.getUrl(ENV.projectName),
- baseName(ENV.projectName),
- ENV.urlEncodedUserName(),
- ENV.fqdn));
+ "git clone \"%s\" && %s",
+ httpScheme.getUrl(ENV.projectName), getDefaultHookCommand()));
}
@Test
@@ -105,18 +102,28 @@
assertThat(command)
.isEqualTo(
String.format(
- "git clone \"%s\" && (cd %s && %s)",
- httpScheme.getUrl(ENV.projectName), baseName(ENV.projectName), hookCommand));
+ "git clone \"%s\" && (cd %s && %s) && (cd %s && %s)",
+ httpScheme.getUrl(ENV.projectName),
+ baseName(ENV.projectName),
+ hookCommand,
+ baseName(ENV.projectName),
+ extraCommand));
}
private String baseName(String projectName) {
return projectName.substring(projectName.lastIndexOf('/') + 1);
}
+ private String getDefaultHookCommand() {
+ return String.format(
+ "(cd %s && mkdir -p .git/hooks && curl -Lo `git rev-parse --git-dir`/hooks/commit-msg https://%s/tools/hooks/commit-msg; chmod +x `git rev-parse --git-dir`/hooks/commit-msg)",
+ baseName(ENV.projectName), ENV.fqdn);
+ }
+
private CloneCommand getCloneCommand(String hookCommand, String extraCommand) {
Config cfg = new Config();
cfg.setString("gerrit", null, HOOK_COMMAND_KEY, hookCommand);
cfg.setString("gerrit", null, EXTRA_COMMAND_KEY, extraCommand);
- return new CloneWithCommitMsgHook(cfg, sshScheme, userProvider);
+ return new CloneWithCommitMsgHook(cfg, urlProvider);
}
}
