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: b8adce7587d5c0fb55a08b33dbfda7a2cf67855b
Pull Request resolved: #200
  • Loading branch information
ezyang committed Dec 6, 2023
1 parent 2a2dc47 commit 4b8e712
Show file tree
Hide file tree
Showing 6 changed files with 1,157 additions and 897 deletions.
24 changes: 24 additions & 0 deletions ghstack/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -68,6 +69,7 @@ def main(
no_skip: bool,
draft: bool,
base: Optional[str],
stack: bool,
) -> None:
"""
Submit stacks of diffs to Github
Expand All @@ -84,6 +86,7 @@ def main(
no_skip=no_skip,
draft=draft,
base=base,
stack=stack,
)


Expand Down Expand Up @@ -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,
Expand All @@ -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
Expand All @@ -233,6 +255,8 @@ def submit(
github_url=config.github_url,
remote_name=config.remote_name,
base=base,
revs=revs,
stack=stack,
)


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)]
5 changes: 4 additions & 1 deletion ghstack/shell.py
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
Loading

0 comments on commit 4b8e712

Please sign in to comment.