Skip to content

Commit

Permalink
Update on "Rewrite ghstack head/base commit construction algorithm"
Browse files Browse the repository at this point in the history 
Fixes #123
Fixes #101

Signed-off-by: Edward Z. Yang <ezyang@meta.com>

[ghstack-poisoned]
  • Loading branch information
@ezyang
ezyang committed Dec 6, 2023
1 parent dc6d783 commit 590db73
Show file tree
Hide file tree
Showing 3 changed files with 189 additions and 45 deletions.
5 changes: 4 additions & 1 deletion ghstack/shell.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ def sh(
input: Optional[str] = None,
stdin: _HANDLE = None,
stdout: _HANDLE = subprocess.PIPE,
exitcode: bool = False
exitcode: bool = False,
tick: bool = False
) -> _SHELL_RET:
"""
Run a command specified by args, and return string representing
Expand All @@ -116,6 +117,8 @@ def sh(
code 0. We never raise an exception when this is True.
"""
assert not (stdin and input)
if tick:
self.test_tick()
if input:
stdin = subprocess.PIPE
if not self.quiet:
Expand Down
3 changes: 1 addition & 2 deletions ghstack/submit.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,8 +187,6 @@ def main(
"fetch", "--prune", remote_name, f"+refs/heads/*:refs/remotes/{remote_name}/*"
)

base_ref = f"{remote_name}/{default_branch}"

# There are two distinct usage patterns:
#
# 1. You may want to submit only HEAD, but not everything below it,
Expand Down Expand Up @@ -246,6 +244,7 @@ def main(
# unrelated reasons, and we don't want to treat them as non-draft if
# this happens!

base_ref = f"{remote_name}/{default_branch}"
commits_to_submit_and_boundary = []
if stack:
# Easy case, make rev-list do the hard work
Expand Down
226 changes: 184 additions & 42 deletions test_ghstack.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2736,27 +2736,26 @@ def test_fail_same_source_id(self) -> None:
RuntimeError, "occurs twice", lambda: self.gh("Should fail")
)

def test_submit_prefix_only_no_stack(self) -> None:
self.writeFileAndAdd("file1.txt", "A")
self.sh.git("commit", "-m", "Commit 1")
def make_commit(self, name: str) -> None:
self.writeFileAndAdd(f"{name}.txt", "A")
self.sh.git("commit", "-m", f"Commit {name}")
self.sh.test_tick()

self.writeFileAndAdd("file2.txt", "A")
self.sh.git("commit", "-m", "Commit 2")
self.sh.test_tick()
self.substituteRev("HEAD", "rCOM2")
def amend_commit(self, name: str) -> None:
self.writeFileAndAdd(f"{name}.txt", "A")
self.sh.git("commit", "--amend", "--no-edit", tick=True)

def test_submit_prefix_only_no_stack(self) -> None:
self.make_commit("A")
self.make_commit("B")
self.gh("Initial")
B = self.sh.git("rev-parse", "HEAD")

self.sh.git("checkout", "HEAD~")
self.writeFileAndAdd("file1.txt", "ABBA")
self.sh.git("commit", "--amend", "--no-edit")
self.sh.test_tick()

self.sh.git("cherry-pick", self.lookupRev("rCOM2"))
self.sh.test_tick()

self.amend_commit("A2")
self.sh.git("cherry-pick", B, tick=True)
self.gh("Update base only", revs=["HEAD~"], stack=False)

self.assertExpectedInline(
self.dump_github(),
"""\
Expand Down Expand Up @@ -2791,57 +2790,200 @@ def test_submit_prefix_only_no_stack(self) -> None:
)

def test_submit_suffix_only_no_stack(self) -> None:
self.writeFileAndAdd("file1.txt", "A")
self.sh.git("commit", "-m", "Commit 1")
self.sh.test_tick()
self.make_commit("A")
self.make_commit("B")
self.gh("Initial")
B = self.sh.git("rev-parse", "HEAD")

self.writeFileAndAdd("file2.txt", "A")
self.sh.git("commit", "-m", "Commit 2")
self.sh.test_tick()
self.sh.git("checkout", "HEAD~")
self.amend_commit("A2")
self.sh.git("cherry-pick", B, tick=True)
self.gh("Update head only", revs=["HEAD"], stack=False)

self.assertExpectedInline(
self.dump_github(),
"""\
[O] #500 Commit A (gh/ezyang/1/head -> gh/ezyang/1/base)
Stack:
* #501
* __->__ #500
* cb3c5eb (gh/ezyang/1/head)
| Commit A
* 9e2ff2f (gh/ezyang/1/base)
Update base for Initial on "Commit A"
[O] #501 Commit B (gh/ezyang/2/head -> gh/ezyang/2/base)
Stack:
* __->__ #501
* b3d3bb5 (gh/ezyang/2/head)
|\\ Update head only on "Commit B"
| * 3d127c6 (gh/ezyang/2/base)
| | Update base for Update head only on "Commit B"
* | 3cf9ec1
|/ Commit B
* 97d9a92
Update base for Initial on "Commit B"
""",
)

def test_submit_prefix_only_stack(self) -> None:
self.make_commit("A")
self.make_commit("B")
self.make_commit("C")
self.gh("Initial")
self.substituteRev("HEAD", "rCOM2")
B = self.sh.git("rev-parse", "HEAD~")
C = self.sh.git("rev-parse", "HEAD")

self.sh.git("checkout", "HEAD~")
self.writeFileAndAdd("file1.txt", "ABBA")
self.sh.git("commit", "--amend", "--no-edit")
self.sh.test_tick()
self.sh.git("checkout", "HEAD~~")
self.amend_commit("A2")
self.sh.git("cherry-pick", B, tick=True)
self.sh.git("cherry-pick", C, tick=True)
self.gh("Don't update C", revs=["HEAD~"], stack=True)

self.sh.git("cherry-pick", self.lookupRev("rCOM2"))
self.sh.test_tick()
self.assertExpectedInline(
self.dump_github(),
"""\
[O] #500 Commit A (gh/ezyang/1/head -> gh/ezyang/1/base)
Stack:
* #501
* __->__ #500
* c70b354 (gh/ezyang/1/head)
| Don't update C on "Commit A"
* 290340a
| Commit A
* 1fa4e09 (gh/ezyang/1/base)
Update base for Initial on "Commit A"
[O] #501 Commit B (gh/ezyang/2/head -> gh/ezyang/2/base)
Stack:
* __->__ #501
* #500
* ebfcd77 (gh/ezyang/2/head)
|\\ Don't update C on "Commit B"
| * 78a7d98 (gh/ezyang/2/base)
| | Update base for Don't update C on "Commit B"
* | ff5373b
|/ Commit B
* 31b98af
Update base for Initial on "Commit B"
[O] #502 Commit C (gh/ezyang/3/head -> gh/ezyang/3/base)
Stack:
* __->__ #502
* #501
* #500
* 3a7e22b (gh/ezyang/3/head)
| Commit C
* 4f0f679 (gh/ezyang/3/base)
Update base for Initial on "Commit C"
""",
)

def test_submit_range_only_stack(self) -> None:
self.make_commit("A")
self.make_commit("B")
self.make_commit("C")
self.make_commit("D")
self.gh("Initial")
B = self.sh.git("rev-parse", "HEAD~~")
C = self.sh.git("rev-parse", "HEAD~")
D = self.sh.git("rev-parse", "HEAD")

self.sh.git("checkout", "HEAD~~~")
self.amend_commit("A2")
self.sh.git("cherry-pick", B, tick=True)
self.sh.git("cherry-pick", C, tick=True)
self.sh.git("cherry-pick", D, tick=True)
self.gh("Update B and C only", revs=["HEAD~~~..HEAD~"], stack=True)

self.gh("Update head only", revs=["HEAD"], stack=False)
self.assertExpectedInline(
self.dump_github(),
"""\
[O] #500 Commit 1 (gh/ezyang/1/head -> gh/ezyang/1/base)
[O] #500 Commit A (gh/ezyang/1/head -> gh/ezyang/1/base)
Stack:
* #503
* #502
* #501
* __->__ #500
* 9820f4b (gh/ezyang/1/head)
| Commit 1
* 9f734b6 (gh/ezyang/1/base)
Update base for Initial on "Commit 1"
* af9017a (gh/ezyang/1/head)
| Commit A
* 65b6341 (gh/ezyang/1/base)
Update base for Initial on "Commit A"
[O] #501 Commit 2 (gh/ezyang/2/head -> gh/ezyang/2/base)
[O] #501 Commit B (gh/ezyang/2/head -> gh/ezyang/2/base)
Stack:
* #502
* __->__ #501
* 0887253 (gh/ezyang/2/head)
|\\ Update head only on "Commit 2"
| * cb34299 (gh/ezyang/2/base)
| | Update base for Update head only on "Commit 2"
* | b7e67b6
|/ Commit 2
* ae5961f
Update base for Initial on "Commit 2"
* d18dfb9 (gh/ezyang/2/head)
|\\ Update B and C only on "Commit B"
| * d876758 (gh/ezyang/2/base)
| | Update base for Update B and C only on "Commit B"
* | aaa7211
|/ Commit B
* 48e3720
Update base for Initial on "Commit B"
[O] #502 Commit C (gh/ezyang/3/head -> gh/ezyang/3/base)
Stack:
* __->__ #502
* #501
* d78bd70 (gh/ezyang/3/head)
|\\ Update B and C only on "Commit C"
| * cf940ad (gh/ezyang/3/base)
| | Update base for Update B and C only on "Commit C"
* | f38afc9
|/ Commit C
* b937b45
Update base for Initial on "Commit C"
[O] #503 Commit D (gh/ezyang/4/head -> gh/ezyang/4/base)
Stack:
* __->__ #503
* #502
* #501
* #500
* d3f5f5e (gh/ezyang/4/head)
| Commit D
* 3f41f25 (gh/ezyang/4/base)
Update base for Initial on "Commit D"
""",
)
Expand Down

0 comments on commit 590db73

Please sign in to comment.