Skip to content

Commit 11c1f20

Browse files
martyrubinmartyrubin
committed
Allow --force to override use_merge
When a GitHub PR workflow run with '--force' or '--github-force-import' tries to import a GitHub 'refs/pull/<PR ID>/merge' commit, Copybara now detects whether such a '.../merge' commit actually exists; if it doesn't, Copybara will import the PR branch from '.../head' instead. Previously, Copybara would have raised an error in such a case, and a Copybara user would have had to rerun Copybara with '--github-pr-merge false' in order to achieve this behavior. PiperOrigin-RevId: 371752943 Change-Id: I93586397580e658034fab9d355c4b06302f7261a
1 parent 8af6364 commit 11c1f20

4 files changed

Lines changed: 102 additions & 34 deletions

File tree

‎docs/reference.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2370,6 +2370,7 @@ Implicit labels that can be used/exposed:
23702370
- COPYBARA_INTEGRATE_REVIEW: A label that when exposed, can be used to integrate automatically in the reverse workflow.
23712371
- GITHUB_BASE_BRANCH: The name of the branch which serves as the base for the Pull Request.
23722372
- GITHUB_BASE_BRANCH_SHA1: The SHA-1 of the commit used as baseline. Generally, the baseline commit is the point of divergence between the PR's 'base' and 'head' branches. When `use_merge = True` is specified, the baseline is instead the tip of the PR's base branch.
2373+
- GITHUB_PR_USE_MERGE: Equal to 'true' if the workflow is importing a GitHub PR 'merge' commit and 'false' when importing a GitHub PR 'head' commit.
23732374
- GITHUB_PR_TITLE: Title of the Pull Request.
23742375
- GITHUB_PR_BODY: Body of the Pull Request.
23752376
- GITHUB_PR_URL: GitHub url of the Pull Request.

‎java/com/google/copybara/git/GitHubPROrigin.java‎

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ public class GitHubPROrigin implements Origin<GitRevision> {
9191
public static final String GITHUB_PR_NUMBER_LABEL = "GITHUB_PR_NUMBER";
9292
public static final String GITHUB_BASE_BRANCH = "GITHUB_BASE_BRANCH";
9393
public static final String GITHUB_BASE_BRANCH_SHA1 = "GITHUB_BASE_BRANCH_SHA1";
94+
public static final String GITHUB_PR_USE_MERGE = "GITHUB_PR_USE_MERGE";
9495
public static final String GITHUB_PR_TITLE = "GITHUB_PR_TITLE";
9596
public static final String GITHUB_PR_URL = "GITHUB_PR_URL";
9697
public static final String GITHUB_PR_BODY = "GITHUB_PR_BODY";
@@ -274,8 +275,9 @@ private GitRevision getRevisionForPR(String project, PullRequest prData)
274275
Set<String> requiredLabels = gitHubPrOriginOptions.getRequiredLabels(requiredLabelsField);
275276
Set<String> retryableLabels = gitHubPrOriginOptions.getRetryableLabels(retryableLabelsField);
276277
int prNumber = (int)prData.getNumber();
278+
boolean actuallyUseMerge = this.useMerge;
277279

278-
if (!gitHubPrOriginOptions.forceImport && !requiredLabels.isEmpty()) {
280+
if (!forceImport() && !requiredLabels.isEmpty()) {
279281
int retryCount = 0;
280282
Set<String> requiredButNotPresent;
281283
do {
@@ -311,8 +313,7 @@ private GitRevision getRevisionForPR(String project, PullRequest prData)
311313

312314
// check if check runs are ready if applicable
313315
checkRequiredCheckRuns(api, project, prData);
314-
if (!gitHubPrOriginOptions.forceImport
315-
&& branch != null && !Objects.equals(prData.getBase().getRef(), branch)) {
316+
if (!forceImport() && branch != null && !Objects.equals(prData.getBase().getRef(), branch)) {
316317
throw new EmptyChangeException(String.format(
317318
"Cannot migrate http://github.com/%s/pull/%d because its base branch is '%s', but"
318319
+ " the workflow is configured to only migrate changes for branch '%s'",
@@ -325,7 +326,7 @@ private GitRevision getRevisionForPR(String project, PullRequest prData)
325326
ImmutableList<Review> reviews = api.getReviews(project, prNumber);
326327
ApproverState shouldMigrate =
327328
reviewState.shouldMigrate(reviews, reviewApprovers, prData.getHead().getSha());
328-
if (!gitHubPrOriginOptions.forceImport && !shouldMigrate.shouldMigrate()) {
329+
if (!forceImport() && !shouldMigrate.shouldMigrate()) {
329330
String rejected = "";
330331
if (!shouldMigrate.rejectedReviews().isEmpty()) {
331332
rejected = String.format("\nThe following reviews were ignored because they don't meet "
@@ -353,13 +354,11 @@ private GitRevision getRevisionForPR(String project, PullRequest prData)
353354
labels.putAll(GITHUB_PR_REVIEWER_OTHER, others);
354355
}
355356

356-
if (!gitHubPrOriginOptions.forceImport
357-
&& requiredState == StateFilter.OPEN && !prData.isOpen()) {
357+
if (!forceImport() && requiredState == StateFilter.OPEN && !prData.isOpen()) {
358358
throw new EmptyChangeException(String.format("Pull Request %d is not open", prNumber));
359359
}
360360

361-
if (!gitHubPrOriginOptions.forceImport
362-
&& requiredState == StateFilter.CLOSED && prData.isOpen()) {
361+
if (!forceImport() && requiredState == StateFilter.CLOSED && prData.isOpen()) {
363362
throw new EmptyChangeException(String.format("Pull Request %d is open", prNumber));
364363
}
365364

@@ -372,23 +371,31 @@ private GitRevision getRevisionForPR(String project, PullRequest prData)
372371
// Prefix the branch name with 'refs/heads/' since some implementations of
373372
// GitRepository need the whole reference name.
374373
.add(String.format("refs/heads/%s:" + LOCAL_PR_BASE_BRANCH, prData.getBase().getRef()));
375-
if (useMerge) {
376-
if (prData.isMergeable() == null) {
374+
375+
if (actuallyUseMerge) {
376+
if (Boolean.TRUE.equals(prData.isMergeable())) {
377+
refSpecBuilder.add(String.format("%s:%s", asMergeRef(prNumber), LOCAL_PR_MERGE_REF));
378+
} else if (forceImport()) {
379+
console.warnFmt(
380+
"PR %d is not mergeable, but continuing with PR Head instead because of %s",
381+
prNumber,
382+
GeneralOptions.FORCE);
383+
actuallyUseMerge = false;
384+
} else if (prData.isMergeable() == null) {
377385
throw new CannotResolveRevisionException(
378386
String.format(
379387
"Cannot find a merge reference for Pull Request %d."
380388
+ " GitHub might still be generating it.",
381389
prNumber));
382-
} else if (!prData.isMergeable()) {
390+
} else {
383391
throw new CannotResolveRevisionException(
384392
String.format(
385393
"Cannot find a merge reference for Pull Request %d."
386394
+ " It might have a conflict with head.",
387395
prNumber));
388-
} else {
389-
refSpecBuilder.add(String.format("%s:%s", asMergeRef(prNumber), LOCAL_PR_MERGE_REF));
390396
}
391397
}
398+
392399
ImmutableList<String> refspec = refSpecBuilder.build();
393400
try (ProfilerTask ignore = generalOptions.profiler().start("fetch")) {
394401
getRepository()
@@ -399,7 +406,7 @@ private GitRevision getRevisionForPR(String project, PullRequest prData)
399406
refspec,
400407
partialFetch);
401408
} catch (CannotResolveRevisionException e) {
402-
if (useMerge) {
409+
if (actuallyUseMerge) {
403410
throw new CannotResolveRevisionException(
404411
String.format("Cannot find a merge reference for Pull Request %d, even though GitHub"
405412
+ " reported that this merge reference should exist.", prNumber), e);
@@ -409,7 +416,7 @@ private GitRevision getRevisionForPR(String project, PullRequest prData)
409416
}
410417
}
411418

412-
String refForMigration = useMerge ? LOCAL_PR_MERGE_REF : LOCAL_PR_HEAD_REF;
419+
String refForMigration = actuallyUseMerge ? LOCAL_PR_MERGE_REF : LOCAL_PR_HEAD_REF;
413420
GitRevision gitRevision = getRepository().resolveReference(refForMigration);
414421

415422
String headPrSha1 = getRepository().resolveReference(LOCAL_PR_HEAD_REF).getSha1();
@@ -427,6 +434,7 @@ private GitRevision getRevisionForPR(String project, PullRequest prData)
427434
labels.put(GitModule.DEFAULT_INTEGRATE_LABEL, integrateLabel);
428435
labels.put(GITHUB_BASE_BRANCH, prData.getBase().getRef());
429436
labels.put(GITHUB_PR_HEAD_SHA, headPrSha1);
437+
labels.put(GITHUB_PR_USE_MERGE, Boolean.toString(actuallyUseMerge));
430438

431439
String mergeBase = getRepository().mergeBase(refForMigration, LOCAL_PR_BASE_BRANCH);
432440
labels.put(GITHUB_BASE_BRANCH_SHA1, mergeBase);
@@ -444,7 +452,7 @@ private GitRevision getRevisionForPR(String project, PullRequest prData)
444452
gitRevision.getSha1(),
445453
// TODO(malcon): Decide the format to use here:
446454
/*reviewReference=*/null,
447-
useMerge ? asMergeRef(prNumber) : asHeadRef(prNumber),
455+
actuallyUseMerge ? asMergeRef(prNumber) : asHeadRef(prNumber),
448456
labels.build(),
449457
url);
450458

@@ -453,7 +461,7 @@ private GitRevision getRevisionForPR(String project, PullRequest prData)
453461

454462
private void checkRequiredCheckRuns(GitHubApi api, String project, PullRequest prData)
455463
throws ValidationException, RepoException {
456-
if (gitHubPrOriginOptions.forceImport || requiredCheckRuns.isEmpty()) {
464+
if (forceImport() || requiredCheckRuns.isEmpty()) {
457465
return;
458466
}
459467
try (ProfilerTask ignore = generalOptions.profiler()
@@ -477,7 +485,7 @@ private void checkRequiredCheckRuns(GitHubApi api, String project, PullRequest p
477485

478486
private void checkRequiredStatusContextNames(GitHubApi api, String project, PullRequest prData)
479487
throws ValidationException, RepoException {
480-
if (gitHubPrOriginOptions.forceImport || requiredStatusContextNames.isEmpty()) {
488+
if (forceImport() || requiredStatusContextNames.isEmpty()) {
481489
return;
482490
}
483491
try (ProfilerTask ignore = generalOptions.profiler()
@@ -520,8 +528,8 @@ public Reader<GitRevision> newReader(Glob originFiles, Authoring authoring)
520528
partialFetch,
521529
patchTransformation,
522530
describeVersion,
523-
/*configPath=*/null,
524-
/*workflowName=*/null) {
531+
/*configPath=*/ null,
532+
/*workflowName=*/ null) {
525533

526534
/** Disable rebase since this is controlled by useMerge field. */
527535
@Override
@@ -569,7 +577,10 @@ public Endpoint getFeedbackEndPoint(Console console) throws ValidationException
569577
@Override
570578
public ChangesResponse<GitRevision> changes(@Nullable GitRevision fromRef, GitRevision toRef)
571579
throws RepoException, ValidationException {
572-
if (!useMerge) {
580+
checkCondition(
581+
toRef.associatedLabels().containsKey(GITHUB_PR_USE_MERGE),
582+
"Cannot determine whether 'use_merge' was set.");
583+
if (toRef.associatedLabel(GITHUB_PR_USE_MERGE).contains("false")) {
573584
return super.changes(fromRef, toRef);
574585
}
575586
GitLogEntry merge =
@@ -645,6 +656,10 @@ public ImmutableSetMultimap<String, String> describe(Glob originFiles) {
645656
return builder.build();
646657
}
647658

659+
private boolean forceImport() {
660+
return generalOptions.isForced() || gitHubPrOriginOptions.forceImport;
661+
}
662+
648663
/**
649664
* Only migrate PR in one of the following states:
650665
*/

‎java/com/google/copybara/git/GitModule.java‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import static com.google.copybara.git.GitHubPROrigin.GITHUB_PR_TITLE;
3131
import static com.google.copybara.git.GitHubPROrigin.GITHUB_PR_URL;
3232
import static com.google.copybara.git.GitHubPROrigin.GITHUB_PR_USER;
33+
import static com.google.copybara.git.GitHubPROrigin.GITHUB_PR_USE_MERGE;
3334
import static com.google.copybara.git.LatestVersionSelector.VersionElementType.ALPHABETIC;
3435
import static com.google.copybara.git.LatestVersionSelector.VersionElementType.NUMERIC;
3536
import static com.google.copybara.git.github.api.GitHubEventType.WATCHABLE_EVENTS;
@@ -45,6 +46,8 @@
4546
import com.google.copybara.Options;
4647
import com.google.copybara.Transformation;
4748
import com.google.copybara.WorkflowOptions;
49+
import com.google.copybara.action.Action;
50+
import com.google.copybara.action.StarlarkAction;
4851
import com.google.copybara.checks.Checker;
4952
import com.google.copybara.config.ConfigFile;
5053
import com.google.copybara.config.GlobalMigrations;
@@ -55,8 +58,6 @@
5558
import com.google.copybara.doc.annotations.UsesFlags;
5659
import com.google.copybara.exception.RepoException;
5760
import com.google.copybara.exception.ValidationException;
58-
import com.google.copybara.action.Action;
59-
import com.google.copybara.action.StarlarkAction;
6061
import com.google.copybara.git.GerritDestination.ChangeIdPolicy;
6162
import com.google.copybara.git.GerritDestination.NotifyOption;
6263
import com.google.copybara.git.GitDestination.WriterImpl.DefaultWriteHook;
@@ -75,7 +76,14 @@
7576
import com.google.copybara.util.console.Console;
7677
import com.google.re2j.Matcher;
7778
import com.google.re2j.Pattern;
78-
79+
import java.util.ArrayList;
80+
import java.util.HashSet;
81+
import java.util.LinkedHashSet;
82+
import java.util.List;
83+
import java.util.Map;
84+
import java.util.TreeMap;
85+
import javax.annotation.CheckReturnValue;
86+
import javax.annotation.Nullable;
7987
import net.starlark.java.annot.Param;
8088
import net.starlark.java.annot.ParamType;
8189
import net.starlark.java.annot.StarlarkBuiltin;
@@ -93,16 +101,6 @@
93101
import net.starlark.java.eval.StarlarkValue;
94102
import net.starlark.java.syntax.Location;
95103

96-
import java.util.ArrayList;
97-
import java.util.HashSet;
98-
import java.util.LinkedHashSet;
99-
import java.util.List;
100-
import java.util.Map;
101-
import java.util.TreeMap;
102-
103-
import javax.annotation.CheckReturnValue;
104-
import javax.annotation.Nullable;
105-
106104
/** Main module that groups all the functions that create Git origins and destinations. */
107105
@StarlarkBuiltin(name = "git", doc = "Set of functions to define Git origins and destinations.")
108106
@UsesFlags(GitOptions.class)
@@ -679,6 +677,10 @@ public GitOrigin gerritOrigin(
679677
+ " point of divergence between the PR's 'base' and 'head' branches. When `use_merge"
680678
+ " = True` is specified, the baseline is instead the tip of the PR's base branch.\n"
681679
+ " - "
680+
+ GITHUB_PR_USE_MERGE
681+
+ ": Equal to 'true' if the workflow is importing a GitHub PR 'merge' commit and"
682+
+ " 'false' when importing a GitHub PR 'head' commit.\n"
683+
+ " - "
682684
+ GITHUB_PR_TITLE
683685
+ ": Title of the Pull Request.\n"
684686
+ " - "

‎javatests/com/google/copybara/git/GitHubPrOriginTest.java‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import static com.google.copybara.git.GitHubPROrigin.GITHUB_PR_TITLE;
2727
import static com.google.copybara.git.GitHubPROrigin.GITHUB_PR_URL;
2828
import static com.google.copybara.git.GitHubPROrigin.GITHUB_PR_USER;
29+
import static com.google.copybara.git.GitHubPROrigin.GITHUB_PR_USE_MERGE;
2930
import static com.google.copybara.testing.git.GitTestUtil.getGitEnv;
3031
import static com.google.copybara.testing.git.GitTestUtil.mockResponse;
3132
import static com.google.copybara.testing.git.GitTestUtil.mockResponseAndValidateRequest;
@@ -1024,6 +1025,55 @@ public void testCheckout_noMergeRef() throws Exception {
10241025
.contains("Cannot find a merge reference for Pull Request 123");
10251026
}
10261027

1028+
@Test
1029+
public void testCheckout_noMergeRef_withForce() throws Exception {
1030+
GitRepository remote = withTmpWorktree
1031+
(gitUtil.mockRemoteRepo("github.com/google/example"));
1032+
String baseRef = addFiles(remote, "base", ImmutableMap.of("test.txt", "a"));
1033+
1034+
remote.simpleCommand("branch", "feature");
1035+
String featureRef =
1036+
addFiles(remote, "commit to feature branch", ImmutableMap.of("test.txt", "c"));
1037+
1038+
remote.forceCheckout(baseRef);
1039+
String mainRef =
1040+
addFiles(remote, "commit to main branch", ImmutableMap.of("test.txt", "b"));
1041+
remote.simpleCommand("merge", "feature");
1042+
String mergeRef = remote.parseRef("HEAD");
1043+
remote.forceCheckout(mainRef);
1044+
1045+
remote.simpleCommand("update-ref", GitHubUtil.asHeadRef(123), featureRef);
1046+
remote.simpleCommand("update-ref", GitHubUtil.asMergeRef(123), mergeRef);
1047+
1048+
// Using --force should respect "use_merge = True" when a merge commit does exist
1049+
options.setForce(true);
1050+
skylark = new SkylarkTestExecutor(options);
1051+
1052+
mockPullRequestAndIssue("open", "main", 123, /*mergeable = */ true);
1053+
1054+
GitHubPROrigin origin =
1055+
githubPrOrigin("url = 'https://github.com/google/example'", "use_merge = True");
1056+
1057+
assertThat(origin.resolve("123").getSha1())
1058+
.isEqualTo(remote.resolveReference(GitHubUtil.asMergeRef(123)).getSha1());
1059+
assertThat(origin.resolve("123").associatedLabel(GITHUB_PR_USE_MERGE)).containsExactly("true");
1060+
1061+
// Using --force should override "use_merge = True", since no merge commit exists
1062+
1063+
options.setForce(true);
1064+
skylark = new SkylarkTestExecutor(options);
1065+
1066+
remote.simpleCommand("reset", "--hard", "HEAD~1"); //delete the merge commit
1067+
mockPullRequestAndIssue("open", "main", 123, /*mergeable = */ false);
1068+
1069+
origin =
1070+
githubPrOrigin("url = 'https://github.com/google/example'", "use_merge = True");
1071+
1072+
assertThat(origin.resolve("123").getSha1())
1073+
.isEqualTo(remote.resolveReference(GitHubUtil.asHeadRef(123)).getSha1());
1074+
assertThat(origin.resolve("123").associatedLabel(GITHUB_PR_USE_MERGE)).containsExactly("false");
1075+
}
1076+
10271077
private void checkResolve(GitHubPROrigin origin, String reference, int prNumber)
10281078
throws RepoException, IOException, ValidationException {
10291079
GitRepository remote = gitUtil.mockRemoteRepo("github.com/google/example");

0 commit comments

Comments
 (0)