From ae4c487a33a195d9ec122af19d2fc10d6acfbb54 Mon Sep 17 00:00:00 2001 From: Yannic Bonenberger Date: Fri, 20 Mar 2020 18:01:31 +0100 Subject: [PATCH] Export of internal change --- docs/reference.md | 4 +- .../google/copybara/git/GitDestination.java | 166 ++++++++---- .../copybara/git/GitHubPrDestination.java | 237 ++++++++++++------ java/com/google/copybara/git/GitModule.java | 38 +++ .../copybara/git/github/util/GitHubUtil.java | 14 ++ .../copybara/git/GitHubPrDestinationTest.java | 74 ++++++ .../git/github/util/GitHubUtilTest.java | 21 ++ third_party/BUILD | 16 +- 8 files changed, 446 insertions(+), 124 deletions(-) diff --git a/docs/reference.md b/docs/reference.md index 44bf7a3ee..e91f046e6 100755 --- a/docs/reference.md +++ b/docs/reference.md @@ -2132,7 +2132,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: @@ -2141,6 +2141,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.

The url of the fork is inferred from `url` and the credentials of the GitHub user (e.g., if `url` is `https://github.com/google/copybara.git` and the workflow is executed by `copybara-bot`, `fork_url` is assumed to be `https://github.com/copybara-bot/copybara.git`).

If `fork_url` is set, this will be ignored.

+fork_url | `string`

Sets the url of the fork Copybara will push the change to. The PullRequest will still be created on the upstream project.

`push_to_fork` is ignored if this is set.

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..b05dbf91a 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,18 +219,75 @@ public static class WriterImpl private final int visitChangePageSize; private final boolean gitTagOverwrite; - /** - * Create a new git.destination writer - */ - 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, + @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 fetchRepoUrl, + String pushRepoUrl, + 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 = skipPush; - this.repoUrl = checkNotNull(repoUrl); + this.fetchRepoUrl = checkNotNull(fetchRepoUrl); + this.pushRepoUrl = checkNotNull(pushRepoUrl); this.remoteFetch = checkNotNull(remoteFetch); this.remotePush = checkNotNull(remotePush); this.tagNameTemplate = tagNameTemplate; @@ -251,6 +309,16 @@ public static class WriterImpl this.gitTagOverwrite = gitTagOverwrite; } + @VisibleForTesting + String getFetchRepoUrl() { + return fetchRepoUrl; + } + + @VisibleForTesting + String getPushRepoUrl() { + return pushRepoUrl; + } + @Override public void visitChanges(@Nullable GitRevision start, ChangesVisitor visitor) throws RepoException, ValidationException { @@ -284,7 +352,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, getFetchRepoUrl(), remoteFetch); if (revision != null) { repo.simpleCommand("branch", state.localBranch, revision.getSha1()); } @@ -343,8 +411,10 @@ private GitRevision getLocalBranchRevision(GitRepository gitRepository) throws R if (force) { return null; } - throw new RepoException(String.format("Could not find %s in %s and '%s' was not used", - remoteFetch, repoUrl, GeneralOptions.FORCE)); + throw new RepoException( + String.format( + "Could not find %s in %s and '%s' was not used", + remoteFetch, getFetchRepoUrl(), GeneralOptions.FORCE)); } } @@ -436,7 +506,8 @@ 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(), getPushRepoUrl(), remotePush); String baseline = transformResult.getBaseline(); GitRepository scratchClone = getRepository(console); @@ -450,13 +521,13 @@ public ImmutableList write(TransformResult transformResult, if (state.firstWrite) { String reference = baseline != null ? baseline : state.localBranch; - configForPush(getRepository(console), repoUrl, remotePush); + configForPush(getRepository(console), getPushRepoUrl(), 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, - GeneralOptions.FORCE)); + 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, getPushRepoUrl(), GeneralOptions.FORCE)); } if (localBranchRevision != null) { scratchClone.simpleCommand("checkout", "-f", "-q", reference); @@ -468,7 +539,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, getFetchRepoUrl(), remoteFetch); } PathMatcher pathMatcher = destinationFiles.relativeTo(scratchClone.getWorkTree()); @@ -505,7 +576,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", getPushRepoUrl(), Iterables.size(integrates)); for (GitIntegrateChanges integrate : integrates) { integrate.run(alternate, generalOptions, messageInfo, @@ -562,7 +633,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?", getPushRepoUrl(), remotePush))) { console.warn("Migration aborted by user."); throw new ChangeRejectedException( "User aborted execution: did not confirm diff changes."); @@ -588,24 +659,29 @@ 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", getPushRepoUrl(), push)); checkCondition(!nonFastForwardPush || !Objects.equals(remoteFetch, remotePush), "non fast-forward push is only" + " allowed when fetch != push"); - String serverResponse = generalOptions.repoTask( - "push", - () -> scratchClone.push() - .withRefspecs(repoUrl, - tagName != null - ? ImmutableList.of(scratchClone.createRefSpec( - (nonFastForwardPush ? "+" : "") + "HEAD:" + push), - scratchClone.createRefSpec((gitTagOverwrite ? "+" : "") - + tagName)) - : ImmutableList.of(scratchClone.createRefSpec( - (nonFastForwardPush ? "+" : "") + "HEAD:" + push))) - .run() - ); + String serverResponse = + generalOptions.repoTask( + "push", + () -> + scratchClone + .push() + .withRefspecs( + getPushRepoUrl(), + tagName != null + ? ImmutableList.of( + scratchClone.createRefSpec( + (nonFastForwardPush ? "+" : "") + "HEAD:" + push), + scratchClone.createRefSpec( + (gitTagOverwrite ? "+" : "") + tagName)) + : ImmutableList.of( + scratchClone.createRefSpec( + (nonFastForwardPush ? "+" : "") + "HEAD:" + push))) + .run()); return writeHook.afterPush(serverResponse, messageInfo, head, originChanges); } @@ -661,11 +737,15 @@ public GitRepository getRepository(Console console) throws RepoException, Valida private void updateLocalBranchToBaseline(GitRepository repo, String baseline) throws RepoException { if (baseline != null && !repo.refExists(baseline)) { - throw new RepoException("Cannot find baseline '" + baseline - + (getLocalBranchRevision(repo) != null - ? "' from fetch reference '" + remoteFetch + "'" - : "' and fetch reference '" + remoteFetch + "' itself") - + " in " + repoUrl + "."); + throw new RepoException( + "Cannot find baseline '" + + baseline + + (getLocalBranchRevision(repo) != null + ? "' from fetch reference '" + remoteFetch + "'" + : "' and fetch reference '" + remoteFetch + "' itself") + + " in " + + getFetchRepoUrl() + + "."); } 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..08fc5e991 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; @@ -61,8 +62,12 @@ * A destination for creating/updating Github Pull Requests. */ public class GitHubPrDestination implements Destination { + private static final String CANNOT_INFER_FORK_URL_MESSAGE = + "Could not infer fork url. Please set 'fork_url'."; private final String url; + private final boolean pushToFork; + private final Optional forkUrl; private final String destinationRef; private final String prBranch; private final GeneralOptions generalOptions; @@ -82,6 +87,8 @@ public class GitHubPrDestination implements Destination { GitHubPrDestination( String url, String destinationRef, + boolean pushToFork, + Optional forkUrl, @Nullable String prBranch, GeneralOptions generalOptions, GitHubOptions gitHubOptions, @@ -96,6 +103,8 @@ public class GitHubPrDestination implements Destination { @Nullable Checker endpointChecker, boolean updateDescription) { this.url = Preconditions.checkNotNull(url); + this.pushToFork = pushToFork; + this.forkUrl = Preconditions.checkNotNull(forkUrl); this.destinationRef = Preconditions.checkNotNull(destinationRef); this.prBranch = prBranch; this.generalOptions = Preconditions.checkNotNull(generalOptions); @@ -108,7 +117,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(getFetchUrl())); this.mainConfigFile = Preconditions.checkNotNull(mainConfigFile); this.endpointChecker = endpointChecker; } @@ -123,35 +132,45 @@ public ImmutableSetMultimap describe(Glob originFiles) { ImmutableSetMultimap.Builder builder = new ImmutableSetMultimap.Builder() .put("type", getType()) - .put("url", url) - .put("destination_ref", destinationRef); + .put("url", getFetchUrl()) + .put("push_to_fork", (pushToFork || forkUrl.isPresent()) ? "True" : "False"); + if (forkUrl.isPresent()) { + builder.put("fork_url", forkUrl.get()); + } + builder.put("destination_ref", destinationRef); return builder.build(); } @Override public Writer newWriter(WriterContext writerContext) throws ValidationException { - - String prBranch = - getPullRequestBranchName( + LazyResourceLoader githubApi = + memoized(gitHubOptions.newGitHubApiSupplier(getFetchUrl(), null)); + Optional pushUrl = pushUrl(githubApi); + PrBranch prBranch = + new PrBranch( writerContext.getOriginalRevision(), writerContext.getWorkflowName(), - writerContext.getWorkflowIdentityUser()); - - GitHubWriterState state = new GitHubWriterState( - localRepo, - destinationOptions.localRepoPath != null - ? prBranch - : "copybara/push-" - + UUID.randomUUID() - + (writerContext.isDryRun() ? "-dryrun" : "")); + writerContext.getWorkflowIdentityUser(), + url, + pushUrl); + + GitHubWriterState state = + new GitHubWriterState( + localRepo, + destinationOptions.localRepoPath != null + ? prBranch.getLocalName() + : "copybara/push-" + + UUID.randomUUID() + + (writerContext.isDryRun() ? "-dryrun" : "")); return new WriterImpl( writerContext.isDryRun(), - url, + getFetchUrl(), + pushUrl.orElse(getFetchUrl()), destinationRef, - prBranch, - /*tagName*/null, - /*tagMsg*/null, + prBranch.getLocalName(), + /*tagName*/ null, + /*tagMsg*/ null, generalOptions, writeHook, state, @@ -180,36 +199,36 @@ public ImmutableList write( console.infoFmt( "Please create a PR manually following this link: %s/compare/%s...%s" + " (Only needed once)", - asHttpsUrl(), destinationRef, prBranch); + asGithubHttpsUrl(pushUrl), destinationRef, prBranch.getPrRef()); state.pullRequestNumber = -1L; return result.build(); } - GitHubApi api = gitHubOptions.newGitHubApi(getProjectName()); - - ImmutableList pullRequests = api.getPullRequests( - getProjectName(), - PullRequestListParams.DEFAULT - .withHead(String.format("%s:%s", getUserNameFromUrl(url), prBranch))); - ChangeMessage msg = ChangeMessage.parseMessage(transformResult.getSummary().trim()); - String title = GitHubPrDestination.this.title == null - ? msg.firstLine() - : SkylarkUtil.mapLabels(transformResult.getLabelFinder(), - GitHubPrDestination.this.title, "title"); + String title = + GitHubPrDestination.this.title == null + ? msg.firstLine() + : SkylarkUtil.mapLabels( + transformResult.getLabelFinder(), GitHubPrDestination.this.title, "title"); String prBody = GitHubPrDestination.this.body == null ? msg.toString() - : SkylarkUtil.mapLabels(transformResult.getLabelFinder(), - GitHubPrDestination.this.body, "body"); - + : SkylarkUtil.mapLabels( + transformResult.getLabelFinder(), GitHubPrDestination.this.body, "body"); + + ImmutableList pullRequests = + githubApi + .load(console) + .getPullRequests( + getProjectName(), + PullRequestListParams.DEFAULT.withHead(prBranch.getQualifiedPrRef())); 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.getPrRef(), asGithubHttpsUrl(pushUrl), pr.getNumber()); if (!pr.getBase().getRef().equals(destinationRef)) { // TODO(malcon): Update PR or create a new one? console.warnFmt( @@ -221,9 +240,12 @@ public ImmutableList write( !Strings.isNullOrEmpty(title), "Pull Request title cannot be empty. Either use 'title' field in" + " git.github_pr_destination or modify the message to not be empty"); - api.updatePullRequest(getProjectName(), pr.getNumber(), - new UpdatePullRequest(title, prBody, /*state=*/null)); - + githubApi + .load(console) + .updatePullRequest( + getProjectName(), + pr.getNumber(), + new UpdatePullRequest(title, prBody, /*state=*/ null)); } result.add( new DestinationEffect( @@ -242,13 +264,14 @@ public ImmutableList write( + " git.github_pr_destination or modify the message to not be empty"); PullRequest pr = - api.createPullRequest( - getProjectName(), - new CreatePullRequest( - title, prBody, prBranch, destinationRef)); + githubApi + .load(console) + .createPullRequest( + getProjectName(), + new CreatePullRequest(title, prBody, prBranch.getPrRef(), destinationRef)); console.infoFmt( "Pull Request %s/pull/%s created using branch '%s'.", - asHttpsUrl(), pr.getNumber(), prBranch); + asGithubHttpsUrl(pushUrl), pr.getNumber(), prBranch.getPrRef()); state.pullRequestNumber = pr.getNumber(); result.add( new DestinationEffect( @@ -263,19 +286,48 @@ public ImmutableList write( @Override public Endpoint getFeedbackEndPoint(Console console) throws ValidationException { gitHubOptions.validateEndpointChecker(endpointChecker); + String url = pushUrl.orElse(getFetchUrl()); return new GitHubEndPoint( gitHubOptions.newGitHubApiSupplier(url, endpointChecker), url, console); } }; } - private String asHttpsUrl() throws ValidationException { - return "https://github.com/" + getProjectName(); + @VisibleForTesting + Optional pushUrl(LazyResourceLoader api) throws ValidationException { + if (forkUrl.isPresent()) { + return forkUrl; + } + if (pushToFork) { + String repoName = GitHubUtil.getRepoNameFromUrl(getFetchUrl()); + if (!Strings.isNullOrEmpty(repoName)) { + try { + String username = api.load(generalOptions.console()).getAuthenticatedUser().getLogin(); + return Optional.of(asGithubHttpsUrl(username + "/" + repoName)); + } catch (RepoException e) { + throw new ValidationException(CANNOT_INFER_FORK_URL_MESSAGE, e); + } + } + throw new ValidationException(CANNOT_INFER_FORK_URL_MESSAGE); + } + return Optional.empty(); + } + + private String getFetchUrl() { + return url; + } + + private String asGithubHttpsUrl(String project) { + return "https://github.com/" + project; + } + + private String asGithubHttpsUrl(Optional pushUrl) throws ValidationException { + return asGithubHttpsUrl(GitHubUtil.getProjectNameFromUrl(pushUrl.orElse(getFetchUrl()))); } @VisibleForTesting String getProjectName() throws ValidationException { - return GitHubUtil.getProjectNameFromUrl(url); + return GitHubUtil.getProjectNameFromUrl(getFetchUrl()); } @VisibleForTesting @@ -288,31 +340,74 @@ 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 getUpstreamUrl() { + return url; + } + + public String getLocalName() { + return name; + } + + private String qualifiedName(String url) throws ValidationException { + return String.format("%s:%s", getUserNameFromUrl(url), getLocalName()); + } + + public String getPrRef() throws ValidationException { + if (!forkUrl.isPresent()) { + return getLocalName(); + } + return qualifiedName(forkUrl.get()); + } + + public String getQualifiedPrRef() throws ValidationException { + return qualifiedName(forkUrl.orElse(url)); } - 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..e5d407716 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,32 @@ 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." + + "

The url of the fork is inferred from `url` and the credentials of the " + + "GitHub user (e.g., if `url` is `https://github.com/google/copybara.git` and " + + "the workflow is executed by `copybara-bot`, `fork_url` is assumed to be " + + "`https://github.com/copybara-bot/copybara.git`).

" + + "

If `fork_url` is set, this will be ignored.

"), + @Param( + name = "fork_url", + type = String.class, + defaultValue = "None", + noneable = true, + named = true, + positional = false, + doc = + "Sets the url of the fork Copybara will push the change to. " + + "The PullRequest will still be created on the upstream project." + + "

`push_to_fork` is ignored if this is set.

"), @Param( name = "pr_branch", type = String.class, @@ -1297,6 +1324,8 @@ public GitDestination gitHubDestination( public GitHubPrDestination githubPrDestination( String url, String destinationRef, + Boolean pushToFork, + Object forkUrl, Object prBranch, Object title, Object body, @@ -1309,12 +1338,21 @@ 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, + pushToFork, + 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/java/com/google/copybara/git/github/util/GitHubUtil.java b/java/com/google/copybara/git/github/util/GitHubUtil.java index 675377fd8..07dc31674 100644 --- a/java/com/google/copybara/git/github/util/GitHubUtil.java +++ b/java/com/google/copybara/git/github/util/GitHubUtil.java @@ -51,6 +51,20 @@ public static String getUserNameFromUrl(String url) throws ValidationException { return i == -1 ? project : project.substring(0, i); } + /** + * Return the repository part of a github url. For example in https://github.com/foo/bar/baz, + * 'bar' would be the repository. + */ + public static String getRepoNameFromUrl(String url) throws ValidationException { + String project = getProjectNameFromUrl(url); + int start = project.indexOf("/"); + if (start < 0) { + throw new ValidationException("Cannot find repo name from url " + url); + } + int end = project.indexOf("/", start + 1); + return end < 0 ? project.substring(start + 1) : project.substring(start + 1, end); + } + /** * Given a url that represents a GitHub repository, return the project name. */ diff --git a/javatests/com/google/copybara/git/GitHubPrDestinationTest.java b/javatests/com/google/copybara/git/GitHubPrDestinationTest.java index b1c5cd04a..560c2cc28 100644 --- a/javatests/com/google/copybara/git/GitHubPrDestinationTest.java +++ b/javatests/com/google/copybara/git/GitHubPrDestinationTest.java @@ -42,6 +42,8 @@ import com.google.copybara.WriterContext; import com.google.copybara.exception.RepoException; import com.google.copybara.exception.ValidationException; +import com.google.copybara.git.GitDestination.WriterImpl; +import com.google.copybara.git.GitDestination.WriterState; import com.google.copybara.git.GitIntegrateChanges.Strategy; import com.google.copybara.git.GitRepository.GitLogEntry; import com.google.copybara.testing.DummyRevision; @@ -60,6 +62,7 @@ import java.nio.file.Path; import java.util.Map; import java.util.Map.Entry; +import java.util.Optional; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -530,6 +533,77 @@ public void testWriteNoMaster() throws ValidationException, IOException, RepoExc + "DummyOrigin-RevId: one\n"); } + @Test + public void testForkUrl() throws Exception { + { + GitHubPrDestination d = + skylark.eval( + "r", "r = git.github_pr_destination(" + " url = 'https://github.com/foo'," + ")"); + Optional forkUrl = d.pushUrl(null); + assertThat(forkUrl.isPresent()).isFalse(); + } + + { + GitHubPrDestination d = + skylark.eval( + "r", + "r = git.github_pr_destination(" + + " url = 'https://github.com/foo'," + + " fork_url = 'https://github.com/bar'," + + ")"); + Optional forkUrl = d.pushUrl(null); + assertThat(forkUrl.isPresent()).isTrue(); + assertThat(forkUrl.get()).isEqualTo("https://github.com/bar"); + } + + { + GitHubPrDestination d = + skylark.eval( + "r", + "r = git.github_pr_destination(" + + " url = 'https://github.com/foo'," + + " push_to_fork = True," + + " fork_url = 'https://github.com/bar'," + + ")"); + Optional forkUrl = d.pushUrl(null); + assertThat(forkUrl.isPresent()).isTrue(); + assertThat(forkUrl.get()).isEqualTo("https://github.com/bar"); + } + + { + // fork_url wins over push_to_fork. + GitHubPrDestination d = + skylark.eval( + "r", + "r = git.github_pr_destination(" + + " url = 'https://github.com/foo'," + + " push_to_fork = False," + + " fork_url = 'https://github.com/bar'," + + ")"); + Optional forkUrl = d.pushUrl(null); + assertThat(forkUrl.isPresent()).isTrue(); + assertThat(forkUrl.get()).isEqualTo("https://github.com/bar"); + } + } + + @Test + public void testPushToForkUrl() throws Exception { + GitHubPrDestination d = + skylark.eval( + "r", + "r = git.github_pr_destination(" + + " url = 'https://github.com/foo'," + + " fork_url = 'https://github.com/bar'," + + ")"); + DummyRevision dummyRevision = new DummyRevision("dummyReference", "feature"); + WriterContext writerContext = + new WriterContext( + "piper_to_github_pr", "TEST", false, dummyRevision, Glob.ALL_FILES.roots()); + WriterImpl writer = (WriterImpl) d.newWriter(writerContext); + assertThat(writer.getFetchRepoUrl()).isEqualTo("https://github.com/foo"); + assertThat(writer.getPushRepoUrl()).isEqualTo("https://github.com/bar"); + } + @Test public void testBranchNameFromUserWithLabel() throws ValidationException, IOException, RepoException { testBranchNameFromUser("test${CONTEXT_REFERENCE}", "test_feature", "&feature"); diff --git a/javatests/com/google/copybara/git/github/util/GitHubUtilTest.java b/javatests/com/google/copybara/git/github/util/GitHubUtilTest.java index 0911c389b..ab3679551 100644 --- a/javatests/com/google/copybara/git/github/util/GitHubUtilTest.java +++ b/javatests/com/google/copybara/git/github/util/GitHubUtilTest.java @@ -18,6 +18,7 @@ import static com.google.common.truth.Truth.assertThat; import static com.google.copybara.git.github.util.GitHubUtil.getProjectNameFromUrl; +import static com.google.copybara.git.github.util.GitHubUtil.getRepoNameFromUrl; import static com.google.copybara.git.github.util.GitHubUtil.getUserNameFromUrl; import static com.google.copybara.git.github.util.GitHubUtil.getValidBranchName; import static org.junit.Assert.assertThrows; @@ -65,6 +66,26 @@ public void testGetUserNameFromUrl() throws Exception { assertThat(e).hasMessageThat().contains("Cannot find project name"); } + private void assertCannotFindRepoName(String url) throws Exception { + ValidationException e = assertThrows(ValidationException.class, () -> getRepoNameFromUrl(url)); + assertThat(e).hasMessageThat().contains("Cannot find repo name"); + } + + @Test + public void testGetRepoNameFromUrl() throws Exception { + assertThat(getRepoNameFromUrl("https://github.com/foo/bar")).isEqualTo("bar"); + assertThat(getRepoNameFromUrl("https://github.com/foo/bar.git")).isEqualTo("bar"); + assertThat(getRepoNameFromUrl("https://github.com/foo/bar/baz")).isEqualTo("bar"); + assertThat(getRepoNameFromUrl("ssh://git@github.com/foo/bar.git")).isEqualTo("bar"); + assertThat(getRepoNameFromUrl("git@github.com/foo/bar.git")).isEqualTo("bar"); + assertThat(getRepoNameFromUrl("git@github.com:foo/bar.git")).isEqualTo("bar"); + assertCannotFindRepoName("https://github.com/foo"); + assertCannotFindRepoName("https://github.com/foo.git"); + ValidationException e = + assertThrows(ValidationException.class, () -> getRepoNameFromUrl("foo@bar:baz")); + assertThat(e).hasMessageThat().contains("Cannot find project name"); + } + @Test public void testGetValidBranchName() throws ValidationException { assertThat(getValidBranchName("test/cl_1234")).isEqualTo("test/cl_1234"); 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"]) - -