Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow creating PullRequest on a different repo than pushing #120

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow creating PullRequest on a different repo than pushing #120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion docs/reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2132,7 +2132,7 @@ version_selector | `latestVersionSelector`<br><p>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:
Expand All @@ -2141,6 +2141,8 @@ Parameter | Description
--------- | -----------
url | `string`<br><p>Url of the GitHub project. For example "https://github.com/google/copybara'"</p>
destination_ref | `string`<br><p>Destination reference for the change. By default 'master'</p>
push_to_fork | `boolean`<br><p>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.<p>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`).</p><p>If `fork_url` is set, this will be ignored.</p></p>
fork_url | `string`<br><p>Sets the url of the fork Copybara will push the change to. The PullRequest will still be created on the upstream project.<p>`push_to_fork` is ignored if this is set.</p></p>
pr_branch | `string`<br><p>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.).</p>
title | `string`<br><p>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}"`</p>
body | `string`<br><p>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}"`</p>
Expand Down
166 changes: 123 additions & 43 deletions java/com/google/copybara/git/GitDestination.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,8 @@ public static class WriterImpl<S extends WriterState>
implements Writer<GitRevision> {

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;
Expand All @@ -218,18 +219,75 @@ public static class WriterImpl<S extends WriterState>
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<GitIntegrateChanges> 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<GitIntegrateChanges> 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<GitIntegrateChanges> 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;
Expand All @@ -251,6 +309,16 @@ public static class WriterImpl<S extends WriterState>
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 {
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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));
}
}

Expand Down Expand Up @@ -436,7 +506,8 @@ public ImmutableList<DestinationEffect> 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);
Expand All @@ -450,13 +521,13 @@ public ImmutableList<DestinationEffect> 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);
Expand All @@ -468,7 +539,7 @@ public ImmutableList<DestinationEffect> 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());
Expand Down Expand Up @@ -505,7 +576,7 @@ public ImmutableList<DestinationEffect> 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,
Expand Down Expand Up @@ -562,7 +633,7 @@ public ImmutableList<DestinationEffect> 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.");
Expand All @@ -588,24 +659,29 @@ public ImmutableList<DestinationEffect> 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);
}

Expand Down Expand Up @@ -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);
Expand Down
Loading