From 380e619d3d9f7310e2075162f4a449ccc8bd4e38 Mon Sep 17 00:00:00 2001 From: vmoens Date: Tue, 13 Jan 2026 16:51:20 -0800 Subject: [PATCH 1/5] init --- src/ghstack/land.py | 26 ++++++- src/ghstack/submit.py | 96 +++++++++++++++++------- test/land/default_branch_change.py.test | 4 +- test/land/ff.py.test | 2 +- test/land/ff_stack.py.test | 4 +- test/land/ff_stack_two_phase.py.test | 4 +- test/land/invalid_resubmit.py.test | 10 +-- test/land/non_ff.py.test | 2 +- test/land/non_ff_stack_two_phase.py.test | 4 +- test/land/update_after_land.py.test | 10 +-- 10 files changed, 115 insertions(+), 47 deletions(-) diff --git a/src/ghstack/land.py b/src/ghstack/land.py index 4a96d60..f018594 100644 --- a/src/ghstack/land.py +++ b/src/ghstack/land.py @@ -145,9 +145,33 @@ def main( stack_orig_refs.append((ref, pr_resolved)) # OK, actually do the land now - for orig_ref, _ in stack_orig_refs: + for orig_ref, pr_resolved in stack_orig_refs: try: sh.git("cherry-pick", f"{remote_name}/{orig_ref}") + # Add PR number to commit message like GitHub does + commit_msg = sh.git("log", "-1", "--pretty=%B") + # Get the original author and committer dates to preserve the commit hash + author_date = sh.git("log", "-1", "--pretty=%aD") + committer_date = sh.git("log", "-1", "--pretty=%cD") + lines = commit_msg.split("\n") + if lines: + # Add PR number to the subject line (first line) + subject = lines[0].rstrip() + # Only add if not already present + pr_tag = f"(#{pr_resolved.number})" + if pr_tag not in subject: + subject = f"{subject} {pr_tag}" + lines[0] = subject + new_msg = "\n".join(lines) + # Preserve dates to keep the commit hash consistent + sh.sh( + "git", + "commit", + "--amend", + "-m", + new_msg, + env={"GIT_AUTHOR_DATE": author_date, "GIT_COMMITTER_DATE": committer_date}, + ) except BaseException: sh.git("cherry-pick", "--abort") raise diff --git a/src/ghstack/submit.py b/src/ghstack/submit.py index b9568b9..74cbc92 100644 --- a/src/ghstack/submit.py +++ b/src/ghstack/submit.py @@ -487,18 +487,20 @@ def run(self) -> List[DiffMeta]: d = ghstack.git.convert_header(h, self.github_url) if d.pull_request_resolved is not None: ed = self.elaborate_diff(d) - pre_branch_state_index[h.commit_id] = PreBranchState( - head_commit_id=GitCommitHash( - self.sh.git( - "rev-parse", f"{self.remote_name}/{ed.head_ref}" - ) - ), - base_commit_id=GitCommitHash( - self.sh.git( - "rev-parse", f"{self.remote_name}/{ed.base_ref}" - ) - ), - ) + # Skip closed PRs (e.g., after landing) where branches have been deleted + if not ed.closed: + pre_branch_state_index[h.commit_id] = PreBranchState( + head_commit_id=GitCommitHash( + self.sh.git( + "rev-parse", f"{self.remote_name}/{ed.head_ref}" + ) + ), + base_commit_id=GitCommitHash( + self.sh.git( + "rev-parse", f"{self.remote_name}/{ed.base_ref}" + ) + ), + ) # NB: deduplicates commit_index = { @@ -870,21 +872,22 @@ def elaborate_diff( "--header", self.remote_name + "/" + branch_orig(username, gh_number), ) - except RuntimeError as e: + except RuntimeError: if r["closed"]: - raise RuntimeError( - f"Cannot ghstack a stack with closed PR #{number} whose branch was deleted. " - "If you were just trying to update a later PR in the stack, `git rebase` and try again. " - "Otherwise, you may have been trying to update a PR that was already closed. " - "To disassociate your update from the old PR and open a new PR, " - "run `ghstack unlink`, `git rebase` and then try again." - ) from e - raise - remote_summary = ghstack.git.split_header(rev_list)[0] - m_remote_source_id = RE_GHSTACK_SOURCE_ID.search(remote_summary.commit_msg) - remote_source_id = m_remote_source_id.group(1) if m_remote_source_id else None - m_comment_id = RE_GHSTACK_COMMENT_ID.search(remote_summary.commit_msg) - comment_id = int(m_comment_id.group(1)) if m_comment_id else None + # If the PR is closed and the branch is deleted (e.g., after landing), + # we can't get the remote source ID. Return None for it, which will + # signal to process_commit that this commit has been landed and should + # be skipped (not updated). + remote_source_id = None + comment_id = None + else: + raise + else: + remote_summary = ghstack.git.split_header(rev_list)[0] + m_remote_source_id = RE_GHSTACK_SOURCE_ID.search(remote_summary.commit_msg) + remote_source_id = m_remote_source_id.group(1) if m_remote_source_id else None + m_comment_id = RE_GHSTACK_COMMENT_ID.search(remote_summary.commit_msg) + comment_id = int(m_comment_id.group(1)) if m_comment_id else None return DiffWithGitHubMetadata( diff=diff, @@ -917,6 +920,47 @@ def process_commit( if elab_diff is not None and elab_diff.closed: if self.direct: self._raise_needs_rebase() + # If we're trying to submit a closed commit, check if it has been modified + if elab_diff.remote_source_id is None: + # The branch was deleted (e.g., after landing). Check if the commit has been + # modified by comparing source_ids. If the commit is reachable from master with + # the same source_id (tree hash), it means it was landed and we should skip it. + # Otherwise, it's been modified and we should raise an error. + try: + # Check if there's a commit on master with the same tree (source_id) + master_commits = self.sh.git( + "log", + "--format=%H %T", + f"{self.remote_name}/{self.base}", + "-n", "100", # Check last 100 commits + ) + for line in master_commits.split("\n"): + if not line.strip(): + continue + commit_hash, tree_hash = line.split() + if tree_hash == diff.source_id: + # Found a commit on master with the same tree, so this commit + # was landed (just with a different commit message/hash) + return None + except Exception: + pass + # Didn't find a matching commit on master, so this is a modified closed commit + raise RuntimeError( + f"Cannot ghstack a stack with closed PR #{elab_diff.number} whose branch was deleted. " + "If you were just trying to update a later PR in the stack, `git rebase` and try again. " + "Otherwise, you may have been trying to update a PR that was already closed. " + "To disassociate your update from the old PR and open a new PR, " + "run `ghstack unlink`, `git rebase` and then try again." + ) + elif diff.source_id != elab_diff.remote_source_id: + # The commit has been modified locally + raise RuntimeError( + f"Cannot ghstack a stack with closed PR #{elab_diff.number} whose branch was deleted. " + "If you were just trying to update a later PR in the stack, `git rebase` and try again. " + "Otherwise, you may have been trying to update a PR that was already closed. " + "To disassociate your update from the old PR and open a new PR, " + "run `ghstack unlink`, `git rebase` and then try again." + ) return None # Edge case: check if the commit is empty; if so skip submitting diff --git a/test/land/default_branch_change.py.test b/test/land/default_branch_change.py.test index 7041dc4..5f35ef5 100644 --- a/test/land/default_branch_change.py.test +++ b/test/land/default_branch_change.py.test @@ -107,8 +107,8 @@ gh_land(diff2.pr_url) assert_expected_inline( get_upstream_sh().git("log", "--oneline", "master"), """\ -d779d84 Commit B -542f79d Commit A +838086c Commit B (#501) +da05922 Commit A (#500) dc8bfe4 Initial commit""", ) assert_expected_inline( diff --git a/test/land/ff.py.test b/test/land/ff.py.test index c2a1a3d..aa32259 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"), """\ -8927014 Commit A +8b039fb Commit A (#500) dc8bfe4 Initial commit""", ) diff --git a/test/land/ff_stack.py.test b/test/land/ff_stack.py.test index 55c9ece..adb9cde 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"), """\ -b6c40ad Commit B -851cf96 Commit A +34cfe12 Commit B (#501) +049a42c Commit A (#500) dc8bfe4 Initial commit""", ) diff --git a/test/land/ff_stack_two_phase.py.test b/test/land/ff_stack_two_phase.py.test index 85f6212..c405090 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"), """\ -b6c40ad Commit B -851cf96 Commit A +34cfe12 Commit B (#501) +049a42c Commit A (#500) dc8bfe4 Initial commit""", ) diff --git a/test/land/invalid_resubmit.py.test b/test/land/invalid_resubmit.py.test index d50f9e8..5395730 100644 --- a/test/land/invalid_resubmit.py.test +++ b/test/land/invalid_resubmit.py.test @@ -44,16 +44,16 @@ else: This is commit A - * 98fcb03 New PR + * d700a19 New PR Repository state: - * 98fcb03 (gh/ezyang/1/head) + * d700a19 (gh/ezyang/1/head) | New PR - * 0d09e7d (gh/ezyang/1/base) + * 239fc10 (gh/ezyang/1/base) | New PR (base update) - * 8927014 (HEAD -> master) - | Commit A + * 8b039fb (HEAD -> master) + | Commit A (#500) * dc8bfe4 Initial commit """ diff --git a/test/land/non_ff.py.test b/test/land/non_ff.py.test index 7ae7ebf..1bcc832 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"), """\ -d43d06e Commit A +0388cc8 Commit A (#500) 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 751b1fd..3c20bf7 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"), """\ -ec1d0de Commit B -d8a6272 Commit A +3657657 Commit B (#501) +18a5a80 Commit A (#500) 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 412304f..ef1dd69 100644 --- a/test/land/update_after_land.py.test +++ b/test/land/update_after_land.py.test @@ -55,17 +55,17 @@ else: This is commit B - * c6c8f43 Run 3 + * 0b71837 Run 3 * 16e1e12 Initial 1 Repository state: - * c6c8f43 (gh/ezyang/2/head) + * 0b71837 (gh/ezyang/2/head) |\\ Run 3 - | * 36376f9 (gh/ezyang/2/base) + | * 16c66a5 (gh/ezyang/2/base) | |\\ Run 3 (base update) - | | * 9bf93f4 (HEAD -> master) - | | | Commit A + | | * 71f0c87 (HEAD -> master) + | | | Commit A (#500) | | * 7f0288c | | | Commit U * | | 16e1e12 From ae880479e366c02f0be8bd5fbaab75873665078a Mon Sep 17 00:00:00 2001 From: vmoens Date: Tue, 13 Jan 2026 17:04:10 -0800 Subject: [PATCH 2/5] fix-failing --- src/ghstack/land.py | 52 +++++++++++++++++++++++-------------------- src/ghstack/submit.py | 7 ++++-- 2 files changed, 33 insertions(+), 26 deletions(-) diff --git a/src/ghstack/land.py b/src/ghstack/land.py index f018594..dc888f7 100644 --- a/src/ghstack/land.py +++ b/src/ghstack/land.py @@ -148,33 +148,37 @@ def main( for orig_ref, pr_resolved in stack_orig_refs: try: sh.git("cherry-pick", f"{remote_name}/{orig_ref}") - # Add PR number to commit message like GitHub does - commit_msg = sh.git("log", "-1", "--pretty=%B") - # Get the original author and committer dates to preserve the commit hash - author_date = sh.git("log", "-1", "--pretty=%aD") - committer_date = sh.git("log", "-1", "--pretty=%cD") - lines = commit_msg.split("\n") - if lines: - # Add PR number to the subject line (first line) - subject = lines[0].rstrip() - # Only add if not already present - pr_tag = f"(#{pr_resolved.number})" - if pr_tag not in subject: - subject = f"{subject} {pr_tag}" - lines[0] = subject - new_msg = "\n".join(lines) - # Preserve dates to keep the commit hash consistent - sh.sh( - "git", - "commit", - "--amend", - "-m", - new_msg, - env={"GIT_AUTHOR_DATE": author_date, "GIT_COMMITTER_DATE": committer_date}, - ) except BaseException: sh.git("cherry-pick", "--abort") raise + + # Add PR number to commit message like GitHub does + commit_msg = sh.git("log", "-1", "--pretty=%B") + # Get the original author and committer dates to preserve the commit hash + author_date = sh.git("log", "-1", "--pretty=%aD") + committer_date = sh.git("log", "-1", "--pretty=%cD") + lines = commit_msg.split("\n") + if lines: + # Add PR number to the subject line (first line) + subject = lines[0].rstrip() + # Only add if not already present + pr_tag = f"(#{pr_resolved.number})" + if pr_tag not in subject: + subject = f"{subject} {pr_tag}" + lines[0] = subject + new_msg = "\n".join(lines) + # Preserve dates to keep the commit hash consistent + sh.sh( + "git", + "commit", + "--amend", + "-m", + new_msg, + env={ + "GIT_AUTHOR_DATE": author_date, + "GIT_COMMITTER_DATE": committer_date, + }, + ) # All good! Push! maybe_force_arg = [] diff --git a/src/ghstack/submit.py b/src/ghstack/submit.py index 74cbc92..dffdf1d 100644 --- a/src/ghstack/submit.py +++ b/src/ghstack/submit.py @@ -885,7 +885,9 @@ def elaborate_diff( else: remote_summary = ghstack.git.split_header(rev_list)[0] m_remote_source_id = RE_GHSTACK_SOURCE_ID.search(remote_summary.commit_msg) - remote_source_id = m_remote_source_id.group(1) if m_remote_source_id else None + remote_source_id = ( + m_remote_source_id.group(1) if m_remote_source_id else None + ) m_comment_id = RE_GHSTACK_COMMENT_ID.search(remote_summary.commit_msg) comment_id = int(m_comment_id.group(1)) if m_comment_id else None @@ -932,7 +934,8 @@ def process_commit( "log", "--format=%H %T", f"{self.remote_name}/{self.base}", - "-n", "100", # Check last 100 commits + "-n", + "100", # Check last 100 commits ) for line in master_commits.split("\n"): if not line.strip(): From 2ba4e2d6fe255f8722c4dd459ed21aac706cf896 Mon Sep 17 00:00:00 2001 From: vmoens Date: Tue, 13 Jan 2026 17:27:12 -0800 Subject: [PATCH 3/5] fix-failing --- src/ghstack/land.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ghstack/land.py b/src/ghstack/land.py index dc888f7..6d0af5e 100644 --- a/src/ghstack/land.py +++ b/src/ghstack/land.py @@ -172,8 +172,9 @@ def main( "git", "commit", "--amend", - "-m", - new_msg, + "-F", + "-", + input=new_msg, env={ "GIT_AUTHOR_DATE": author_date, "GIT_COMMITTER_DATE": committer_date, From 5647e5d3a32b75e9cef62869601de1a4179f6e09 Mon Sep 17 00:00:00 2001 From: vmoens Date: Tue, 13 Jan 2026 17:39:32 -0800 Subject: [PATCH 4/5] fixes --- src/ghstack/land.py | 3 +-- test/land/default_branch_change.py.test | 4 ++-- test/land/ff.py.test | 2 +- test/land/ff_stack.py.test | 4 ++-- test/land/ff_stack_two_phase.py.test | 4 ++-- test/land/invalid_resubmit.py.test | 8 ++++---- test/land/non_ff.py.test | 2 +- test/land/non_ff_stack_two_phase.py.test | 4 ++-- test/land/update_after_land.py.test | 8 ++++---- 9 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/ghstack/land.py b/src/ghstack/land.py index 6d0af5e..19b3696 100644 --- a/src/ghstack/land.py +++ b/src/ghstack/land.py @@ -168,8 +168,7 @@ def main( lines[0] = subject new_msg = "\n".join(lines) # Preserve dates to keep the commit hash consistent - sh.sh( - "git", + sh.git( "commit", "--amend", "-F", diff --git a/test/land/default_branch_change.py.test b/test/land/default_branch_change.py.test index 5f35ef5..7160e23 100644 --- a/test/land/default_branch_change.py.test +++ b/test/land/default_branch_change.py.test @@ -107,8 +107,8 @@ gh_land(diff2.pr_url) assert_expected_inline( get_upstream_sh().git("log", "--oneline", "master"), """\ -838086c Commit B (#501) -da05922 Commit A (#500) +6b7e56e Commit B (#501) +1132c50 Commit A (#500) dc8bfe4 Initial commit""", ) assert_expected_inline( diff --git a/test/land/ff.py.test b/test/land/ff.py.test index aa32259..6e30a9d 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"), """\ -8b039fb Commit A (#500) +d518c9f Commit A (#500) dc8bfe4 Initial commit""", ) diff --git a/test/land/ff_stack.py.test b/test/land/ff_stack.py.test index adb9cde..5460c9f 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"), """\ -34cfe12 Commit B (#501) -049a42c Commit A (#500) +4099517 Commit B (#501) +c28edd5 Commit A (#500) dc8bfe4 Initial commit""", ) diff --git a/test/land/ff_stack_two_phase.py.test b/test/land/ff_stack_two_phase.py.test index c405090..d73dff2 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"), """\ -34cfe12 Commit B (#501) -049a42c Commit A (#500) +4099517 Commit B (#501) +c28edd5 Commit A (#500) dc8bfe4 Initial commit""", ) diff --git a/test/land/invalid_resubmit.py.test b/test/land/invalid_resubmit.py.test index 5395730..1ee1657 100644 --- a/test/land/invalid_resubmit.py.test +++ b/test/land/invalid_resubmit.py.test @@ -44,15 +44,15 @@ else: This is commit A - * d700a19 New PR + * d1c3c7e New PR Repository state: - * d700a19 (gh/ezyang/1/head) + * d1c3c7e (gh/ezyang/1/head) | New PR - * 239fc10 (gh/ezyang/1/base) + * 5f392f5 (gh/ezyang/1/base) | New PR (base update) - * 8b039fb (HEAD -> master) + * d518c9f (HEAD -> master) | Commit A (#500) * dc8bfe4 Initial commit diff --git a/test/land/non_ff.py.test b/test/land/non_ff.py.test index 1bcc832..7c9aab9 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"), """\ -0388cc8 Commit A (#500) +8b61aeb Commit A (#500) 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 3c20bf7..ecc45a1 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"), """\ -3657657 Commit B (#501) -18a5a80 Commit A (#500) +402e96c Commit B (#501) +e388a10 Commit A (#500) 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 ef1dd69..88fa5c5 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 - * 0b71837 Run 3 + * 87c9ccd Run 3 * 16e1e12 Initial 1 Repository state: - * 0b71837 (gh/ezyang/2/head) + * 87c9ccd (gh/ezyang/2/head) |\\ Run 3 - | * 16c66a5 (gh/ezyang/2/base) + | * a800ca6 (gh/ezyang/2/base) | |\\ Run 3 (base update) - | | * 71f0c87 (HEAD -> master) + | | * 70eb094 (HEAD -> master) | | | Commit A (#500) | | * 7f0288c | | | Commit U From e731c458005b76088777d316e1ef4f49266b0309 Mon Sep 17 00:00:00 2001 From: vmoens Date: Tue, 13 Jan 2026 17:46:43 -0800 Subject: [PATCH 5/5] linter --- src/ghstack/land.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ghstack/land.py b/src/ghstack/land.py index 19b3696..08adfb6 100644 --- a/src/ghstack/land.py +++ b/src/ghstack/land.py @@ -151,7 +151,7 @@ def main( except BaseException: sh.git("cherry-pick", "--abort") raise - + # Add PR number to commit message like GitHub does commit_msg = sh.git("log", "-1", "--pretty=%B") # Get the original author and committer dates to preserve the commit hash