Skip to content

Commit ddcaf20

Browse files
committed
Allow additional mergeable statuses to skip upload
We have some statuses that we know are safe to skip to upload when the git diff would be empty for github_pr_destination. But there might be other statuses where we want to skip too. This allows users to add additional statuses to skip. The inmediate usage is to skip when "BLOCKED". We think this would make sense in general when this GitHub destination is not the SoT, but we want to have the flexibility to enable it for individual use cases. Update update docs script to use java 11 (otherwise it fails) Change-Id: Ie21d0b1b46a2d28f930f834de0f2b1e93099ec3b
1 parent 126613a commit ddcaf20

File tree

5 files changed

+56
-6
lines changed

5 files changed

+56
-6
lines changed

docs/reference.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -2535,7 +2535,7 @@ primary_branch_migration | `bool`<br><p>When enabled, copybara will ignore the '
25352535

25362536
Creates changes in a new pull request in the destination.
25372537

2538-
`destination` `git.github_pr_destination(url, destination_ref='master', pr_branch=None, partial_fetch=False, allow_empty_diff=True, title=None, body=None, integrates=None, api_checker=None, update_description=False, primary_branch_migration=False, checker=None, draft=False)`
2538+
`destination` `git.github_pr_destination(url, destination_ref='master', pr_branch=None, partial_fetch=False, allow_empty_diff=True, empty_diff_merge_statuses=[], title=None, body=None, integrates=None, api_checker=None, update_description=False, primary_branch_migration=False, checker=None, draft=False)`
25392539

25402540

25412541
#### Parameters:
@@ -2547,6 +2547,7 @@ destination_ref | `string`<br><p>Destination reference for the change.</p>
25472547
pr_branch | `string` or `NoneType`<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>
25482548
partial_fetch | `bool`<br><p>This is an experimental feature that only works for certain origin globs.</p>
25492549
allow_empty_diff | `bool`<br><p>By default, copybara migrates changes without checking existing PRs. If set, copybara will skip pushing a change to an existing PR only if the git three of the pending migrating change is the same as the existing PR.</p>
2550+
empty_diff_merge_statuses | `sequence of string`<br><p>By default, if `allow_empty_diff = False` is set, Copybara will also check the **experimental** GitHub field `mergeable status` to decide if it uploads or not. For example if status is 'CLEAN', 'DRAFT', 'HAS_HOOKS' or 'BEHIND', it assumes that the uploaded content is up to date. This field allows to add additional states that the PR could be in where we skip uploading, for example 'BLOCKED'. The statuses values are described at: https://docs.github.com/en/github-ae@latest/graphql/reference/enums#mergestatestatus. **Note that this field is experimental and is subject to change by GitHub without notice**. Please consult Copybara team before using this field</p>
25502551
title | `string` or `NoneType`<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>
25512552
body | `string` or `NoneType`<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>
25522553
integrates | `sequence of git_integrate` or `NoneType`<br><p>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</p>

java/com/google/copybara/git/GitHubPrWriteHook.java

+11-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818

1919
import com.google.common.base.Preconditions;
2020
import com.google.common.collect.ImmutableList;
21+
import com.google.common.collect.ImmutableSet;
2122
import com.google.copybara.GeneralOptions;
23+
import com.google.copybara.config.SkylarkUtil;
2224
import com.google.copybara.exception.RedundantChangeException;
2325
import com.google.copybara.exception.RepoException;
2426
import com.google.copybara.exception.ValidationException;
@@ -42,6 +44,7 @@ public class GitHubPrWriteHook extends DefaultWriteHook {
4244
private final GeneralOptions generalOptions;
4345
private final GitHubOptions gitHubOptions;
4446
private final boolean partialFetch;
47+
private final ImmutableSet<String> emptyDiffMergeStatuses;
4548
private final Console console;
4649
private GitHubHost ghHost;
4750
@Nullable private final String prBranchToUpdate;
@@ -54,14 +57,15 @@ public class GitHubPrWriteHook extends DefaultWriteHook {
5457
@Nullable String prBranchToUpdate,
5558
boolean partialFetch,
5659
boolean allowEmptyDiff,
57-
Console console,
60+
ImmutableSet<String> emptyDiffMergeStatuses, Console console,
5861
GitHubHost ghHost) {
5962
this.generalOptions = Preconditions.checkNotNull(generalOptions);
6063
this.repoUrl = Preconditions.checkNotNull(repoUrl);
6164
this.gitHubOptions = Preconditions.checkNotNull(gitHubOptions);
6265
this.prBranchToUpdate = prBranchToUpdate;
6366
this.partialFetch = partialFetch;
6467
this.allowEmptyDiff = allowEmptyDiff;
68+
this.emptyDiffMergeStatuses = emptyDiffMergeStatuses;
6569
this.console = Preconditions.checkNotNull(console);
6670
this.ghHost = Preconditions.checkNotNull(ghHost);
6771
}
@@ -146,6 +150,11 @@ private boolean skipUploadBasedOnPrStatus(String configProjectName, GitHubApi ap
146150
console.verboseFmt("Skipping upload because mergeable status is %s", mergeableState);
147151
return true;
148152
default:
153+
if (emptyDiffMergeStatuses.contains(mergeableState.toUpperCase())) {
154+
console.infoFmt("Skipping upload because mergeable status is %s,"
155+
+ " that is configured in users config.", mergeableState);
156+
return true;
157+
}
149158
console.verboseFmt("Not skipping upload because mergeable status is %s", mergeableState);
150159
return false;
151160
}
@@ -159,6 +168,7 @@ protected GitHubPrWriteHook withUpdatedPrBranch(String prBranchToUpdate) {
159168
prBranchToUpdate,
160169
this.partialFetch,
161170
this.allowEmptyDiff,
171+
this.emptyDiffMergeStatuses,
162172
this.console,
163173
this.ghHost);
164174
}

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

+23-2
Original file line numberDiff line numberDiff line change
@@ -1688,7 +1688,24 @@ public GitDestination gitHubDestination(
16881688
+ "If set, copybara will skip pushing a change to an existing PR "
16891689
+ "only if the git three of the pending migrating change is the same "
16901690
+ "as the existing PR."),
1691-
@Param(
1691+
@Param(
1692+
name = "empty_diff_merge_statuses",
1693+
allowedTypes = {
1694+
@ParamType(type = Sequence.class, generic1 = String.class)
1695+
},
1696+
defaultValue = "[]",
1697+
named = true,
1698+
positional = false,
1699+
doc = "By default, if `allow_empty_diff = False` is set, Copybara will also check"
1700+
+ " the **experimental** GitHub field `mergeable status` to decide if it uploads"
1701+
+ " or not. For example if status is 'CLEAN', 'DRAFT', 'HAS_HOOKS' or 'BEHIND',"
1702+
+ " it assumes that the uploaded content is up to date. This field allows to"
1703+
+ " add additional states that the PR could be in where we skip uploading, for"
1704+
+ " example 'BLOCKED'. The statuses values are described at:"
1705+
+ " https://docs.github.com/en/github-ae@latest/graphql/reference/enums#mergestatestatus."
1706+
+ " **Note that this field is experimental and is subject to change by GitHub"
1707+
+ " without notice**. Please consult Copybara team before using this field"),
1708+
@Param(
16921709
name = "title",
16931710
allowedTypes = {
16941711
@ParamType(type = String.class),
@@ -1778,7 +1795,7 @@ public GitDestination gitHubDestination(
17781795
defaultValue = "False",
17791796
named = true,
17801797
positional = false,
1781-
doc = "Flag create pull request as draft or not."),
1798+
doc = "Flag create pull request as draft or not.")
17821799
},
17831800
useStarlarkThread = true)
17841801
@UsesFlags({GitDestinationOptions.class, GitHubDestinationOptions.class})
@@ -1814,6 +1831,7 @@ public GitHubPrDestination githubPrDestination(
18141831
Object prBranch,
18151832
Boolean partialFetch,
18161833
Boolean allowEmptyDiff,
1834+
Sequence<?> emptyDiffMergeStatuses,
18171835
Object title,
18181836
Object body,
18191837
Object integrates,
@@ -1853,6 +1871,9 @@ public GitHubPrDestination githubPrDestination(
18531871
destinationPrBranch,
18541872
partialFetch,
18551873
allowEmptyDiff,
1874+
ImmutableSet.copyOf(
1875+
SkylarkUtil.convertStringList(emptyDiffMergeStatuses,
1876+
"empty_diff_merge_statuses")),
18561877
getGeneralConsole(),
18571878
GITHUB_COM),
18581879
Starlark.isNullOrNone(integrates)

javatests/com/google/copybara/git/GitHubPrDestinationTest.java

+19-1
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import com.google.copybara.WriterContext;
4444
import com.google.copybara.authoring.Author;
4545
import com.google.copybara.effect.DestinationEffect;
46+
import com.google.copybara.exception.CannotResolveRevisionException;
4647
import com.google.copybara.exception.RedundantChangeException;
4748
import com.google.copybara.exception.RepoException;
4849
import com.google.copybara.exception.ValidationException;
@@ -67,6 +68,7 @@
6768
import java.nio.file.Path;
6869
import java.util.Map;
6970
import java.util.Map.Entry;
71+
import javax.annotation.Nullable;
7072
import net.starlark.java.eval.StarlarkList;
7173
import org.junit.Before;
7274
import org.junit.Test;
@@ -84,6 +86,7 @@ public class GitHubPrDestinationTest {
8486

8587
private Path workdir;
8688
private String primaryBranchMigration;
89+
@Nullable private String emptyDiffMergeStatus;
8790

8891
@Before
8992
public void setup() throws Exception {
@@ -636,6 +639,18 @@ public void testWriteNomain() throws ValidationException, IOException, RepoExcep
636639
@Test
637640
public void emptyChange() throws Exception {
638641
Writer<GitRevision> writer = getWriterForTestEmptyDiff();
642+
runEmptyChange(writer, "clean");
643+
}
644+
645+
@Test
646+
public void emptyChangeBecauseUserConfigureStatus() throws Exception {
647+
emptyDiffMergeStatus = "BLOCKED";
648+
Writer<GitRevision> writer = getWriterForTestEmptyDiff();
649+
runEmptyChange(writer, "blocked");
650+
}
651+
652+
private void runEmptyChange(Writer<GitRevision> writer, String prMergeableState)
653+
throws RepoException, IOException, CannotResolveRevisionException {
639654
GitRepository remote = gitUtil.mockRemoteRepo("github.com/foo");
640655
addFiles(
641656
remote,
@@ -665,7 +680,7 @@ public void emptyChange() throws Exception {
665680
+ " \"state\": \"closed\",\n"
666681
+ " \"title\": \"test summary\",\n"
667682
+ " \"mergeable\": true,\n"
668-
+ " \"mergeable_state\": \"clean\",\n"
683+
+ " \"mergeable_state\": \"" + prMergeableState + "\",\n"
669684
+ " \"body\": \"test summary\","
670685
+ " \"head\": {\"sha\": \"" + changeHead + "\"},"
671686
+ " \"base\": {\"sha\": \"" + baseline + "\"}"
@@ -813,6 +828,9 @@ private Writer<GitRevision> getWriterForTestEmptyDiff() throws Exception {
813828
+ " allow_empty_diff = False,\n"
814829
+ " destination_ref = 'main',\n"
815830
+ " pr_branch = 'test_${CONTEXT_REFERENCE}',\n"
831+
+ (emptyDiffMergeStatus != null
832+
? "empty_diff_merge_statuses = ['" + emptyDiffMergeStatus + "'],\n"
833+
: "")
816834
+ " primary_branch_migration = " + primaryBranchMigration + ",\n"
817835
+ ")");
818836
WriterContext writerContext =

scripts/update_docs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/bin/bash
2-
bazel build java/com/google/copybara:generate_reference
2+
bazel build --java_language_version=11 --tool_java_language_version=11 --java_runtime_version=remotejdk_11 java/com/google/copybara:generate_reference
33
mkdir -p docs
44
cp bazel-bin/java/com/google/copybara/reference.md docs/reference.md
55
git add docs/reference.md

0 commit comments

Comments
 (0)