diff --git a/src/ghstack/diff.py b/src/ghstack/diff.py index fbe4f30..1fd5be7 100644 --- a/src/ghstack/diff.py +++ b/src/ghstack/diff.py @@ -14,7 +14,7 @@ RAW_PULL_REQUEST_RESOLVED = ( - r"(Pull Request resolved|Pull-Request-resolved): " + r"(Pull Request resolved|Pull-Request-resolved|Pull-Request): " r"https://{github_url}/(?P[^/]+)/(?P[^/]+)/pull/(?P[0-9]+)" ) @@ -93,7 +93,7 @@ class Diff: # be the same. source_id: str - # The contents of 'Pull-Request-resolved'. This is None for + # The contents of 'Pull-Request'. This is None for # diffs that haven't been submitted by ghstack. For BC reasons, # this also accepts gh-metadata. pull_request_resolved: Optional[PullRequestResolved] diff --git a/src/ghstack/land.py b/src/ghstack/land.py index c2bc853..4a96d60 100644 --- a/src/ghstack/land.py +++ b/src/ghstack/land.py @@ -152,11 +152,18 @@ def main( sh.git("cherry-pick", "--abort") raise + # All good! Push! + maybe_force_arg = [] + if needs_force: + maybe_force_arg = ["--force-with-lease"] + sh.git( + "push", *maybe_force_arg, remote_name, f"HEAD:refs/heads/{default_branch}" + ) + # Advance base to head to "close" the PR for all PRs. - # This has to happen before the push because the push - # will trigger a closure, but we want a *merge*. This should - # happen after the cherry-pick, because the cherry-picks can - # fail + # This happens after the cherry-pick and push, because the cherry-picks + # can fail (merge conflict) and the push can also fail (race condition) + # TODO: It might be helpful to advance orig to reflect the true # state of upstream at the time we are doing the land, and then # directly *merge* head into base, so that the PR accurately @@ -173,14 +180,6 @@ def main( ) github.notify_merged(pr_resolved) - # All good! Push! - maybe_force_arg = [] - if needs_force: - maybe_force_arg = ["--force-with-lease"] - sh.git( - "push", *maybe_force_arg, remote_name, f"HEAD:refs/heads/{default_branch}" - ) - # Delete the branches for orig_ref, _ in stack_orig_refs: # TODO: regex here so janky diff --git a/src/ghstack/submit.py b/src/ghstack/submit.py index 278afb5..87b8067 100644 --- a/src/ghstack/submit.py +++ b/src/ghstack/submit.py @@ -810,7 +810,7 @@ def elaborate_diff( convert a pre-existing PR in the old style to the new format which supports bidirectional syncing. If you would like to blow away the old PR and start anew, edit the Summary in the Phabricator diff to delete -the line 'Pull-Request-resolved' and then run ghexport again. +the line 'Pull-Request' and then run ghexport again. """ ) @@ -827,7 +827,7 @@ def elaborate_diff( to export your diff. If you think this is in error, edit the Summary in the Phabricator diff -to delete the line 'Pull-Request-resolved' and then run ghexport again. +to delete the line 'Pull-Request' and then run ghexport again. """ ) else: @@ -964,9 +964,7 @@ def process_commit( if self.direct: trailers_to_add.append(f"ghstack-comment-id: {elab_diff.comment_id}") - trailers_to_add.append( - f"Pull-Request-resolved: {pull_request_resolved.url()}" - ) + trailers_to_add.append(f"Pull-Request: {pull_request_resolved.url()}") commit_msg = ghstack.trailers.interpret_trailers( strip_mentions(diff.summary.rstrip()), trailers_to_add @@ -1773,7 +1771,7 @@ def _default_title_and_body( commit_body = RE_GHSTACK_SOURCE_ID.sub("", commit_body) # Comment ID is not necessary; source of truth is orig commit commit_body = RE_GHSTACK_COMMENT_ID.sub("", commit_body) - # Don't store Pull request resolved in the PR body; it's + # Don't store Pull request in the PR body; it's # unnecessary commit_body = ghstack.diff.re_pull_request_resolved_w_sp(self.github_url).sub( "", commit_body diff --git a/test/land/default_branch_change.py.test b/test/land/default_branch_change.py.test index 7da69ad..7041dc4 100644 --- a/test/land/default_branch_change.py.test +++ b/test/land/default_branch_change.py.test @@ -48,7 +48,7 @@ assert_expected_inline( assert_expected_inline( get_upstream_sh().git("log", "--oneline", "main"), """\ -19fa5fa Commit A +8927014 Commit A dc8bfe4 Initial commit""", ) @@ -82,15 +82,15 @@ assert_github_state( This is commit B - * f5135dc Initial 2 + * ab7d5ca Initial 2 Repository state: - * f5135dc (gh/ezyang/2/head) + * ab7d5ca (gh/ezyang/2/head) | Initial 2 - * 61e5c2a (gh/ezyang/2/base) + * 742ae0b (gh/ezyang/2/base) | Initial 2 (base update) - * 19fa5fa (main, gh/ezyang/1/orig) + * 8927014 (main, gh/ezyang/1/orig) | Commit A | * 36fcfdf (gh/ezyang/1/head) | | Initial 1 @@ -107,13 +107,13 @@ gh_land(diff2.pr_url) assert_expected_inline( get_upstream_sh().git("log", "--oneline", "master"), """\ -19e7996 Commit B -cb4e674 Commit A +d779d84 Commit B +542f79d Commit A dc8bfe4 Initial commit""", ) assert_expected_inline( get_upstream_sh().git("log", "--oneline", "main"), """\ -19fa5fa Commit A +8927014 Commit A dc8bfe4 Initial commit""", ) diff --git a/test/land/ff.py.test b/test/land/ff.py.test index 713f262..c2a1a3d 100644 --- a/test/land/ff.py.test +++ b/test/land/ff.py.test @@ -11,7 +11,7 @@ gh_land(pr_url) assert_expected_inline( get_upstream_sh().git("log", "--oneline", "master"), """\ -19fa5fa Commit A +8927014 Commit A dc8bfe4 Initial commit""", ) diff --git a/test/land/ff_stack.py.test b/test/land/ff_stack.py.test index 7f6a019..55c9ece 100644 --- a/test/land/ff_stack.py.test +++ b/test/land/ff_stack.py.test @@ -16,7 +16,7 @@ gh_land(pr_url) assert_expected_inline( get_upstream_sh().git("log", "--oneline", "master"), """\ -3c9c5eb Commit B -6eb4d4f Commit A +b6c40ad Commit B +851cf96 Commit A dc8bfe4 Initial commit""", ) diff --git a/test/land/ff_stack_two_phase.py.test b/test/land/ff_stack_two_phase.py.test index eecfcaf..85f6212 100644 --- a/test/land/ff_stack_two_phase.py.test +++ b/test/land/ff_stack_two_phase.py.test @@ -18,7 +18,7 @@ gh_land(pr_url2) assert_expected_inline( get_upstream_sh().git("log", "--oneline", "master"), """\ -3c9c5eb Commit B -6eb4d4f Commit A +b6c40ad Commit B +851cf96 Commit A dc8bfe4 Initial commit""", ) diff --git a/test/land/invalid_resubmit.py.test b/test/land/invalid_resubmit.py.test index 26fc97f..d50f9e8 100644 --- a/test/land/invalid_resubmit.py.test +++ b/test/land/invalid_resubmit.py.test @@ -44,15 +44,15 @@ else: This is commit A - * c461f2f New PR + * 98fcb03 New PR Repository state: - * c461f2f (gh/ezyang/1/head) + * 98fcb03 (gh/ezyang/1/head) | New PR - * 58e6f57 (gh/ezyang/1/base) + * 0d09e7d (gh/ezyang/1/base) | New PR (base update) - * 19fa5fa (HEAD -> master) + * 8927014 (HEAD -> master) | Commit A * dc8bfe4 Initial commit diff --git a/test/land/non_ff.py.test b/test/land/non_ff.py.test index 4020d82..7ae7ebf 100644 --- a/test/land/non_ff.py.test +++ b/test/land/non_ff.py.test @@ -17,7 +17,7 @@ gh_land(pr_url) assert_expected_inline( get_upstream_sh().git("log", "--oneline", "master"), """\ -1d53e04 Commit A +d43d06e Commit A 38808c0 Commit U dc8bfe4 Initial commit""", ) diff --git a/test/land/non_ff_stack_two_phase.py.test b/test/land/non_ff_stack_two_phase.py.test index 26fe35f..751b1fd 100644 --- a/test/land/non_ff_stack_two_phase.py.test +++ b/test/land/non_ff_stack_two_phase.py.test @@ -22,8 +22,8 @@ gh_land(pr_url2) assert_expected_inline( get_upstream_sh().git("log", "--oneline", "master"), """\ -613567d Commit B -4dfc330 Commit A +ec1d0de Commit B +d8a6272 Commit A a8ca27f Commit C dc8bfe4 Initial commit""", ) diff --git a/test/land/update_after_land.py.test b/test/land/update_after_land.py.test index 3736a01..412304f 100644 --- a/test/land/update_after_land.py.test +++ b/test/land/update_after_land.py.test @@ -55,16 +55,16 @@ else: This is commit B - * 48dc874 Run 3 + * c6c8f43 Run 3 * 16e1e12 Initial 1 Repository state: - * 48dc874 (gh/ezyang/2/head) + * c6c8f43 (gh/ezyang/2/head) |\\ Run 3 - | * 5a46f58 (gh/ezyang/2/base) + | * 36376f9 (gh/ezyang/2/base) | |\\ Run 3 (base update) - | | * 30a9800 (HEAD -> master) + | | * 9bf93f4 (HEAD -> master) | | | Commit A | | * 7f0288c | | | Commit U diff --git a/test/submit/do_not_revert_local_commit_msg_on_skip.py.test b/test/submit/do_not_revert_local_commit_msg_on_skip.py.test index 62b932b..e0918a7 100644 --- a/test/submit/do_not_revert_local_commit_msg_on_skip.py.test +++ b/test/submit/do_not_revert_local_commit_msg_on_skip.py.test @@ -17,7 +17,7 @@ if is_direct(): ghstack-source-id: ac00f28640afe01e4299441bb5041cdf06d0b6b4 ghstack-comment-id: 1500 - Pull-Request-resolved: https://github.com/pytorch/pytorch/pull/500""", + Pull-Request: https://github.com/pytorch/pytorch/pull/500""", ) else: assert_expected_inline( @@ -28,7 +28,7 @@ else: This is commit ARGLE ghstack-source-id: ac00f28640afe01e4299441bb5041cdf06d0b6b4 - Pull-Request-resolved: https://github.com/pytorch/pytorch/pull/500""", + Pull-Request: https://github.com/pytorch/pytorch/pull/500""", ) if is_direct(): diff --git a/test/submit/strip_mentions.py.test b/test/submit/strip_mentions.py.test index 7fc9c31..4ead37c 100644 --- a/test/submit/strip_mentions.py.test +++ b/test/submit/strip_mentions.py.test @@ -46,7 +46,7 @@ if is_direct(): Signed-off-by: foo@gmail.com ghstack-source-id: ddf506901b8d3c2b4079ec39e564f19bd5468397 ghstack-comment-id: 1500 - Pull-Request-resolved: https://github.com/pytorch/pytorch/pull/500""", + Pull-Request: https://github.com/pytorch/pytorch/pull/500""", ) else: assert_expected_inline( @@ -60,5 +60,5 @@ else: Signed-off-by: foo@gmail.com ghstack-source-id: ddf506901b8d3c2b4079ec39e564f19bd5468397 - Pull-Request-resolved: https://github.com/pytorch/pytorch/pull/500""", + Pull-Request: https://github.com/pytorch/pytorch/pull/500""", )