Skip to content

Commit

Permalink
Rewrite ghstack head/base commit construction algorithm
Browse files Browse the repository at this point in the history
Signed-off-by: Edward Z. Yang <[email protected]>

ghstack-source-id: 83f3f7e0641b45638d5aa74dcf7ffafb257a7172
Pull Request resolved: #200
  • Loading branch information
ezyang committed Dec 5, 2023
1 parent 9228889 commit 0eb9bf3
Show file tree
Hide file tree
Showing 5 changed files with 785 additions and 894 deletions.
3 changes: 3 additions & 0 deletions ghstack/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ def status(pull_request: str) -> None:
help="Branch to base the stack off of; "
"defaults to the default branch of a repository",
)
@click.argument("revs", nargs=-1, metavar="REVS")
def submit(
message: str,
update_fields: bool,
Expand All @@ -215,6 +216,7 @@ def submit(
no_skip: bool,
draft: bool,
base: Optional[str],
revs: List[str],
) -> None:
"""
Submit or update a PR stack
Expand All @@ -233,6 +235,7 @@ def submit(
github_url=config.github_url,
remote_name=config.remote_name,
base=base,
revs=revs,
)


Expand Down
11 changes: 11 additions & 0 deletions ghstack/diff.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
46 changes: 22 additions & 24 deletions ghstack/git.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down Expand Up @@ -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 [
Expand Down Expand Up @@ -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)]
Loading

0 comments on commit 0eb9bf3

Please sign in to comment.