From 6167187f11921e80b547d282001e058e31ce5b57 Mon Sep 17 00:00:00 2001 From: Dmitriy Poluyanov Date: Wed, 12 Oct 2022 17:25:35 +0300 Subject: [PATCH] Support git --push-option in order to use copybara with GitLab --- docs/reference.md | 3 ++- .../copybara/git/GerritDestination.java | 3 ++- .../google/copybara/git/GitDestination.java | 14 +++++++++--- .../copybara/git/GitHubPrDestination.java | 3 ++- java/com/google/copybara/git/GitModule.java | 20 +++++++++++++++-- .../google/copybara/git/GitRepository.java | 22 +++++++++++++++---- .../git/GitDestinationIntegrateTest.java | 19 ++++++++++++++++ .../copybara/git/GitRepositoryTest.java | 2 +- 8 files changed, 73 insertions(+), 13 deletions(-) diff --git a/docs/reference.md b/docs/reference.md index 808baa9ae..5beadb01f 100755 --- a/docs/reference.md +++ b/docs/reference.md @@ -2202,7 +2202,7 @@ Name | Type | Description Creates a commit in a git repository using the transformed worktree.

For GitHub use git.github_destination. For creating Pull Requests in GitHub, use git.github_pr_destination. For creating a Gerrit change use git.gerrit_destination.

Given that Copybara doesn't ask for user/password in the console when doing the push to remote repos, you have to use ssh protocol, have the credentials cached or use a credential manager. -`destination` `git.destination(url, push='master', tag_name=None, tag_msg=None, fetch=None, partial_fetch=False, integrates=None, primary_branch_migration=False, checker=None)` +`destination` `git.destination(url, push='master', tag_name=None, tag_msg=None, fetch=None, partial_fetch=False, integrates=None, primary_branch_migration=False, checker=None, push_options=None)` #### Parameters: @@ -2218,6 +2218,7 @@ partial_fetch | `bool`

This is an experimental feature that only works for integrates | `sequence of git_integrate` or `NoneType`

Integrate changes from a url present in the migrated change label. Defaults to a semi-fake merge if COPYBARA_INTEGRATE_REVIEW label is present in the message

primary_branch_migration | `bool`

When enabled, copybara will ignore the 'push' and 'fetch' params if either is 'master' or 'main' and instead try to establish the default git branch. If this fails, it will fall back to the param's declared value.
This is intended to help migrating to the new standard of using 'main' without breaking users relying on the legacy default.

checker | `checker` or `NoneType`

A checker that can check leaks or other checks in the commit created.

+push_options | `sequence` or `NoneType`

A sequence of git push options that can pass into push command. Defaults to none which represents no push options.

diff --git a/java/com/google/copybara/git/GerritDestination.java b/java/com/google/copybara/git/GerritDestination.java index cc5a31846..bb1acc8e8 100644 --- a/java/com/google/copybara/git/GerritDestination.java +++ b/java/com/google/copybara/git/GerritDestination.java @@ -615,7 +615,8 @@ static GerritDestination newGerritDestination( gerritSubmit, primaryBranchMigrationMode), integrates, - checker), + checker, + ImmutableList.of()), submit); } diff --git a/java/com/google/copybara/git/GitDestination.java b/java/com/google/copybara/git/GitDestination.java index f0225595f..676906a96 100644 --- a/java/com/google/copybara/git/GitDestination.java +++ b/java/com/google/copybara/git/GitDestination.java @@ -110,6 +110,7 @@ static class MessageInfo { @Nullable private String resolvedPrimary = null; private final Iterable integrates; private final WriteHook writerHook; + private final Iterable pushOptions; @Nullable private final Checker checker; private final LazyResourceLoader localRepo; @@ -126,7 +127,8 @@ static class MessageInfo { GeneralOptions generalOptions, WriteHook writerHook, Iterable integrates, - @Nullable Checker checker) { + @Nullable Checker checker, + Iterable pushOptions) { this.repoUrl = checkNotNull(repoUrl); this.fetch = checkNotNull(fetch); this.push = checkNotNull(push); @@ -140,6 +142,7 @@ static class MessageInfo { this.integrates = checkNotNull(integrates); this.writerHook = checkNotNull(writerHook); this.checker = checker; + this.pushOptions = checkNotNull(pushOptions); this.localRepo = memoized(ignored -> destinationOptions.localGitRepo(repoUrl)); } @@ -191,7 +194,8 @@ public Writer newWriter(WriterContext writerContext) throws Validat destinationOptions.rebaseWhenBaseline(), gitOptions.visitChangePageSize, gitOptions.gitTagOverwrite, - checker); + checker, + pushOptions); } /** @@ -242,6 +246,7 @@ public static class WriterImpl private final int visitChangePageSize; private final boolean gitTagOverwrite; @Nullable private final Checker checker; + private final Iterable pushOptions; /** Create a new git.destination writer */ WriterImpl( @@ -265,7 +270,8 @@ public static class WriterImpl boolean rebase, int visitChangePageSize, boolean gitTagOverwrite, - Checker checker) { + Checker checker, + Iterable pushOptions) { this.skipPush = skipPush; this.repoUrl = checkNotNull(repoUrl); this.remoteFetch = checkNotNull(remoteFetch); @@ -289,6 +295,7 @@ public static class WriterImpl this.visitChangePageSize = visitChangePageSize; this.gitTagOverwrite = gitTagOverwrite; this.checker = checker; + this.pushOptions = pushOptions; } @Override @@ -659,6 +666,7 @@ public ImmutableList write(TransformResult transformResult, String serverResponse = generalOptions.repoTask( "push", () -> scratchClone.push() + .pushOptions(pushOptions) .withRefspecs(repoUrl, tagName != null ? ImmutableList.of(scratchClone.createRefSpec( diff --git a/java/com/google/copybara/git/GitHubPrDestination.java b/java/com/google/copybara/git/GitHubPrDestination.java index d72381282..276a4f1d5 100644 --- a/java/com/google/copybara/git/GitHubPrDestination.java +++ b/java/com/google/copybara/git/GitHubPrDestination.java @@ -191,7 +191,8 @@ public Writer newWriter(WriterContext writerContext) throws Validat destinationOptions.rebaseWhenBaseline(), gitOptions.visitChangePageSize, gitOptions.gitTagOverwrite, - checker) { + checker, + ImmutableList.of()) { @Override public ImmutableList write( TransformResult transformResult, Glob destinationFiles, Console console) diff --git a/java/com/google/copybara/git/GitModule.java b/java/com/google/copybara/git/GitModule.java index fdd6b427f..21672ec57 100644 --- a/java/com/google/copybara/git/GitModule.java +++ b/java/com/google/copybara/git/GitModule.java @@ -1339,6 +1339,16 @@ private boolean convertDescribeVersion(Object describeVersion) { doc = "A checker that can check leaks or other checks in the commit created. ", named = true, positional = false), + @Param( + name = "push_options", + allowedTypes = { + @ParamType(type = Sequence.class), + @ParamType(type = NoneType.class), + }, + defaultValue = "None", + doc = "A sequence of git push options that can pass into push command. Defaults to none which represents no push options.", + named = true, + positional = false), }, useStarlarkThread = true) @UsesFlags(GitDestinationOptions.class) @@ -1352,6 +1362,7 @@ public GitDestination destination( Object integrates, Boolean primaryBranchMigration, Object checker, + Object pushOptions, StarlarkThread thread) throws EvalException { GitDestinationOptions destinationOptions = options.get(GitDestinationOptions.class); @@ -1365,6 +1376,9 @@ public GitDestination destination( "Skipping git checker for git.destination. Note that this could" + " cause leaks or other problems"); } + Iterable resolvedPushOptions = Starlark.isNullOrNone(pushOptions) + ? ImmutableList.of() + : Sequence.cast(pushOptions, String.class, "push_options"); return new GitDestination( fixHttp( checkNotEmpty(firstNotNull(destinationOptions.url, url), "url"), @@ -1384,7 +1398,8 @@ public GitDestination destination( Starlark.isNullOrNone(integrates) ? defaultGitIntegrate : Sequence.cast(integrates, GitIntegrateChanges.class, "integrates"), - maybeChecker); + maybeChecker, + resolvedPushOptions); } @SuppressWarnings("unused") @@ -1612,7 +1627,8 @@ public GitDestination gitHubDestination( Starlark.isNullOrNone(integrates) ? defaultGitIntegrate : Sequence.cast(integrates, GitIntegrateChanges.class, "integrates"), - checkerObj); + checkerObj, + ImmutableList.of()); } @SuppressWarnings("unused") diff --git a/java/com/google/copybara/git/GitRepository.java b/java/com/google/copybara/git/GitRepository.java index c4b5a76cd..91baeb9f5 100644 --- a/java/com/google/copybara/git/GitRepository.java +++ b/java/com/google/copybara/git/GitRepository.java @@ -442,7 +442,7 @@ public LogCmd log(String referenceExpr) { @CheckReturnValue public PushCmd push() { - return new PushCmd(this, /*url=*/null, ImmutableList.of(), /*prune=*/false); + return new PushCmd(this, /*url=*/null, ImmutableList.of(), /*prune=*/false, ImmutableList.of()); } @CheckReturnValue @@ -678,6 +678,12 @@ protected String runPush(PushCmd pushCmd) throws RepoException, ValidationExcept cmd.add("--no-verify"); } + if(!pushCmd.pushOptions.isEmpty()) { + for(String option : pushCmd.pushOptions) { + cmd.add("--push-option=" + option); + } + } + if (pushCmd.url != null) { cmd.add(validateUrl(pushCmd.url)); for (Refspec refspec : pushCmd.refspecs) { @@ -1887,6 +1893,8 @@ public static class PushCmd { private final ImmutableList refspecs; private final boolean prune; + private final ImmutableList pushOptions; + @Nullable public String getUrl() { return url; @@ -1903,24 +1911,30 @@ public boolean isPrune() { @CheckReturnValue public PushCmd(GitRepository repo, @Nullable String url, ImmutableList refspecs, - boolean prune) { + boolean prune, ImmutableList pushOptions) { this.repo = checkNotNull(repo); this.url = url; this.refspecs = checkNotNull(refspecs); Preconditions.checkArgument(refspecs.isEmpty() || url != null, "refspec can only be" + " used when a url is passed"); this.prune = prune; + this.pushOptions = pushOptions; } @CheckReturnValue public PushCmd withRefspecs(String url, Iterable refspecs) { return new PushCmd(repo, checkNotNull(url), ImmutableList.copyOf(refspecs), - prune); + prune, pushOptions); } @CheckReturnValue public PushCmd prune(boolean prune) { - return new PushCmd(repo, url, this.refspecs, prune); + return new PushCmd(repo, url, this.refspecs, prune, pushOptions); + } + + @CheckReturnValue + public PushCmd pushOptions(Iterable pushOptions) { + return new PushCmd(repo, url, refspecs, prune, ImmutableList.copyOf(pushOptions)); } /** diff --git a/javatests/com/google/copybara/git/GitDestinationIntegrateTest.java b/javatests/com/google/copybara/git/GitDestinationIntegrateTest.java index c2373cd5e..8dc120e78 100644 --- a/javatests/com/google/copybara/git/GitDestinationIntegrateTest.java +++ b/javatests/com/google/copybara/git/GitDestinationIntegrateTest.java @@ -351,6 +351,25 @@ public void testIncludeFiles() throws ValidationException, IOException, RepoExce .isEqualTo(Lists.newArrayList(previous.getCommit().getSha1())); } + @Test + public void testPushOptionsTrigger() throws Exception { + RepoException e = assertThrows(RepoException.class, () -> migrateOriginChange( + destination( + "url = '" + url + "'", + String.format("fetch = '%s'", primaryBranch), + String.format("push = '%s'", primaryBranch), + "integrates = []", + "push_options = ['ci.skip']"), + "Test change\n\n" + + GitModule.DEFAULT_INTEGRATE_LABEL + "=http://should_not_be_used\n", "some content")); + assertThat(e) + .hasMessageThat() + .contains("--push-option=ci.skip"); + assertThat(e) + .hasMessageThat() + .contains("the receiving end does not support push options"); + } + @Test public void testIncludeFilesOutdatedBranch() throws Exception { Path repoPath = Files.createTempDirectory("test"); diff --git a/javatests/com/google/copybara/git/GitRepositoryTest.java b/javatests/com/google/copybara/git/GitRepositoryTest.java index ea8827b08..66829118c 100644 --- a/javatests/com/google/copybara/git/GitRepositoryTest.java +++ b/javatests/com/google/copybara/git/GitRepositoryTest.java @@ -1094,7 +1094,7 @@ public void doPushWithHook(GitRepository origin) throws Exception { origin, "file://" + remote.getGitDir(), ImmutableList.of(repository.createRefSpec("+" + defaultBranch + ":" + defaultBranch)), - false).run(); + false, ImmutableList.of()).run(); }