diff --git a/docs/reference.md b/docs/reference.md index cd7465c65..2cfcaa8be 100755 --- a/docs/reference.md +++ b/docs/reference.md @@ -2131,7 +2131,7 @@ version_selector | `latestVersionSelector`

Select a custom version (tag)to Creates changes in a new pull request in the destination. -`gitHubPrDestination git.github_pr_destination(url, destination_ref="master", pr_branch=None, title=None, body=None, integrates=None, api_checker=None, update_description=False)` +`gitHubPrDestination git.github_pr_destination(url, destination_ref="master", push_to_fork=False, fork_url=None, pr_branch=None, title=None, body=None, integrates=None, api_checker=None, update_description=False)` #### Parameters: @@ -2140,6 +2140,8 @@ Parameter | Description --------- | ----------- url | `string`

Url of the GitHub project. For example "https://github.com/google/copybara'"

destination_ref | `string`

Destination reference for the change. By default 'master'

+push_to_fork | `boolean`

Indicates that the result of the change should be pushed to the current user's personal fork. The PullRequest will still be created on the upstream project.

+fork_url | `string`

TODO

pr_branch | `string`

Customize the pull request branch. Any variable present in the message in the form of ${CONTEXT_REFERENCE} will be replaced by the corresponding stable reference (head, PR number, Gerrit change number, etc.).

title | `string`

When creating (or updating if `update_description` is set) a pull request, use this title. By default it uses the change first line. This field accepts a template with labels. For example: `"Change ${CONTEXT_REFERENCE}"`

body | `string`

When creating (or updating if `update_description` is set) a pull request, use this body. By default it uses the change summary. This field accepts a template with labels. For example: `"Change ${CONTEXT_REFERENCE}"`

diff --git a/java/com/google/copybara/git/GitDestination.java b/java/com/google/copybara/git/GitDestination.java index 7f810bfeb..54d490d66 100644 --- a/java/com/google/copybara/git/GitDestination.java +++ b/java/com/google/copybara/git/GitDestination.java @@ -195,7 +195,8 @@ public static class WriterImpl implements Writer { final boolean skipPush; - private final String repoUrl; + private final String fetchRepoUrl; + private final String pushRepoUrl; private final String remoteFetch; private final String remotePush; @Nullable private final String tagNameTemplate; @@ -218,10 +219,25 @@ public static class WriterImpl private final int visitChangePageSize; private final boolean gitTagOverwrite; + @Deprecated + WriterImpl(boolean skipPush, String repoUrl, String remoteFetch, + String remotePush, String tagNameTemplate, String tagMsgTemplate, + GeneralOptions generalOptions, WriteHook writeHook, S state, + boolean nonFastForwardPush, Iterable integrates, + boolean lastRevFirstParent, boolean ignoreIntegrationErrors, String localRepoPath, + String committerName, String committerEmail, boolean rebase, int visitChangePageSize, + boolean gitTagOverwrite) { + this( + skipPush, repoUrl, repoUrl, remoteFetch, remotePush, tagNameTemplate, tagMsgTemplate, + generalOptions, writeHook, state, nonFastForwardPush, integrates, lastRevFirstParent, + ignoreIntegrationErrors, localRepoPath, committerName, committerEmail, rebase, + visitChangePageSize, gitTagOverwrite); + } + /** * Create a new git.destination writer */ - WriterImpl(boolean skipPush, String repoUrl, String remoteFetch, + WriterImpl(boolean skipPush, String fetchRepoUrl, String pushRepoUrl, String remoteFetch, String remotePush, String tagNameTemplate, String tagMsgTemplate, GeneralOptions generalOptions, WriteHook writeHook, S state, boolean nonFastForwardPush, Iterable integrates, @@ -229,7 +245,8 @@ public static class WriterImpl String committerName, String committerEmail, boolean rebase, int visitChangePageSize, boolean gitTagOverwrite) { this.skipPush = skipPush; - this.repoUrl = checkNotNull(repoUrl); + this.fetchRepoUrl = checkNotNull(fetchRepoUrl); + this.pushRepoUrl = checkNotNull(pushRepoUrl); this.remoteFetch = checkNotNull(remoteFetch); this.remotePush = checkNotNull(remotePush); this.tagNameTemplate = tagNameTemplate; @@ -284,7 +301,7 @@ public void visitChanges(@Nullable GitRevision start, ChangesVisitor visitor) private void fetchIfNeeded(GitRepository repo, Console console) throws RepoException, ValidationException { if (!state.alreadyFetched) { - GitRevision revision = fetchFromRemote(console, repo, repoUrl, remoteFetch); + GitRevision revision = fetchFromRemote(console, repo, fetchRepoUrl, remoteFetch); if (revision != null) { repo.simpleCommand("branch", state.localBranch, revision.getSha1()); } @@ -344,7 +361,7 @@ private GitRevision getLocalBranchRevision(GitRepository gitRepository) throws R return null; } throw new RepoException(String.format("Could not find %s in %s and '%s' was not used", - remoteFetch, repoUrl, GeneralOptions.FORCE)); + remoteFetch, fetchRepoUrl, GeneralOptions.FORCE)); } } @@ -436,7 +453,7 @@ public ImmutableList write(TransformResult transformResult, Glob destinationFiles, Console console) throws ValidationException, RepoException, IOException { logger.atInfo().log( - "Exporting from %s to: url=%s ref=%s", transformResult.getPath(), repoUrl, remotePush); + "Exporting from %s to: url=%s ref=%s", transformResult.getPath(), pushRepoUrl, remotePush); String baseline = transformResult.getBaseline(); GitRepository scratchClone = getRepository(console); @@ -450,12 +467,12 @@ public ImmutableList write(TransformResult transformResult, if (state.firstWrite) { String reference = baseline != null ? baseline : state.localBranch; - configForPush(getRepository(console), repoUrl, remotePush); + configForPush(getRepository(console), pushRepoUrl, remotePush); if (!force && localBranchRevision == null) { throw new RepoException(String.format( "Cannot checkout '%s' from '%s'. Use '%s' if the destination is a new git repo or" + " you don't care about the destination current status", reference, - repoUrl, + pushRepoUrl, GeneralOptions.FORCE)); } if (localBranchRevision != null) { @@ -468,7 +485,7 @@ public ImmutableList write(TransformResult transformResult, } else if (!skipPush) { // Should be a no-op, but an iterative migration could take several minutes between // migrations so lets fetch the latest first. - fetchFromRemote(console, scratchClone, repoUrl, remoteFetch); + fetchFromRemote(console, scratchClone, fetchRepoUrl, remoteFetch); } PathMatcher pathMatcher = destinationFiles.relativeTo(scratchClone.getWorkTree()); @@ -505,7 +522,7 @@ public ImmutableList write(TransformResult transformResult, commitMessage); // Don't remove. Used internally in test - console.verboseFmt("Integrates for %s: %s", repoUrl, Iterables.size(integrates)); + console.verboseFmt("Integrates for %s: %s", pushRepoUrl, Iterables.size(integrates)); for (GitIntegrateChanges integrate : integrates) { integrate.run(alternate, generalOptions, messageInfo, @@ -562,7 +579,7 @@ public ImmutableList write(TransformResult transformResult, console.info(DiffUtil.colorize( console, scratchClone.simpleCommand("show", "HEAD").getStdout())); if (!console.promptConfirmation( - String.format("Proceed with push to %s %s?", repoUrl, remotePush))) { + String.format("Proceed with push to %s %s?", pushRepoUrl, remotePush))) { console.warn("Migration aborted by user."); throw new ChangeRejectedException( "User aborted execution: did not confirm diff changes."); @@ -588,7 +605,7 @@ public ImmutableList write(TransformResult transformResult, new DestinationEffect.DestinationRef(head.getSha1(), "commit", /*url=*/ null))); } String push = writeHook.getPushReference(getCompleteRef(remotePush), transformResult); - console.progress(String.format("Git Destination: Pushing to %s %s", repoUrl, push)); + console.progress(String.format("Git Destination: Pushing to %s %s", pushRepoUrl, push)); checkCondition(!nonFastForwardPush || !Objects.equals(remoteFetch, remotePush), "non fast-forward push is only" + " allowed when fetch != push"); @@ -596,7 +613,7 @@ public ImmutableList write(TransformResult transformResult, String serverResponse = generalOptions.repoTask( "push", () -> scratchClone.push() - .withRefspecs(repoUrl, + .withRefspecs(pushRepoUrl, tagName != null ? ImmutableList.of(scratchClone.createRefSpec( (nonFastForwardPush ? "+" : "") + "HEAD:" + push), @@ -665,7 +682,7 @@ private void updateLocalBranchToBaseline(GitRepository repo, String baseline) + (getLocalBranchRevision(repo) != null ? "' from fetch reference '" + remoteFetch + "'" : "' and fetch reference '" + remoteFetch + "' itself") - + " in " + repoUrl + "."); + + " in " + fetchRepoUrl + "."); } else if (baseline != null) { // Update the local branch to use the baseline repo.simpleCommand("update-ref", state.localBranch, baseline); diff --git a/java/com/google/copybara/git/GitHubPrDestination.java b/java/com/google/copybara/git/GitHubPrDestination.java index 633a2d2c8..7bc35456d 100644 --- a/java/com/google/copybara/git/GitHubPrDestination.java +++ b/java/com/google/copybara/git/GitHubPrDestination.java @@ -54,6 +54,7 @@ import com.google.copybara.util.Identity; import com.google.copybara.util.console.Console; import java.io.IOException; +import java.util.Optional; import java.util.UUID; import javax.annotation.Nullable; @@ -63,6 +64,7 @@ public class GitHubPrDestination implements Destination { private final String url; + private final Optional forkUrl; private final String destinationRef; private final String prBranch; private final GeneralOptions generalOptions; @@ -82,6 +84,7 @@ public class GitHubPrDestination implements Destination { GitHubPrDestination( String url, String destinationRef, + Optional forkUrl, @Nullable String prBranch, GeneralOptions generalOptions, GitHubOptions gitHubOptions, @@ -96,6 +99,7 @@ public class GitHubPrDestination implements Destination { @Nullable Checker endpointChecker, boolean updateDescription) { this.url = Preconditions.checkNotNull(url); + this.forkUrl = Preconditions.checkNotNull(forkUrl); this.destinationRef = Preconditions.checkNotNull(destinationRef); this.prBranch = prBranch; this.generalOptions = Preconditions.checkNotNull(generalOptions); @@ -108,7 +112,7 @@ public class GitHubPrDestination implements Destination { this.title = title; this.body = body; this.updateDescription = updateDescription; - this.localRepo = memoized(ignored -> destinationOptions.localGitRepo(url)); + this.localRepo = memoized(ignored -> destinationOptions.localGitRepo(getPushUrl())); this.mainConfigFile = Preconditions.checkNotNull(mainConfigFile); this.endpointChecker = endpointChecker; } @@ -123,7 +127,7 @@ public ImmutableSetMultimap describe(Glob originFiles) { ImmutableSetMultimap.Builder builder = new ImmutableSetMultimap.Builder() .put("type", getType()) - .put("url", url) + .put("url", getFetchUrl()) .put("destination_ref", destinationRef); return builder.build(); } @@ -131,25 +135,28 @@ public ImmutableSetMultimap describe(Glob originFiles) { @Override public Writer newWriter(WriterContext writerContext) throws ValidationException { - String prBranch = - getPullRequestBranchName( + PrBranch prBranch = + new PrBranch( writerContext.getOriginalRevision(), writerContext.getWorkflowName(), - writerContext.getWorkflowIdentityUser()); + writerContext.getWorkflowIdentityUser(), + url, + forkUrl); GitHubWriterState state = new GitHubWriterState( localRepo, destinationOptions.localRepoPath != null - ? prBranch + ? prBranch.getLocalName() : "copybara/push-" + UUID.randomUUID() + (writerContext.isDryRun() ? "-dryrun" : "")); return new WriterImpl( writerContext.isDryRun(), - url, + getFetchUrl(), + getPushUrl(), destinationRef, - prBranch, + prBranch.getLocalName(), /*tagName*/null, /*tagMsg*/null, generalOptions, @@ -180,7 +187,7 @@ public ImmutableList write( console.infoFmt( "Please create a PR manually following this link: %s/compare/%s...%s" + " (Only needed once)", - asHttpsUrl(), destinationRef, prBranch); + getPushUrlAsHttps(), destinationRef, prBranch.getPrRef()); state.pullRequestNumber = -1L; return result.build(); } @@ -188,9 +195,7 @@ public ImmutableList write( GitHubApi api = gitHubOptions.newGitHubApi(getProjectName()); ImmutableList pullRequests = api.getPullRequests( - getProjectName(), - PullRequestListParams.DEFAULT - .withHead(String.format("%s:%s", getUserNameFromUrl(url), prBranch))); + getProjectName(), PullRequestListParams.DEFAULT.withHead(prBranch.getQualifiedPrRef())); ChangeMessage msg = ChangeMessage.parseMessage(transformResult.getSummary().trim()); @@ -206,10 +211,10 @@ public ImmutableList write( GitHubPrDestination.this.body, "body"); for (PullRequest pr : pullRequests) { - if (pr.isOpen() && pr.getHead().getRef().equals(prBranch)) { + if (pr.isOpen() && pr.getHead().getRef().equals(prBranch.getLocalName())) { console.infoFmt( "Pull request for branch %s already exists as %s/pull/%s", - prBranch, asHttpsUrl(), pr.getNumber()); + prBranch, getPushUrlAsHttps(), pr.getNumber()); if (!pr.getBase().getRef().equals(destinationRef)) { // TODO(malcon): Update PR or create a new one? console.warnFmt( @@ -245,10 +250,10 @@ public ImmutableList write( api.createPullRequest( getProjectName(), new CreatePullRequest( - title, prBody, prBranch, destinationRef)); + title, prBody, prBranch.getPrRef(), destinationRef)); console.infoFmt( "Pull Request %s/pull/%s created using branch '%s'.", - asHttpsUrl(), pr.getNumber(), prBranch); + getPushUrlAsHttps(), pr.getNumber(), prBranch.getPrRef()); state.pullRequestNumber = pr.getNumber(); result.add( new DestinationEffect( @@ -263,19 +268,28 @@ public ImmutableList write( @Override public Endpoint getFeedbackEndPoint(Console console) throws ValidationException { gitHubOptions.validateEndpointChecker(endpointChecker); + String url = getPushUrl(); return new GitHubEndPoint( gitHubOptions.newGitHubApiSupplier(url, endpointChecker), url, console); } }; } - private String asHttpsUrl() throws ValidationException { - return "https://github.com/" + getProjectName(); + private String getFetchUrl() { + return url; + } + + private String getPushUrl() { + return forkUrl.orElse(url); + } + + private String getPushUrlAsHttps() throws ValidationException { + return "https://github.com/" + GitHubUtil.getProjectNameFromUrl(getPushUrl()); } @VisibleForTesting String getProjectName() throws ValidationException { - return GitHubUtil.getProjectNameFromUrl(url); + return GitHubUtil.getProjectNameFromUrl(getFetchUrl()); } @VisibleForTesting @@ -288,31 +302,64 @@ public Iterable getIntegrates() { return integrates; } - private String getPullRequestBranchName( - @Nullable Revision changeRevision, String workflowName, String workflowIdentityUser) - throws ValidationException { - if (!Strings.isNullOrEmpty(gitHubDestinationOptions.destinationPrBranch)) { - return gitHubDestinationOptions.destinationPrBranch; + private class PrBranch { + private final String name; + private final String url; + private final Optional forkUrl; + + public PrBranch( + @Nullable Revision changeRevision, + String workflowName, + String workflowIdentityUser, + String url, + Optional forkUrl) + throws ValidationException { + this.name = getPullRequestBranchName(changeRevision, workflowName, workflowIdentityUser); + this.url = Preconditions.checkNotNull(url); + this.forkUrl = Preconditions.checkNotNull(forkUrl); + } + + private String getPullRequestBranchName( + @Nullable Revision changeRevision, String workflowName, String workflowIdentityUser) + throws ValidationException { + if (!Strings.isNullOrEmpty(gitHubDestinationOptions.destinationPrBranch)) { + return gitHubDestinationOptions.destinationPrBranch; + } + String contextReference = changeRevision.contextReference(); + // We could do more magic here with the change identity. But this is already complex so we + // require a group identity either provided by the origin or the workflow (Will be implemented + // later. + checkCondition(contextReference != null, + "git.github_pr_destination is incompatible with the current origin. Origin has to be" + + " able to provide the contextReference or use '%s' flag", + GitHubDestinationOptions.GITHUB_DESTINATION_PR_BRANCH); + String branchNameFromUser = getCustomBranchName(contextReference); + String branchName = + branchNameFromUser != null + ? branchNameFromUser + : Identity.computeIdentity( + "OriginGroupIdentity", + contextReference, + workflowName, + mainConfigFile.getIdentifier(), + workflowIdentityUser); + return GitHubUtil.getValidBranchName(branchName); + } + + public String getLocalName() { + return name; + } + + public String getPrRef() throws ValidationException { + if (!forkUrl.isPresent()) { + return getLocalName(); + } + return String.format("%s:%s", getUserNameFromUrl(forkUrl.get()), getLocalName()); + } + + public String getQualifiedPrRef() throws ValidationException { + return String.format("%s:%s", getUserNameFromUrl(forkUrl.orElse(url)), getLocalName()); } - String contextReference = changeRevision.contextReference(); - // We could do more magic here with the change identity. But this is already complex so we - // require a group identity either provided by the origin or the workflow (Will be implemented - // later. - checkCondition(contextReference != null, - "git.github_pr_destination is incompatible with the current origin. Origin has to be" - + " able to provide the contextReference or use '%s' flag", - GitHubDestinationOptions.GITHUB_DESTINATION_PR_BRANCH); - String branchNameFromUser = getCustomBranchName(contextReference); - String branchName = - branchNameFromUser != null - ? branchNameFromUser - : Identity.computeIdentity( - "OriginGroupIdentity", - contextReference, - workflowName, - mainConfigFile.getIdentifier(), - workflowIdentityUser); - return GitHubUtil.getValidBranchName(branchName); } @Nullable diff --git a/java/com/google/copybara/git/GitModule.java b/java/com/google/copybara/git/GitModule.java index 0d5642347..fd08b4458 100644 --- a/java/com/google/copybara/git/GitModule.java +++ b/java/com/google/copybara/git/GitModule.java @@ -89,6 +89,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.TreeMap; import javax.annotation.CheckReturnValue; import javax.annotation.Nullable; @@ -1199,6 +1200,24 @@ public GitDestination gitHubDestination( named = true, doc = "Destination reference for the change. By default 'master'", defaultValue = "\"master\""), + @Param( + name = "push_to_fork", + type = Boolean.class, + defaultValue = "False", + named = true, + positional = false, + doc = + "Indicates that the result of the change should be pushed to the current user's " + + "personal fork. The PullRequest will still be created on the upstream " + + "project."), + @Param( + name = "fork_url", + type = String.class, + defaultValue = "None", + noneable = true, + named = true, + positional = false, + doc = "TODO"), @Param( name = "pr_branch", type = String.class, @@ -1297,6 +1316,8 @@ public GitDestination gitHubDestination( public GitHubPrDestination githubPrDestination( String url, String destinationRef, + Boolean pushToFork, + Object forkUrl, Object prBranch, Object title, Object body, @@ -1309,12 +1330,18 @@ public GitHubPrDestination githubPrDestination( // This restricts to github.com, we will have to revisit this to support setups like GitHub // Enterprise. check(GitHubUtil.isGitHubUrl(url), "'%s' is not a valid GitHub url", url); + check(forkUrl == Starlark.NONE || GitHubUtil.isGitHubUrl((String)forkUrl), + "'%s' is not a valid GitHub url", forkUrl); GitDestinationOptions destinationOptions = options.get(GitDestinationOptions.class); return new GitHubPrDestination( fixHttp( checkNotEmpty(firstNotNull(destinationOptions.url, url), "url"), thread.getCallerLocation()), destinationRef, + forkUrl == Starlark.NONE ? + Optional.empty() : + Optional.of( + fixHttp(checkNotEmpty((String)forkUrl, "fork_url"), thread.getCallerLocation())), convertFromNoneable(prBranch, null), generalOptions, options.get(GitHubOptions.class), diff --git a/third_party/BUILD b/third_party/BUILD index aff8566c7..d456baa8d 100644 --- a/third_party/BUILD +++ b/third_party/BUILD @@ -15,7 +15,7 @@ package( java_library( name = "guava", exports = [ - "@maven//:com_google_guava_failureaccess", + "@maven//:com_google_guava_failureaccess", "@maven//:com_google_guava_guava", ], ) @@ -95,18 +95,18 @@ java_library( name = "truth", testonly = 1, exports = [ - "@maven//:com_googlecode_java_diff_utils_diffutils", "@maven//:com_google_truth_truth", + "@maven//:com_googlecode_java_diff_utils_diffutils", ], ) java_library( name = "google_http_client", exports = [ + "@maven//:com_google_code_gson_gson", "@maven//:com_google_http_client_google_http_client", "@maven//:com_google_http_client_google_http_client_gson", - "@maven//:com_google_code_gson_gson", - "@maven//:commons_codec_commons_codec", + "@maven//:commons_codec_commons_codec", ], ) @@ -152,8 +152,8 @@ java_library( "@maven//:com_google_flogger_flogger", ], runtime_deps = [ - "@maven//:com_google_flogger_flogger_system_backend" - ] + "@maven//:com_google_flogger_flogger_system_backend", + ], ) java_library( @@ -161,11 +161,9 @@ java_library( testonly = 1, exports = [ "//java/com/google/copybara/hg/testing", - ] + ], ) # Required temporarily until @io_bazel//src/main/java/com/google/devtools/build/lib/syntax/... # is fixed exports_files(["bazel.patch"]) - -