Rewrite ghstack head/base commit construction algorithm

Signed-off-by: Edward Z. Yang <[email protected]> ghstack-source-id: 82f19adf911174e95fe358163d2e4be9e964599e Pull Request resolved: #200
ezyang · Dec 5, 2023 · e4d252a · e4d252a
1 parent 9228889
commit e4d252a
Show file tree

Hide file tree

Showing 5 changed files with 1,012 additions and 896 deletions.
diff --git a/ghstack/cli.py b/ghstack/cli.py
@@ -58,6 +58,7 @@ def cli_context(
 @click.option("--no-skip", is_flag=True, hidden=True)
 @click.option("--draft", is_flag=True, hidden=True)
 @click.option("--base", "-B", default=None, hidden=True)
+@click.option("--stack/--no-stack", "-s/-S", is_flag=True, hidden=True)
 def main(
     ctx: click.Context,
     debug: bool,
@@ -68,6 +69,7 @@ def main(
     no_skip: bool,
     draft: bool,
     base: Optional[str],
+    stack: bool,
 ) -> None:
     """
     Submit stacks of diffs to Github
@@ -84,6 +86,7 @@ def main(
             no_skip=no_skip,
             draft=draft,
             base=base,
+            stack=stack,
         )
 
 
@@ -207,6 +210,23 @@ def status(pull_request: str) -> None:
     help="Branch to base the stack off of; "
     "defaults to the default branch of a repository",
 )
+@click.option(
+    "--stack/--no-stack",
+    "-s/-S",
+    is_flag=True,
+    default=True,
+    help="Submit the entire of stack of commits reachable from HEAD, versus only single commits.  "
+    "This affects the meaning of REVS.  With --stack, we submit all commits that "
+    "are reachable from REVS, excluding commits already on the base branch.  Revision ranges "
+    "supported by git rev-list are also supported.  "
+    "With --no-stack, we support only non-range identifiers, and will submit each commit "
+    "listed in the command line.",
+)
+@click.argument(
+    "revs",
+    nargs=-1,
+    metavar="REVS",
+)
 def submit(
     message: str,
     update_fields: bool,
@@ -215,6 +235,8 @@ def submit(
     no_skip: bool,
     draft: bool,
     base: Optional[str],
+    revs: Tuple[str, ...],
+    stack: bool,
 ) -> None:
     """
     Submit or update a PR stack
@@ -233,6 +255,8 @@ def submit(
             github_url=config.github_url,
             remote_name=config.remote_name,
             base=base,
+            revs=revs,
+            stack=stack,
         )
 
 

diff --git a/ghstack/diff.py b/ghstack/diff.py
@@ -81,6 +81,13 @@ class Diff:
     # a valid identifier would be the tree hash of the commit (rather
     # than the commit hash itself); in Phabricator it could be the
     # version of the diff.
+    #
+    # It is OK for this source id to wobble even if the tree stays the
+    # same.  This simply means we will think there are changes even
+    # if there aren't any, which should be safe (but just generate
+    # annoying updates).  What we would like is for the id to quiesce:
+    # if you didn't rebase your hg rev, the source id is guaranteed to
+    # be the same.
     source_id: str
 
     # The contents of 'Pull Request resolved'.  This is None for
@@ -101,3 +108,7 @@ class Diff:
     # authorship information when constructing a rebased commit
     author_name: Optional[str]
     author_email: Optional[str]
+
+    # If this isn't actually a diff; it's a boundary commit (not part
+    # of the stack) that we've parsed for administrative purposes
+    boundary: bool
diff --git a/ghstack/git.py b/ghstack/git.py
@@ -8,7 +8,7 @@
 import ghstack.shell
 from ghstack.types import GitCommitHash, GitTreeHash
 
-RE_RAW_COMMIT_ID = re.compile(r"^(?P<commit>[a-f0-9]+)$", re.MULTILINE)
+RE_RAW_COMMIT_ID = re.compile(r"^(?P<boundary>-?)(?P<commit>[a-f0-9]+)$", re.MULTILINE)
 RE_RAW_AUTHOR = re.compile(
     r"^author (?P<author>(?P<name>[^<]+?) <(?P<email>[^>]+)>)", re.MULTILINE
 )
@@ -45,6 +45,10 @@ def title(self) -> str:
     def commit_id(self) -> GitCommitHash:
         return GitCommitHash(self._search_group(RE_RAW_COMMIT_ID, "commit"))
 
+    @cached_property
+    def boundary(self) -> bool:
+        return self._search_group(RE_RAW_COMMIT_ID, "boundary") == "-"
+
     @cached_property
     def parents(self) -> List[GitCommitHash]:
         return [
@@ -75,27 +79,21 @@ def split_header(s: str) -> List[CommitHeader]:
     return list(map(CommitHeader, s.split("\0")[:-1]))
 
 
-def parse_header(s: str, github_url: str) -> List[ghstack.diff.Diff]:
-    def convert(h: CommitHeader) -> ghstack.diff.Diff:
-        parents = h.parents
-        if len(parents) != 1:
-            raise RuntimeError(
-                "The commit {} has {} parents, which makes my head explode.  "
-                "`git rebase -i` your diffs into a stack, then try again.".format(
-                    h.commit_id, len(parents)
-                )
-            )
-        return ghstack.diff.Diff(
-            title=h.title,
-            summary=h.commit_msg,
-            oid=h.commit_id,
-            source_id=h.tree,
-            pull_request_resolved=ghstack.diff.PullRequestResolved.search(
-                h.raw_header, github_url
-            ),
-            tree=h.tree,
-            author_name=h.author_name,
-            author_email=h.author_email,
-        )
+def convert_header(h: CommitHeader, github_url: str) -> ghstack.diff.Diff:
+    return ghstack.diff.Diff(
+        title=h.title,
+        summary=h.commit_msg,
+        oid=h.commit_id,
+        source_id=h.tree,
+        pull_request_resolved=ghstack.diff.PullRequestResolved.search(
+            h.raw_header, github_url
+        ),
+        tree=h.tree,
+        author_name=h.author_name,
+        author_email=h.author_email,
+        boundary=h.boundary,
+    )
+
 
-    return list(map(convert, split_header(s)))
+def parse_header(s: str, github_url: str) -> List[ghstack.diff.Diff]:
+    return [convert_header(h, github_url) for h in split_header(s)]