diff --git a/eden/scm/sapling/ext/github/__init__.py b/eden/scm/sapling/ext/github/__init__.py index fb2ca7a802722..31e994e35cd77 100644 --- a/eden/scm/sapling/ext/github/__init__.py +++ b/eden/scm/sapling/ext/github/__init__.py @@ -50,6 +50,13 @@ def unlink_closed_pr_hint() -> str: ) +@hint("pr-submit-restack") +def pr_submit_restack_hint() -> str: + return _( + "use '@prog@ pr submit --restack' to dissolve the stack on GitHub and recreate it to match your local stack" + ) + + def reposetup(ui, repo): ui.setconfig("hooks", "post-pull.prmarker", pr_marker.cleanup_landed_pr_hook) @@ -99,6 +106,15 @@ def pull_request_command(ui, repo, *args, **opts): ("m", "message", None, _("message describing changes to updated commits")), ("d", "draft", False, _("mark new pull requests as draft")), ("o", "open", False, _("open pull requests in browser after creation")), + ( + "", + "restack", + False, + _( + "with github.pr-workflow=stacked, dissolve and recreate the " + "stack on GitHub if it has diverged from the local stack" + ), + ), ], ) def submit_cmd(ui, repo, *args, **opts): diff --git a/eden/scm/sapling/ext/github/consts/query.py b/eden/scm/sapling/ext/github/consts/query.py index ef111962c8153..5b5a4882d1410 100644 --- a/eden/scm/sapling/ext/github/consts/query.py +++ b/eden/scm/sapling/ext/github/consts/query.py @@ -81,6 +81,22 @@ } """ +# Like GRAPHQL_UPDATE_PULL_REQUEST, but does not touch the base branch. +# GitHub rejects updatePullRequest mutations that include baseRefName for +# pull requests that are part of a native stack (the stack manages base +# branches itself), so this variant is used to update only the title/body. +GRAPHQL_UPDATE_PULL_REQUEST_NO_BASE = """ +mutation ($pullRequestId: ID!, $title: String!, $body: String!) { + updatePullRequest( + input: {pullRequestId: $pullRequestId, title: $title, body: $body} + ) { + pullRequest { + id + } + } +} +""" + GRAPHQL_CREATE_BRANCH = """ mutation ($repositoryId: ID!, $name: String!, $oid: GitObjectID!) { createRef(input: {repositoryId: $repositoryId, name: $name, oid: $oid}) { diff --git a/eden/scm/sapling/ext/github/gh_submit.py b/eden/scm/sapling/ext/github/gh_submit.py index 6004ef4959eec..12a36052b693c 100644 --- a/eden/scm/sapling/ext/github/gh_submit.py +++ b/eden/scm/sapling/ext/github/gh_submit.py @@ -12,14 +12,14 @@ import enum from dataclasses import dataclass -from typing import Dict, Optional, Tuple, Union +from typing import Dict, List, Optional, Tuple, Union from sapling.i18n import _ from sapling.result import Err, Ok, Result from . import github_gh_cli as gh_cli from .consts import query -from .github_gh_cli import JsonDict +from .github_gh_cli import JsonDict, ParamValue from .pullrequest import PullRequestId _Params = Union[str, int, bool] @@ -336,18 +336,28 @@ async def update_pull_request( node_id: str, title: str, body: str, - base: str, + base: Optional[str], ) -> Result[str, str]: """Returns an "ID!" for the pull request, which should match the node_id that was passed in. + + If base is None, the base branch is left untouched. This is required for + pull requests that are part of a native GitHub stack: GitHub rejects + updatePullRequest mutations that include baseRefName for such pull + requests, as the stack manages base branches itself. """ params: Dict[str, _Params] = { - "query": query.GRAPHQL_UPDATE_PULL_REQUEST, + "query": ( + query.GRAPHQL_UPDATE_PULL_REQUEST + if base is not None + else query.GRAPHQL_UPDATE_PULL_REQUEST_NO_BASE + ), "pullRequestId": node_id, "title": title, "body": body, - "base": base, } + if base is not None: + params["base"] = base result = await gh_cli.make_request(params, hostname=hostname) if result.is_err(): return Err(result.unwrap_err()) @@ -408,3 +418,159 @@ async def get_username(hostname: str) -> Result[str, str]: return Err(result.unwrap_err()) else: return Ok(result.unwrap()["data"]["viewer"]["login"]) + + +# Native GitHub "pull request stack" REST endpoints. The stacks API is in +# public preview and requires an explicit API version header: +# https://docs.github.com/en/rest/pulls/stacks +_STACKS_API_HEADERS = {"X-GitHub-Api-Version": "2026-03-10"} + + +@dataclass +class StackDetails: + """A native GitHub pull request stack. + + https://docs.github.com/en/rest/pulls/stacks + """ + + # Number that identifies the stack within the repo. Note that GitHub + # allocates stack numbers and pull request/issue numbers from disjoint + # ranges, so a stack number never collides with a pull request number. + number: int + # URL for the stack. + url: str + # True if the stack is still open. + is_open: bool + # Numbers of the *open* pull requests in the stack, ordered from the + # bottom of the stack (closest to the trunk) to the top. Merged and + # closed pull requests are excluded. + pull_requests: List[int] + + +def _parse_stack_from_dict(stack_obj: JsonDict) -> StackDetails: + """Parses a "Pull Request Stack" object from the REST API. + + Note that merged (and otherwise closed) pull requests are excluded from + `pull_requests`: + + >>> _parse_stack_from_dict({ + ... "id": 1, + ... "number": 7, + ... "node_id": "PRS_1", + ... "url": "https://api.github.com/repos/facebook/sapling/stacks/7", + ... "open": True, + ... "base": {"ref": "main"}, + ... "created_at": "2026-07-30T00:00:00Z", + ... "pull_requests": [ + ... {"number": 101, "state": "closed", + ... "merged_at": "2026-07-30T01:00:00Z", "draft": False, + ... "head": {"ref": "pr101", "sha": "0" * 40}}, + ... {"number": 102, "state": "open", "merged_at": None, + ... "draft": False, "head": {"ref": "pr102", "sha": "1" * 40}}, + ... {"number": 103, "state": "open", "merged_at": None, + ... "draft": True, "head": {"ref": "pr103", "sha": "2" * 40}}, + ... ], + ... }) + StackDetails(number=7, url='https://api.github.com/repos/facebook/sapling/stacks/7', is_open=True, pull_requests=[102, 103]) + """ + return StackDetails( + number=stack_obj["number"], + url=stack_obj["url"], + is_open=stack_obj["open"], + pull_requests=[ + pr["number"] for pr in stack_obj["pull_requests"] if pr["state"] == "open" + ], + ) + + +async def get_stack_for_pull_request( + hostname: str, owner: str, name: str, number: int +) -> Result[Optional[StackDetails], str]: + """Returns the stack containing the specified pull request, or None if the + pull request is not part of a stack. + """ + endpoint = f"repos/{owner}/{name}/stacks?pull_request={number}" + result = await gh_cli.make_request( + {}, hostname=hostname, endpoint=endpoint, headers=_STACKS_API_HEADERS + ) + if result.is_err(): + return Err(result.unwrap_err()) + + # The response is a JSON array of stacks. Because a pull request can be in + # at most one stack, the `pull_request` filter yields at most one entry. + stacks = result.unwrap() + if not stacks: + return Ok(None) + return Ok(_parse_stack_from_dict(stacks[0])) + + +async def create_stack( + hostname: str, owner: str, name: str, pr_numbers: List[int] +) -> Result[StackDetails, str]: + """Creates a native GitHub stack from the specified pull requests. + + `pr_numbers` must be ordered from the bottom of the stack to the top: the + bottom pull request's base must be the trunk, and each subsequent pull + request's base branch must match the head branch of the one below it. The + caller is responsible for having set up the base branches accordingly. + """ + endpoint = f"repos/{owner}/{name}/stacks" + params: Dict[str, ParamValue] = {"pull_requests": pr_numbers} + result = await gh_cli.make_request( + params, + hostname=hostname, + endpoint=endpoint, + method="POST", + headers=_STACKS_API_HEADERS, + ) + if result.is_err(): + return Err(result.unwrap_err()) + return Ok(_parse_stack_from_dict(result.unwrap())) + + +async def add_prs_to_stack( + hostname: str, owner: str, name: str, stack_number: int, pr_numbers: List[int] +) -> Result[StackDetails, str]: + """Appends pull requests onto the top of an existing stack. + + `pr_numbers` must contain only the pull requests to add, ordered from the + current top of the stack upward: the first one's base branch must match + the head branch of the stack's current top pull request. + """ + endpoint = f"repos/{owner}/{name}/stacks/{stack_number}/add" + params: Dict[str, ParamValue] = {"pull_requests": pr_numbers} + result = await gh_cli.make_request( + params, + hostname=hostname, + endpoint=endpoint, + method="POST", + headers=_STACKS_API_HEADERS, + ) + if result.is_err(): + return Err(result.unwrap_err()) + return Ok(_parse_stack_from_dict(result.unwrap())) + + +async def unstack( + hostname: str, owner: str, name: str, stack_number: int +) -> Result[Optional[StackDetails], str]: + """Removes the unmerged pull requests from a stack. + + Pull requests that cannot be unstacked (e.g., merged or queued for merge) + are left in place. Returns the updated stack if pull requests remain in + it; returns None if the stack was dissolved entirely (HTTP 204). + """ + endpoint = f"repos/{owner}/{name}/stacks/{stack_number}/unstack" + result = await gh_cli.make_request( + {}, + hostname=hostname, + endpoint=endpoint, + method="POST", + headers=_STACKS_API_HEADERS, + ) + if result.is_err(): + return Err(result.unwrap_err()) + data = result.unwrap() + if not data: + return Ok(None) + return Ok(_parse_stack_from_dict(data)) diff --git a/eden/scm/sapling/ext/github/github_gh_cli.py b/eden/scm/sapling/ext/github/github_gh_cli.py index d87248f6c8dea..a50fde86c81e6 100644 --- a/eden/scm/sapling/ext/github/github_gh_cli.py +++ b/eden/scm/sapling/ext/github/github_gh_cli.py @@ -16,32 +16,43 @@ JsonDict = Dict[str, Any] +# Scalar value that can be passed as a field to `gh api`. +_ScalarParam = Union[str, int, bool] +# `gh api` also supports array fields via repeated `key[]=value` args. +ParamValue = Union[_ScalarParam, List[_ScalarParam]] + async def make_request( - params: Dict[str, Union[str, int, bool]], + params: Dict[str, ParamValue], hostname: str, endpoint="graphql", method: Optional[str] = None, + headers: Optional[Dict[str, str]] = None, ) -> Result[JsonDict, str]: """If successful, returns a Result whose value is parsed JSON returned by the request. """ - return await _make_request(params, hostname, endpoint, method) + return await _make_request(params, hostname, endpoint, method, headers) # Unexported extension/mock point. async def _make_request( - params: Dict[str, Union[str, int, bool]], + params: Dict[str, ParamValue], hostname: str, endpoint: str, method: Optional[str], + headers: Optional[Dict[str, str]] = None, ) -> Result[JsonDict, str]: if method: endpoint_args = ["-X", method.upper(), endpoint] else: endpoint_args = [endpoint] + header_args = list( + itertools.chain(*[["-H", f"{k}: {v}"] for (k, v) in (headers or {}).items()]) + ) args = ( ["gh", "api", "--hostname", hostname] + + header_args + endpoint_args + list(itertools.chain(*[_format_param(k, v) for (k, v) in params.items()])) ) @@ -68,7 +79,13 @@ async def _make_request( response = None if proc.returncode == 0: - assert response is not None + if response is None: + # Some REST endpoints return "204 No Content" on success (e.g., + # dissolving a pull request stack), in which case `gh api` prints + # no JSON to parse. + if not stdout.strip(): + return Ok({}) + return Err(f"could not parse JSON from response: {stdout.decode()}") assert "errors" not in response return Ok(response) elif response is not None: @@ -82,7 +99,38 @@ async def _make_request( ) -def _format_param(key: str, value: Union[str, int, bool]) -> List[str]: +def _format_param(key: str, value: ParamValue) -> List[str]: + r"""Formats a param as a list of arguments to pass to `gh api`. + + >>> _format_param("body", "hello") + ['-f', 'body=hello'] + >>> _format_param("number", 42) + ['-F', 'number=42'] + >>> _format_param("draft", True) + ['-F', 'draft=true'] + + Array values use the `gh api` repeated-field syntax, e.g. + `-F "pull_requests[]=101" -F "pull_requests[]=102"`: + + >>> _format_param("pull_requests", [101, 102]) + ['-F', 'pull_requests[]=101', '-F', 'pull_requests[]=102'] + >>> _format_param("labels", ["bug", "help wanted"]) + ['-f', 'labels[]=bug', '-f', 'labels[]=help wanted'] + + An empty array is passed explicitly as `key[]` without a value (per the + `gh api` manual) rather than dropped: an empty array and a missing field + can have different meanings to an endpoint: + + >>> _format_param("empty", []) + ['-F', 'empty[]'] + """ + if isinstance(value, list): + if not value: + return ["-F", f"{key}[]"] + return list( + itertools.chain(*[_format_param(f"{key}[]", v) for v in value]) + ) + # In Python, bool is a subclass of int, so check it first. if isinstance(value, bool): opt = "-F" diff --git a/eden/scm/sapling/ext/github/mock_utils.py b/eden/scm/sapling/ext/github/mock_utils.py index added9869489b..e6ea85e985c0d 100644 --- a/eden/scm/sapling/ext/github/mock_utils.py +++ b/eden/scm/sapling/ext/github/mock_utils.py @@ -11,7 +11,7 @@ from sapling.ext.github.consts import query from sapling.ext.github.gh_submit import PullRequestState from sapling.ext.github.pull_request_body import title_and_body -from sapling.result import Ok, Result +from sapling.result import Err, Ok, Result from .consts import GITHUB_HOSTNAME from .github_gh_cli import JsonDict @@ -28,8 +28,11 @@ REPO_ID = "R_test_github_repo" USER_NAME = "facebook_username" -ParamsType = Dict[str, Union[bool, int, str]] -MakeRequestType = Callable[[ParamsType, str, str, Optional[str]], Result[JsonDict, str]] +ParamsType = Dict[str, Union[bool, int, str, List[bool], List[int], List[str]]] +MakeRequestType = Callable[ + [ParamsType, str, str, Optional[str], Optional[Dict[str, str]]], + Result[JsonDict, str], +] RunGitCommandType = Callable[[List[str], str], bytes] @@ -75,6 +78,7 @@ class MockGitHubServer: def __init__(self, hostname: str = GITHUB_HOSTNAME): self.hostname: str = hostname self.requests: Dict[str, MockRequest] = {} + self._consumed_keys: set = set() async def make_request( self, @@ -83,10 +87,15 @@ async def make_request( hostname: str, endpoint: str = "graphql", method: Optional[str] = None, + headers: Optional[Dict[str, str]] = None, ) -> Result[JsonDict, str]: """Wrapper function for `github_gh_cli.make_request`. It reads mock data from `self.requests` instead of sending network requests. + + Note that `headers` is intentionally not part of the request key: the + headers we send (e.g., X-GitHub-Api-Version) do not affect which mock + response should be returned. """ assert real_make_request.__name__ == "_make_request", ( f"expected '_make_request', but got '{real_make_request.__name__}'" @@ -96,11 +105,45 @@ async def make_request( if key not in self.requests: raise MockRequestNotFound(key, self.requests) + self._consumed_keys.add(key) return self.requests[key].get_response() def _add_request(self, request_key: str, request: "MockRequest") -> None: self.requests[request_key] = request + def unconsumed_requests(self) -> List[str]: + """Keys of expectations that were never requested.""" + return sorted(k for k in self.requests if k not in self._consumed_keys) + + def report_unconsumed(self, ui) -> None: + """Prints a warning for every expectation that was never requested. + + Intended to be called after the command under test has finished (see + wrap_with_consumption_check). Tests that use this produce no extra + output when all expectations were consumed, so any warning makes the + test fail: this catches code paths that silently stopped making a + request the test author expected. + """ + for key in self.unconsumed_requests(): + first_line = key.splitlines()[0] + ui.status_err(f"warning, unconsumed mock request: {first_line}\n") + + +def wrap_with_consumption_check(server: "MockGitHubServer", module, funcname) -> None: + """Wraps `module.funcname` (a command function taking `ui` as its first + argument) so that unconsumed mock expectations are reported after the + command finishes, even if it aborts. + """ + from sapling import extensions + + def wrapped(orig, ui, *args, **kwargs): + try: + return orig(ui, *args, **kwargs) + finally: + server.report_unconsumed(ui) + + extensions.wrapfunction(module, funcname, wrapped) + def expect_get_repository_request( self, owner: str = OWNER, name: str = REPO_NAME ) -> "GetRepositoryRequest": @@ -279,7 +322,9 @@ def expect_update_pr_request( pr_id: str, pr_number: int, commit_msg: str, - base: str = "main", + # None means the update does not touch the base branch (used for the + # "stacked" workflow, where the native GitHub stack manages bases). + base: Optional[str] = "main", owner: str = OWNER, name: str = REPO_NAME, stack_pr_ids: Optional[List[int]] = None, @@ -302,12 +347,17 @@ def expect_update_pr_request( title, body = title_and_body(commit_msg) params: ParamsType = { - "query": query.GRAPHQL_UPDATE_PULL_REQUEST, + "query": ( + query.GRAPHQL_UPDATE_PULL_REQUEST + if base is not None + else query.GRAPHQL_UPDATE_PULL_REQUEST_NO_BASE + ), "pullRequestId": pr_id, "title": title, "body": body, - "base": base, } + if base is not None: + params["base"] = base key = create_request_key(params, self.hostname) request = UpdatePrRequest(key, pr_id) self._add_request(key, request) @@ -341,6 +391,61 @@ def expect_merge_into_branch( self._add_request(key, request) return request + def expect_get_stack_request( + self, + pr_number: int, + owner: str = OWNER, + name: str = REPO_NAME, + ) -> "GetStackRequest": + endpoint = f"repos/{owner}/{name}/stacks?pull_request={pr_number}" + key = create_request_key({}, self.hostname, endpoint=endpoint) + request = GetStackRequest(key, owner, name) + self._add_request(key, request) + return request + + def expect_create_stack_request( + self, + pr_numbers: List[int], + owner: str = OWNER, + name: str = REPO_NAME, + ) -> "CreateStackRequest": + endpoint = f"repos/{owner}/{name}/stacks" + params: ParamsType = {"pull_requests": pr_numbers} + key = create_request_key( + params, self.hostname, endpoint=endpoint, method="POST" + ) + request = CreateStackRequest(key, owner, name, pr_numbers) + self._add_request(key, request) + return request + + def expect_add_to_stack_request( + self, + stack_number: int, + pr_numbers: List[int], + owner: str = OWNER, + name: str = REPO_NAME, + ) -> "AddToStackRequest": + endpoint = f"repos/{owner}/{name}/stacks/{stack_number}/add" + params: ParamsType = {"pull_requests": pr_numbers} + key = create_request_key( + params, self.hostname, endpoint=endpoint, method="POST" + ) + request = AddToStackRequest(key, owner, name, stack_number) + self._add_request(key, request) + return request + + def expect_unstack_request( + self, + stack_number: int, + owner: str = OWNER, + name: str = REPO_NAME, + ) -> "UnstackRequest": + endpoint = f"repos/{owner}/{name}/stacks/{stack_number}/unstack" + key = create_request_key({}, self.hostname, endpoint=endpoint, method="POST") + request = UnstackRequest(key, owner, name, stack_number) + self._add_request(key, request) + return request + class MockRequest: @abstractmethod @@ -578,6 +683,151 @@ def get_response(self) -> Result[JsonDict, str]: return self._response +def create_stack_response( + owner: str, name: str, stack_number: int, pr_numbers: List[int], is_open: bool = True +) -> JsonDict: + """Builds a "Pull Request Stack" REST API response object. + + `pr_numbers` is ordered from the bottom of the stack to the top. + """ + return { + "id": stack_number, + "number": stack_number, + "node_id": f"PRS_id_{stack_number}", + "url": f"https://github.com/{owner}/{name}/stacks/{stack_number}", + "open": is_open, + "base": {"ref": "main"}, + "created_at": "2026-07-30T00:00:00Z", + "pull_requests": [ + { + "number": n, + "state": "open", + "draft": False, + "merged_at": None, + "head": {"ref": f"pr{n}", "sha": gen_hash_hexdigest(f"pr{n}")}, + } + for n in pr_numbers + ], + } + + +class GetStackRequest(MockRequest): + def __init__(self, key: str, owner: str, name: str) -> None: + self._key = key + self._response: Optional[Result[JsonDict, str]] = None + + self._owner = owner + self._name = name + + def and_respond( + self, + stack_number: Optional[int] = None, + pr_numbers: Optional[List[int]] = None, + is_open: bool = True, + ): + """Responds with the stack containing the pull request, or an empty + list (meaning "not part of any stack") if stack_number is None. + """ + if stack_number is None: + self._response = Ok([]) + else: + self._response = Ok( + [ + create_stack_response( + self._owner, + self._name, + stack_number, + pr_numbers or [], + is_open=is_open, + ) + ] + ) + + def and_respond_error(self, message: str): + """Responds with an error, as if the stacks API request failed.""" + self._response = Err(message) + + def get_response(self) -> Result[JsonDict, str]: + if self._response is None: + raise MockResponseNotSet(self._key) + return self._response + + +class CreateStackRequest(MockRequest): + def __init__(self, key: str, owner: str, name: str, pr_numbers: List[int]) -> None: + self._key = key + self._response: Optional[Result[JsonDict, str]] = None + + self._owner = owner + self._name = name + self._pr_numbers = pr_numbers + + def and_respond(self, stack_number: int): + self._response = Ok( + create_stack_response( + self._owner, self._name, stack_number, self._pr_numbers + ) + ) + + def get_response(self) -> Result[JsonDict, str]: + if self._response is None: + raise MockResponseNotSet(self._key) + return self._response + + +class AddToStackRequest(MockRequest): + def __init__(self, key: str, owner: str, name: str, stack_number: int) -> None: + self._key = key + self._response: Optional[Result[JsonDict, str]] = None + + self._owner = owner + self._name = name + self._stack_number = stack_number + + def and_respond(self, pr_numbers: List[int]): + """`pr_numbers` is the full list of pull requests in the stack after + the addition, ordered from the bottom to the top. + """ + self._response = Ok( + create_stack_response( + self._owner, self._name, self._stack_number, pr_numbers + ) + ) + + def get_response(self) -> Result[JsonDict, str]: + if self._response is None: + raise MockResponseNotSet(self._key) + return self._response + + +class UnstackRequest(MockRequest): + def __init__(self, key: str, owner: str, name: str, stack_number: int) -> None: + self._key = key + self._response: Optional[Result[JsonDict, str]] = None + + self._owner = owner + self._name = name + self._stack_number = stack_number + + def and_respond(self, remaining_pr_numbers: Optional[List[int]] = None): + """Responds with the updated stack if pull requests remain in it, or + with an empty response (HTTP 204: stack dissolved) by default. + """ + if remaining_pr_numbers is None: + self._response = Ok({}) + else: + self._response = Ok( + create_stack_response( + self._owner, self._name, self._stack_number, remaining_pr_numbers + ) + ) + + def get_response(self) -> Result[JsonDict, str]: + if self._response is None: + raise MockResponseNotSet(self._key) + return self._response + + class MockRequestNotFound(error.Abort): def __init__(self, key: str, requests: Dict[str, MockRequest]) -> None: import textwrap diff --git a/eden/scm/sapling/ext/github/pull_request_body.py b/eden/scm/sapling/ext/github/pull_request_body.py index 46317b281e765..d0b0788b5e6a5 100644 --- a/eden/scm/sapling/ext/github/pull_request_body.py +++ b/eden/scm/sapling/ext/github/pull_request_body.py @@ -18,6 +18,7 @@ def create_pull_request_title_and_body( pr_numbers_index: int, repository: Repository, reviewstack: bool = True, + stack_list: bool = True, ) -> Tuple[str, str]: r"""Returns (title, body) for the pull request. @@ -84,6 +85,18 @@ def create_pull_request_title_and_body( * __->__ #42 * #4 + Disable the stack list entirely (used for the native "stacked" workflow, + where GitHub renders the stack in the pull request UI itself). Note that + this also suppresses the ReviewStack link: + >>> title, body = create_pull_request_title_and_body(commit_msg, pr_numbers_and_num_commits, + ... pr_numbers_index, contributor_repo, stack_list=False) + >>> print(title) + The original commit message. + >>> print(body) + Second line of message. + + + Single commit stack: >>> title, body = create_pull_request_title_and_body("Foo", [(1, 1)], 0, contributor_repo) >>> print(title) @@ -106,7 +119,7 @@ def create_pull_request_title_and_body( body = _strip_stack_information(body) extra = [] - if len(pr_numbers_and_num_commits) > 1: + if stack_list and len(pr_numbers_and_num_commits) > 1: if reviewstack: reviewstack_url = f"https://reviewstack.dev/{owner}/{name}/pull/{pr}" review_stack_message = f"Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack]({reviewstack_url})." diff --git a/eden/scm/sapling/ext/github/submit.py b/eden/scm/sapling/ext/github/submit.py index 423bdaff16bcf..ee3d9c018309c 100644 --- a/eden/scm/sapling/ext/github/submit.py +++ b/eden/scm/sapling/ext/github/submit.py @@ -34,9 +34,15 @@ def submit(ui, repo, *args, **opts) -> int: github_repo = check_github_repo(repo) is_draft = opts.get("draft") is_open = opts.get("open") + is_restack = opts.get("restack") return asyncio.run( update_commits_in_stack( - ui, repo, github_repo, is_draft=is_draft, is_open=is_open + ui, + repo, + github_repo, + is_draft=is_draft, + is_open=is_open, + restack=is_restack, ) ) @@ -70,6 +76,19 @@ class SubmitWorkflow(Enum): """ OVERLAP = "overlap" + """Like SINGLE, but additionally links the pull requests together using + GitHub's native "stacked pull requests" feature so GitHub renders the + stack natively and can merge/retarget it bottom-up: + https://docs.github.com/en/pull-requests/get-started/about-stacked-prs + """ + STACKED = "stacked" + + def uses_chained_bases(self) -> bool: + """Whether each PR in the stack uses the head branch of the PR below + it as its base branch (as opposed to all PRs sharing a common base). + """ + return self in (SubmitWorkflow.SINGLE, SubmitWorkflow.STACKED) + @staticmethod def from_config(ui) -> "SubmitWorkflow": workflow = ui.config( @@ -80,6 +99,8 @@ def from_config(ui) -> "SubmitWorkflow": return SubmitWorkflow.OVERLAP elif workflow == "single": return SubmitWorkflow.SINGLE + elif workflow == "stacked": + return SubmitWorkflow.STACKED else: # Note that "classic" is not recognized yet. ui.warn( @@ -151,7 +172,12 @@ async def get_partitions(ui, repo, store, filter) -> List[List[CommitData]]: async def update_commits_in_stack( - ui, repo, github_repo: GitHubRepo, is_draft: bool, is_open: bool = False + ui, + repo, + github_repo: GitHubRepo, + is_draft: bool, + is_open: bool = False, + restack: bool = False, ) -> int: parents = repo.dirstate.parents() if parents[0] == nullid: @@ -208,17 +234,145 @@ def get_gitdir() -> str: if not refs_to_update: ui.status_err(_("no pull requests to update\n")) + if workflow == SubmitWorkflow.STACKED: + # Even when there is nothing to push, ensure the pull requests are + # linked into a native GitHub stack. This makes stack creation + # idempotent: if it failed on a previous run (or the stack was + # modified on GitHub), re-running `sl pr submit` reconciles it. + repository = params.repository + if not repository: + repository = await get_repository_for_origin( + origin, github_repo.hostname + ) + await sync_github_stack(ui, partitions, repository, restack=restack) return 0 repository = params.repository - # For the SINGLE workflow, we must update the base branch on existing PRs - # BEFORE pushing the new branch contents. Otherwise, when commits are - # reordered in the stack, GitHub may see that a PR's commits already exist - # in its (old) base branch and auto-close the PR as "merged". + # For the STACKED workflow, consult the native GitHub stack BEFORE making + # any changes: + # + # - If the stack on GitHub has diverged from the local stack and --restack + # was not passed, abort now, before any base updates or pushes: partial + # updates against a diverged stack can corrupt it. + # - With --restack, dissolve the diverged stack now, so that subsequent + # base branch updates are not rejected by GitHub (GitHub does not allow + # changing the base branch of a pull request that is in a stack). + # - If the stack matches (or is extended by) the local stack, remember its + # members: their base branches are managed by the stack itself and must + # not be updated via the API. + github_stack: Optional[gh_submit.StackDetails] = None + github_stack_fetched = False + prs_in_github_stack = set() + if workflow == SubmitWorkflow.STACKED: + if not repository: + repository = await get_repository_for_origin(origin, github_repo.hostname) + if not repository.is_fork: + local_stack = local_open_pr_numbers(partitions) + if local_stack: + fetched, github_stack = await query_github_stack( + ui, repository, local_stack + ) + if not fetched: + # Fail closed: without knowing the state of the stack on + # GitHub, base updates and pushes could corrupt it. (The + # warning with the underlying error was already printed.) + raise error.Abort( + _( + "could not determine the state of the stack on " + "GitHub; re-run 'pr submit' to retry" + ) + ) + github_stack_fetched = True + if github_stack: + stack_prs = github_stack.pull_requests + new_pr_inserted = _has_new_pr_below_stack_top( + partitions, set(stack_prs) + ) + if stack_prs == local_stack[: len(stack_prs)] and not new_pr_inserted: + # The stack matches the local stack (possibly extended + # at the top with new pull requests). + prs_in_github_stack = set(stack_prs) + elif restack: + # Dissolving the stack is only useful if it can be + # recreated afterwards, which requires at least two + # open pull requests in the local stack. + num_new_prs = sum(1 for p in partitions if not p[0].pr) + if len(local_stack) + num_new_prs < 2: + raise error.Abort( + _( + "--restack would leave stack #%d with " + "fewer than two pull requests; dissolve " + "it on GitHub instead if that is intended" + ) + % github_stack.number + ) + # The stacks API has no "reorder" operation, so + # dissolve the diverged stack now; it is recreated + # from the local stack at the end of the submit. + unstack_result = await gh_submit.unstack( + repository.hostname, + *repository.get_upstream_owner_and_name(), + github_stack.number, + ) + if unstack_result.is_err(): + raise error.Abort( + _("failed to dissolve stack #%d: %s") + % (github_stack.number, unstack_result.unwrap_err()) + ) + # Merged or queued pull requests cannot be unstacked + # and are left in place; recreating the stack is not + # possible while they remain in the old one. + remnant = unstack_result.unwrap() + if remnant and remnant.pull_requests: + raise error.Abort( + _( + "stack #%d was only partially dissolved: " + "%s could not be unstacked (merged or " + "queued pull requests are left in place)" + ) + % (remnant.number, _pr_list(remnant.pull_requests)) + ) + github_stack = None + else: + if new_pr_inserted: + ui.status_err( + _( + "new pull requests would be inserted below " + "the top of stack #%d on GitHub (%s), which " + "requires recreating the stack; not " + "updating it\n" + ) + % ( + github_stack.number, + _pr_list(github_stack.pull_requests), + ) + ) + else: + ui.status_err( + _( + "stack #%d on GitHub (%s) does not match " + "your local stack (%s); not updating it\n" + ) + % ( + github_stack.number, + _pr_list(github_stack.pull_requests), + _pr_list(local_stack), + ) + ) + hintutil.triggershow(ui, "pr-submit-restack") + raise error.Abort( + _("stack on GitHub has diverged from your local stack") + ) + + # For workflows with chained base branches (SINGLE, STACKED), we must + # update the base branch on existing PRs BEFORE pushing the new branch + # contents. Otherwise, when commits are reordered in the stack, GitHub may + # see that a PR's commits already exist in its (old) base branch and + # auto-close the PR as "merged". # # See https://github.com/facebook/sapling/issues/1275 - if workflow == SubmitWorkflow.SINGLE: + if workflow.uses_chained_bases(): existing_prs = [ p for p in partitions if p[0].pr and p[0].pr.state == PullRequestState.OPEN ] @@ -234,9 +388,23 @@ def get_gitdir() -> str: pr = partition[0].pr if not pr or pr.state != PullRequestState.OPEN: continue + if pr.number in prs_in_github_stack: + # GitHub rejects base branch changes for pull requests + # that are in a native stack; the stack manages base + # branches itself. + continue base = repository.get_base_branch() - if index < len(partitions) - 1: - base = none_throws(partitions[index + 1][0].head_branch_name) + # Chain to the nearest partition below whose pull request is + # open (or that will get a new pull request). Closed/merged + # pull requests are skipped: using their head branches as + # bases would break the chain. + for below in partitions[index + 1 :]: + below_head = below[0] + below_pr = below_head.pr + if below_pr and below_pr.state != PullRequestState.OPEN: + continue + base = none_throws(below_head.head_branch_name) + break result = await gh_submit.update_pull_request( repository.hostname, pr.node_id, pr.title, pr.body, base ) @@ -304,6 +472,16 @@ def get_gitdir() -> str: ] await asyncio.gather(*rewrite_and_archive_requests) + if workflow == SubmitWorkflow.STACKED: + await sync_github_stack( + ui, + partitions, + repository, + restack=restack, + stack=github_stack, + stack_fetched=github_stack_fetched, + ) + # Open pull requests in browser if --open flag was specified if is_open: pr_urls = [none_throws(p[0].pr).url for p in partitions if p[0].pr] @@ -314,6 +492,198 @@ def get_gitdir() -> str: return 0 +def local_open_pr_numbers(partitions: List[List[CommitData]]) -> List[int]: + """Numbers of the open pull requests in the local stack, ordered from the + bottom of the stack to the top (as expected by the GitHub stacks API). + Note that `partitions` is ordered from the top of the stack to the bottom. + """ + return [ + none_throws(p[0].pr).number + for p in reversed(partitions) + if p[0].pr and p[0].pr.state == PullRequestState.OPEN + ] + + +def _pr_list(numbers: List[int]) -> str: + return ", ".join(f"#{n}" for n in numbers) + + +def _has_new_pr_below_stack_top( + partitions: List[List[CommitData]], stack_prs: set +) -> bool: + """True if a commit without a pull request (i.e. one that will get a new + pull request) sits below a member of the native GitHub stack. Appending + to a stack is only possible at the top, so this requires recreating the + stack even when the existing members are otherwise in order. + """ + seen_new = False + # `partitions` is ordered from the top of the stack to the bottom. + for p in reversed(partitions): + head = p[0] + pr = head.pr + if pr is None: + seen_new = True + elif ( + seen_new + and pr.state == PullRequestState.OPEN + and pr.number in stack_prs + ): + return True + return False + + +async def query_github_stack( + ui, repository: Repository, local: List[int] +) -> Tuple[bool, Optional["gh_submit.StackDetails"]]: + """Queries GitHub for the native stack containing the local stack's pull + requests, if any. Returns (fetched, stack) where `fetched` is False if the + query failed (a warning is printed in that case). + """ + owner, name = repository.get_upstream_owner_and_name() + hostname = repository.hostname + + # Query with the bottom pull request first: it is the most stable member + # of an existing stack. Also try the top one to catch the case where the + # local stack was extended (or reordered) at the bottom. + stack = None + for number in dict.fromkeys([local[0], local[-1]]): + result = await gh_submit.get_stack_for_pull_request( + hostname, owner, name, number + ) + if result.is_err(): + ui.status_err( + _("warning, could not query stacks for #%d: %s\n") + % (number, result.unwrap_err()) + ) + return False, None + stack = result.unwrap() + if stack: + break + if stack and not stack.is_open: + # A closed stack cannot be appended to or dissolved; treat it the + # same as no stack. + stack = None + return True, stack + + +async def sync_github_stack( + ui, + partitions: List[List[CommitData]], + repository: Repository, + restack: bool = False, + stack: Optional["gh_submit.StackDetails"] = None, + stack_fetched: bool = False, +) -> None: + """Links the pull requests together using GitHub's native "stacked pull + requests" feature. + + Creates the stack if it does not exist and appends new pull requests to + the top of an existing one. If the stack on GitHub has diverged from the + local stack (e.g., commits were reordered or removed), no changes are made + to it unless `restack` is True, in which case the stack on GitHub is + dissolved and recreated to match the local stack. + + Failures to sync the stack are reported as warnings rather than aborting: + at this point, the pull requests themselves have already been created or + updated successfully, and re-running `sl pr submit` will retry linking. + """ + if repository.is_fork: + # GitHub requires all branches of a stack to be in the same + # repository, so stacks are not supported across forks. + ui.status_err( + _( + "warning: GitHub does not support stacks across forks; " + "pull requests were submitted without a stack\n" + ) + ) + return + + local = local_open_pr_numbers(partitions) + if len(local) < 2: + # A stack must contain at least two pull requests. Note that GitHub + # automatically removes merged pull requests from existing stacks, so + # there is nothing to clean up here as the stack shrinks. + return + + owner, name = repository.get_upstream_owner_and_name() + hostname = repository.hostname + pr_list = _pr_list + + if not stack_fetched: + fetched, stack = await query_github_stack(ui, repository, local) + if not fetched: + return + + if stack is None: + result = await gh_submit.create_stack(hostname, owner, name, local) + if result.is_err(): + ui.status_err( + _("warning, failed to create stack for %s: %s\n") + % (pr_list(local), result.unwrap_err()) + ) + else: + ui.status_err(_("created stack: %s\n") % result.unwrap().url) + elif stack.pull_requests == local: + ui.status_err(_("stack #%d is up-to-date\n") % stack.number) + elif stack.pull_requests == local[: len(stack.pull_requests)]: + # The local stack extends the stack on GitHub at the top, so the new + # pull requests can simply be appended. + to_add = local[len(stack.pull_requests) :] + result = await gh_submit.add_prs_to_stack( + hostname, owner, name, stack.number, to_add + ) + if result.is_err(): + ui.status_err( + _("warning, failed to add %s to stack #%d: %s\n") + % (pr_list(to_add), stack.number, result.unwrap_err()) + ) + else: + ui.status_err( + _("added %s to stack #%d\n") % (pr_list(to_add), stack.number) + ) + elif restack: + # The stacks API has no "reorder" operation, so dissolve the stack and + # recreate it from the local stack. + unstack_result = await gh_submit.unstack(hostname, owner, name, stack.number) + if unstack_result.is_err(): + ui.status_err( + _("warning, failed to dissolve stack #%d: %s\n") + % (stack.number, unstack_result.unwrap_err()) + ) + return + # Merged or queued pull requests cannot be unstacked and are left in + # place; recreating the stack is not possible while they remain in + # the old one. + remnant = unstack_result.unwrap() + if remnant and remnant.pull_requests: + ui.status_err( + _( + "warning, stack #%d was only partially dissolved: %s " + "could not be unstacked (merged or queued pull requests " + "are left in place)\n" + ) + % (remnant.number, _pr_list(remnant.pull_requests)) + ) + return + result = await gh_submit.create_stack(hostname, owner, name, local) + if result.is_err(): + ui.status_err( + _("warning, failed to recreate stack for %s: %s\n") + % (pr_list(local), result.unwrap_err()) + ) + else: + ui.status_err(_("recreated stack: %s\n") % result.unwrap().url) + else: + ui.status_err( + _( + "warning: stack #%d on GitHub (%s) does not match your local " + "stack (%s); not updating it\n" + ) + % (stack.number, pr_list(stack.pull_requests), pr_list(local)) + ) + hintutil.triggershow(ui, "pr-submit-restack") + + async def rewrite_pull_request_body( partitions: List[List[CommitData]], index: int, @@ -326,9 +696,26 @@ async def rewrite_pull_request_body( # of this branch. Recall that partitions is ordered from the top of the # stack to the bottom. partition = partitions[index] - base = repository.get_base_branch() - if workflow == SubmitWorkflow.SINGLE and index < len(partitions) - 1: - base = none_throws(partitions[index + 1][0].head_branch_name) + base: Optional[str] = repository.get_base_branch() + if workflow == SubmitWorkflow.STACKED: + # GitHub rejects base branch changes for pull requests that are in a + # native stack (the stack manages base branches itself), so leave the + # base untouched when updating the title/body. New pull requests get + # the correct base at creation time. + base = None + elif workflow.uses_chained_bases() and not repository.is_fork: + # Chain to the nearest partition below whose pull request is open (or + # new). Closed/merged pull requests are skipped: using their head + # branches as bases would break the chain. For forks, chained bases + # are not possible at all (the head branches live on the fork, but a + # base branch must be a branch on the upstream repository), so the + # default base branch is kept. + for below in partitions[index + 1 :]: + below_pr = below[0].pr + if below_pr and below_pr.state != PullRequestState.OPEN: + continue + base = none_throws(below[0].head_branch_name) + break head_commit_data = partition[0] @@ -347,6 +734,10 @@ async def rewrite_pull_request_body( index, repository, reviewstack=ui.configbool("github", "pull-request-include-reviewstack"), + # For the native "stacked" workflow, GitHub renders the stack in the + # pull request UI itself, so the footer stack list (and ReviewStack + # link) would be redundant. + stack_list=workflow != SubmitWorkflow.STACKED, ) if pr.state != PullRequestState.OPEN: @@ -373,8 +764,11 @@ class SerialStrategyParams: # git push --force any heads that need updating, creating new branch names, # if necessary. refs_to_update: List[str] - # The str in the Tuple is the head branch name for the commit. - pull_requests_to_create: List[Tuple[CommitData, str]] + # The first str in the Tuple is the head branch name for the commit. The + # second is the head branch name of the partition below it in the stack + # (None if it is at the bottom), to be used as the base branch in + # workflows with chained bases. + pull_requests_to_create: List[Tuple[CommitData, str, Optional[str]]] repository: Optional[Repository] @@ -417,7 +811,7 @@ async def create_serial_strategy_params( # git push --force any heads that need updating, creating new branch names, # if necessary. refs_to_update = [] - pull_requests_to_create: List[Tuple[CommitData, str]] = [] + pull_requests_to_create: List[Tuple[CommitData, str, Optional[str]]] = [] # These are set lazily because they require GraphQL calls. next_pull_request_number = None @@ -426,6 +820,9 @@ async def create_serial_strategy_params( # Note that `partitions` is ordered from the top of the stack to the bottom, # but we want to create PRs from the bottom to the top so the PR numbers are # created in ascending order. + # Head branch of the partition below the current one, i.e. the base branch + # to use for a new pull request in workflows with chained bases. + below_branch: Optional[str] = None for partition in reversed(partitions): top = partition[0] pr = top.pr @@ -466,7 +863,12 @@ async def create_serial_strategy_params( ) refs_to_update.append(f"{hex(top.node)}:refs/heads/{branch_name}") top.head_branch_name = branch_name - pull_requests_to_create.append((top, branch_name)) + pull_requests_to_create.append((top, branch_name, below_branch)) + if not pr or pr.state == PullRequestState.OPEN: + # Only open pull requests (or commits that will get a new pull + # request) can serve as the base for the partition above: + # closed/merged head branches would break the chain. + below_branch = top.head_branch_name return SerialStrategyParams(refs_to_update, pull_requests_to_create, repository) @@ -487,7 +889,7 @@ def get_pull_request_template(commit: CommitData) -> None | str: async def create_pull_requests_serially( - commits: List[Tuple[CommitData, str]], + commits: List[Tuple[CommitData, str, Optional[str]]], workflow: SubmitWorkflow, repository: Repository, store: PullRequestStore, @@ -506,11 +908,21 @@ async def create_pull_requests_serially( # Create the pull requests in order serially to give us the best chance of # the number in the branch name matching that of the actual pull request. commits_to_update = [] - parent = None - for commit, branch_name in commits: + for commit, branch_name, below_branch in commits: base = repository.get_base_branch() - if workflow == SubmitWorkflow.SINGLE and parent: - base = none_throws(parent.head_branch_name) + if workflow.uses_chained_bases() and below_branch and not repository.is_fork: + # Use the head branch of the partition below this commit as the + # base branch, even if that pull request already existed. This is + # required for the STACKED workflow (the base branch cannot be + # corrected afterwards: GitHub rejects base branch changes for + # pull requests in a native stack) and avoids a redundant base + # update for the SINGLE workflow. + # + # For forks this is not possible: the head branches live on the + # fork, but the base branch of a pull request must be a branch on + # the upstream repository, so fall back to the default base + # branch. + base = below_branch commit_msg = commit.get_msg() title, body = title_and_body(commit_msg) @@ -545,8 +957,6 @@ async def create_pull_requests_serially( store.map_commit_to_pull_request(commit.node, pr_id) commits_to_update.append((commit, pr_id)) - parent = commit - # Now that all of the pull requests have been created, update the .pr field # on each CommitData. We prioritize the create_pull_request() calls to try # to get the pull request numbers to match up. @@ -599,7 +1009,11 @@ async def create_placeholder_strategy_params( commit=commit, parent=parent_commit ) commits_that_need_pull_requests.append(commit_needs_pr) - parent_commit = commit + if not pr or pr.state == PullRequestState.OPEN: + # Only open pull requests (or commits that will get a new pull + # request) can serve as the parent for chained bases: + # closed/merged head branches would break the chain. + parent_commit = commit # Reserve one GitHub issue number for each pull request (in parallel) and # then assign them in increasing order. Also ensure head_branch_name is set @@ -657,8 +1071,11 @@ async def create_pull_request(params: PullRequestParams): issue_number = params.number # Note that "overlapping" pull requests will all share the same base. + # For forks, chained bases are not possible either: the head branches + # live on the fork, but the base branch of a pull request must be a + # branch on the upstream repository. base = base_branch_for_repo - if workflow == SubmitWorkflow.SINGLE: + if workflow.uses_chained_bases() and not repository.is_fork: parent = params.parent if parent: base = none_throws(parent.head_branch_name) diff --git a/eden/scm/tests/github/mock_closed_mid_stack.py b/eden/scm/tests/github/mock_closed_mid_stack.py new file mode 100644 index 0000000000000..284cb55beb0d6 --- /dev/null +++ b/eden/scm/tests/github/mock_closed_mid_stack.py @@ -0,0 +1,76 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions, scmutil +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.gh_submit import PullRequestState +from sapling.ext.github.mock_utils import ( + mock_run_git_command, + MockGitHubServer, + wrap_with_consumption_check, +) + +# An extension to mock network requests for `sl pr submit` with +# github.pr-workflow=single when the pull request at the bottom of the stack +# (#42) is closed: the base branches of the pull requests above it must skip +# the closed pull request's head branch (which would break the chain) and +# fall through to the default base branch. +# +# This mock is set up in `reposetup` so that expectations can be derived from +# the actual commit hashes in the test repo instead of hardcoding them. + + +def setup_mock_github_server(repo) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond() + + # All three commits are linked to pull requests via "Pull Request + # resolved" lines in their commit messages. None of the head OIDs match + # the local commits, so all three head branches are pushed (pushing to a + # closed pull request's branch is harmless and preexisting behavior). + github_server.expect_get_pr_details_request(42).and_respond( + "PR_id_42", state=PullRequestState.CLOSED + ) + github_server.expect_get_pr_details_request(43).and_respond("PR_id_43") + github_server.expect_get_pr_details_request(44).and_respond("PR_id_44") + + # Base updates before the push: #44 chains to the open #43 below it, but + # #43 must NOT chain to the closed #42 below it: it falls through to the + # default base branch instead. (No base update is attempted for the + # closed #42 itself.) + github_server.expect_update_pr_request( + "PR_id_44", 44, "", base="pr43" + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_43", 43, "", base="main" + ).and_respond() + + # Body rewrites follow the same base rules. The stack list footer still + # lists all three pull requests (including the closed one). + msg_two = "two\n\nPull Request resolved: https://github.com/facebook/test_github_repo/pull/43" + msg_three = "three\n\nPull Request resolved: https://github.com/facebook/test_github_repo/pull/44" + github_server.expect_update_pr_request( + "PR_id_44", 44, msg_three, base="pr43", stack_pr_ids=[42, 43, 44] + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_43", 43, msg_two, base="main", stack_pr_ids=[42, 43, 44] + ).and_respond() + + github_server.expect_get_username_request().and_respond() + + tip = scmutil.revsingle(repo, "desc(three)").hex() + github_server.expect_merge_into_branch(tip).and_respond() + + return github_server + + +def reposetup(ui, repo): + github_server = setup_mock_github_server(repo) + extensions.wrapfunction( + github_gh_cli, "_make_request", github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) + wrap_with_consumption_check(github_server, submit, "submit") diff --git a/eden/scm/tests/github/mock_closed_stack.py b/eden/scm/tests/github/mock_closed_stack.py new file mode 100644 index 0000000000000..42b021f53a8dd --- /dev/null +++ b/eden/scm/tests/github/mock_closed_stack.py @@ -0,0 +1,59 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions, scmutil +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import ( + mock_run_git_command, + MockGitHubServer, + wrap_with_consumption_check, +) + +# An extension to mock network requests for `sl pr submit` with +# github.pr-workflow=stacked when the pull request's stack on GitHub is +# closed. A closed stack cannot be appended to or dissolved, so it is treated +# the same as no stack: the base branch is updated normally and no stack +# operations are attempted (a single open pull request cannot form a new +# stack). + + +def setup_mock_github_server(repo) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond() + + # #42's head is stale (the commit was amended), so it is pushed. + github_server.expect_get_pr_details_request(42).and_respond("PR_id_42") + + # The stack containing #42 is closed. + github_server.expect_get_stack_request(42).and_respond( + stack_number=100, pr_numbers=[42, 43], is_open=False + ) + + # Since the closed stack does not manage #42 anymore, its base branch is + # updated via the API. + github_server.expect_update_pr_request( + "PR_id_42", 42, "", base="main" + ).and_respond() + + msg = "one\n\nPull Request resolved: https://github.com/facebook/test_github_repo/pull/42" + github_server.expect_update_pr_request( + "PR_id_42", 42, msg, base=None + ).and_respond() + + github_server.expect_get_username_request().and_respond() + tip = scmutil.revsingle(repo, "desc(one)").hex() + github_server.expect_merge_into_branch(tip).and_respond() + + return github_server + + +def reposetup(ui, repo): + github_server = setup_mock_github_server(repo) + extensions.wrapfunction( + github_gh_cli, "_make_request", github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) + wrap_with_consumption_check(github_server, submit, "submit") diff --git a/eden/scm/tests/github/mock_create_prs.py b/eden/scm/tests/github/mock_create_prs.py index 2bd5cd653b503..7979f4bd9d7f6 100644 --- a/eden/scm/tests/github/mock_create_prs.py +++ b/eden/scm/tests/github/mock_create_prs.py @@ -26,14 +26,19 @@ def setup_mock_github_server(ui) -> MockGitHubServer: (43, "two\n"), ] - single = ui.config("github", "pr-workflow") == "single" + # Both "single" and "stacked" chain each PR's base to the PR below it. + workflow = ui.config("github", "pr-workflow") + chained = workflow in ("single", "stacked") + # The "stacked" workflow omits the stack list footer from PR bodies + # because GitHub renders the stack natively. + stacked = workflow == "stacked" for idx, (num, msg) in enumerate(prs): title, body = title_and_body(msg) head = f"pr{num}" base = "main" - if single and idx > 0: + if chained and idx > 0: base = "pr%d" % prs[idx - 1][0] github_server.expect_create_pr_request( @@ -47,7 +52,11 @@ def setup_mock_github_server(ui) -> MockGitHubServer: github_server.expect_get_pr_details_request(num).and_respond(pr_id) github_server.expect_update_pr_request( - pr_id, num, msg, base=base, stack_pr_ids=[pr[0] for pr in prs] + pr_id, + num, + msg, + base=base, + stack_pr_ids=None if stacked else [pr[0] for pr in prs], ).and_respond() github_server.expect_get_username_request().and_respond() diff --git a/eden/scm/tests/github/mock_create_prs_placeholder_fork.py b/eden/scm/tests/github/mock_create_prs_placeholder_fork.py new file mode 100644 index 0000000000000..281263a3790a5 --- /dev/null +++ b/eden/scm/tests/github/mock_create_prs_placeholder_fork.py @@ -0,0 +1,89 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions, scmutil +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import ( + mock_run_git_command, + MockGitHubServer, + wrap_with_consumption_check, +) +from sapling.ext.github.pull_request_body import title_and_body + +# An extension to mock network requests for `sl pr submit` with +# github.placeholder-strategy=true and github.pr-workflow=single when the +# repo is a fork. Chained bases are not possible for forks (the head branches +# live on the fork, but a pull request's base branch must be a branch on the +# upstream repository), so both new pull requests must be created against the +# upstream default branch, and their body rewrites must not attempt to change +# the base to a fork branch. + +UPSTREAM = { + "id": "R_upstream_repo", + "owner": {"id": "upstream_id", "login": "upstream"}, + "name": "test_github_repo", + "isFork": False, + "defaultBranchRef": {"name": "main"}, +} + + +def setup_mock_github_server(repo) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond( + is_fork=True, parent=UPSTREAM + ) + + # Placeholder issues are reserved on the upstream repository. + github_server.expect_create_pr_placeholder_request( + owner="upstream" + ).and_respond(start_number=42, num_times=2) + + prs = [ + (42, "one\n"), + (43, "two\n"), + ] + for num, msg in prs: + _title, body = title_and_body(msg) + # Despite the "single" workflow, both pull requests use the upstream + # default branch as the base: fork head branches cannot be bases. + github_server.expect_create_pr_using_placeholder_request( + body=body, + issue=num, + head=f"facebook:pr{num}", + base="main", + owner="upstream", + ).and_respond() + + pr_id = f"PR_id_{num}" + github_server.expect_get_pr_details_request( + num, owner="upstream" + ).and_respond(pr_id) + + # The body rewrite keeps the default base branch (no chaining). + github_server.expect_update_pr_request( + pr_id, + num, + msg, + base="main", + owner="upstream", + stack_pr_ids=[42, 43], + ).and_respond() + + github_server.expect_get_username_request().and_respond() + + tip = scmutil.revsingle(repo, "desc(two)").hex() + github_server.expect_merge_into_branch(tip).and_respond() + + return github_server + + +def reposetup(ui, repo): + github_server = setup_mock_github_server(repo) + extensions.wrapfunction( + github_gh_cli, "_make_request", github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) + wrap_with_consumption_check(github_server, submit, "submit") diff --git a/eden/scm/tests/github/mock_create_prs_with_open.py b/eden/scm/tests/github/mock_create_prs_with_open.py index 443871249aa37..4b853e23d6456 100644 --- a/eden/scm/tests/github/mock_create_prs_with_open.py +++ b/eden/scm/tests/github/mock_create_prs_with_open.py @@ -28,14 +28,19 @@ def setup_mock_github_server(ui) -> MockGitHubServer: (43, "two\n"), ] - single = ui.config("github", "pr-workflow") == "single" + # Both "single" and "stacked" chain each PR's base to the PR below it. + workflow = ui.config("github", "pr-workflow") + chained = workflow in ("single", "stacked") + # The "stacked" workflow omits the stack list footer from PR bodies + # because GitHub renders the stack natively. + stacked = workflow == "stacked" for idx, (num, msg) in enumerate(prs): title, body = title_and_body(msg) head = f"pr{num}" base = "main" - if single and idx > 0: + if chained and idx > 0: base = "pr%d" % prs[idx - 1][0] github_server.expect_create_pr_request( @@ -49,7 +54,11 @@ def setup_mock_github_server(ui) -> MockGitHubServer: github_server.expect_get_pr_details_request(num).and_respond(pr_id) github_server.expect_update_pr_request( - pr_id, num, msg, base=base, stack_pr_ids=[pr[0] for pr in prs] + pr_id, + num, + msg, + base=base, + stack_pr_ids=None if stacked else [pr[0] for pr in prs], ).and_respond() github_server.expect_get_username_request().and_respond() diff --git a/eden/scm/tests/github/mock_create_stacked_prs.py b/eden/scm/tests/github/mock_create_stacked_prs.py new file mode 100644 index 0000000000000..68b5d0b55dabc --- /dev/null +++ b/eden/scm/tests/github/mock_create_stacked_prs.py @@ -0,0 +1,77 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import mock_run_git_command, MockGitHubServer +from sapling.ext.github.pull_request_body import title_and_body + +# An extension to mock network requests for the initial `sl pr submit` of a +# stack of two commits with github.pr-workflow=stacked. It replaces +# `github_gh_cli.make_request` and `submit.run_git_command` with the +# corresponding mock functions. Check the `uisetup` function for how the mock +# functions are registered. + + +def setup_mock_github_server(ui) -> MockGitHubServer: + """Setup mock GitHub Server for testing happy case of `sl pr submit` with + the "stacked" workflow. + """ + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond() + + github_server.expect_guess_next_pull_request_number().and_respond() + + prs = [ + (42, "one\n"), + (43, "two\n"), + ] + + for idx, (num, msg) in enumerate(prs): + title, body = title_and_body(msg) + head = f"pr{num}" + + # Each PR's base is chained to the head branch of the PR below it. + base = "main" if idx == 0 else "pr%d" % prs[idx - 1][0] + + github_server.expect_create_pr_request( + body=body, + title=title, + head=head, + base=base, + ).and_respond(number=num) + + pr_id = f"PR_id_{num}" + github_server.expect_get_pr_details_request(num).and_respond(pr_id) + + # The "stacked" workflow omits the stack list footer from PR bodies + # (GitHub renders the stack natively), so stack_pr_ids is left unset. + # It also leaves the base branch untouched when rewriting the body + # (base=None), since the native stack manages base branches. + github_server.expect_update_pr_request( + pr_id, num, msg, base=None + ).and_respond() + + github_server.expect_get_username_request().and_respond() + + head = "1a67244b0a776bfcc3be6bf811e98c993d78ce47" + github_server.expect_merge_into_branch(head).and_respond() + + # Neither the bottom (#42) nor the top (#43) pull request is part of a + # stack yet, so a new stack is created. + github_server.expect_get_stack_request(42).and_respond() + github_server.expect_get_stack_request(43).and_respond() + github_server.expect_create_stack_request([42, 43]).and_respond(stack_number=100) + + return github_server + + +def uisetup(ui): + mock_github_server = setup_mock_github_server(ui) + extensions.wrapfunction( + github_gh_cli, "_make_request", mock_github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) diff --git a/eden/scm/tests/github/mock_create_stacked_prs_placeholder.py b/eden/scm/tests/github/mock_create_stacked_prs_placeholder.py new file mode 100644 index 0000000000000..5780cdde3c67c --- /dev/null +++ b/eden/scm/tests/github/mock_create_stacked_prs_placeholder.py @@ -0,0 +1,73 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions, scmutil +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import ( + mock_run_git_command, + MockGitHubServer, + wrap_with_consumption_check, +) +from sapling.ext.github.pull_request_body import title_and_body + +# An extension to mock network requests for the initial `sl pr submit` of a +# stack of two commits with github.pr-workflow=stacked and +# github.placeholder-strategy=true: pull request numbers are reserved via +# placeholder issues, the pull requests are created with chained bases, and +# they are linked into a native GitHub stack. + + +def setup_mock_github_server(repo) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond() + + github_server.expect_create_pr_placeholder_request().and_respond( + start_number=42, num_times=2 + ) + + prs = [ + (42, "one\n"), + (43, "two\n"), + ] + for idx, (num, msg) in enumerate(prs): + _title, body = title_and_body(msg) + + # Each PR's base is chained to the head branch of the PR below it. + base = "main" if idx == 0 else "pr%d" % prs[idx - 1][0] + github_server.expect_create_pr_using_placeholder_request( + body=body, issue=num, base=base + ).and_respond() + + pr_id = f"PR_id_{num}" + github_server.expect_get_pr_details_request(num).and_respond(pr_id) + + # The stacked workflow leaves the base untouched when rewriting the + # body, and omits the stack list footer. + github_server.expect_update_pr_request( + pr_id, num, msg, base=None + ).and_respond() + + github_server.expect_get_username_request().and_respond() + tip = scmutil.revsingle(repo, "desc(two)").hex() + github_server.expect_merge_into_branch(tip).and_respond() + + # Neither pull request is part of a stack yet, so a new stack is created. + github_server.expect_get_stack_request(42).and_respond() + github_server.expect_get_stack_request(43).and_respond() + github_server.expect_create_stack_request([42, 43]).and_respond( + stack_number=100 + ) + + return github_server + + +def reposetup(ui, repo): + github_server = setup_mock_github_server(repo) + extensions.wrapfunction( + github_gh_cli, "_make_request", github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) + wrap_with_consumption_check(github_server, submit, "submit") diff --git a/eden/scm/tests/github/mock_diverged_stack.py b/eden/scm/tests/github/mock_diverged_stack.py new file mode 100644 index 0000000000000..6ee075c72a9cc --- /dev/null +++ b/eden/scm/tests/github/mock_diverged_stack.py @@ -0,0 +1,56 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import mock_run_git_command, MockGitHubServer + +# An extension to mock network requests for `sl pr submit` with +# github.pr-workflow=stacked when the stack on GitHub has diverged from the +# local stack (e.g., it was reordered on GitHub). All local commits are +# up-to-date, so nothing is pushed. +# +# Without --restack, the diverged stack must not be modified (a warning is +# printed instead). With --restack, the stack is dissolved and recreated, so +# this mock also includes the unstack/create expectations (they are simply +# unused in the no---restack case). + +COMMIT_ONE = "ebe5b8faff36687becb7bdbca1e6a61dac428834" +COMMIT_TWO = "1a67244b0a776bfcc3be6bf811e98c993d78ce47" +COMMIT_THREE = "f4185fef85f10d46b859c30076243068b0f59245" + + +def setup_mock_github_server(ui) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond() + + for num, oid in [(42, COMMIT_ONE), (43, COMMIT_TWO), (44, COMMIT_THREE)]: + github_server.expect_get_pr_details_request(num).and_respond( + f"PR_id_{num}", head_ref_oid=oid + ) + + # The stack on GitHub has a different order than the local stack + # (#42, #43, #44). + github_server.expect_get_stack_request(42).and_respond( + stack_number=100, pr_numbers=[43, 42, 44] + ) + + # Only used with --restack: the diverged stack is dissolved (204) and + # recreated to match the local stack. + github_server.expect_unstack_request(100).and_respond() + github_server.expect_create_stack_request([42, 43, 44]).and_respond( + stack_number=101 + ) + + return github_server + + +def uisetup(ui): + mock_github_server = setup_mock_github_server(ui) + extensions.wrapfunction( + github_gh_cli, "_make_request", mock_github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) diff --git a/eden/scm/tests/github/mock_diverged_stack_dirty.py b/eden/scm/tests/github/mock_diverged_stack_dirty.py new file mode 100644 index 0000000000000..c74a1b5d395a9 --- /dev/null +++ b/eden/scm/tests/github/mock_diverged_stack_dirty.py @@ -0,0 +1,88 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions, scmutil +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import ( + mock_run_git_command, + MockGitHubServer, + wrap_with_consumption_check, +) + +# An extension to mock network requests for `sl pr submit` with +# github.pr-workflow=stacked when the stack on GitHub has diverged from the +# local stack AND there are local changes to push (the top commit was +# amended). +# +# Without --restack, the submit must abort BEFORE updating any base branches +# or pushing anything: partial updates against a diverged stack can corrupt +# it. With --restack, the diverged stack is dissolved up front, the bases and +# branches are updated, and the stack is recreated from the local stack. +# +# This mock is set up in `reposetup` so that expectations can be derived from +# the actual commit hashes in the test repo instead of hardcoding them. + + +def setup_mock_github_server(repo) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond() + + commit_one = scmutil.revsingle(repo, "desc(one)").hex() + commit_two = scmutil.revsingle(repo, "desc(two)").hex() + + github_server.expect_get_pr_details_request(42).and_respond( + "PR_id_42", head_ref_oid=commit_one + ) + github_server.expect_get_pr_details_request(43).and_respond( + "PR_id_43", head_ref_oid=commit_two + ) + # PR #44's head is stale (the commit was amended), so it needs a push. + github_server.expect_get_pr_details_request(44).and_respond("PR_id_44") + + # The stack on GitHub has a different order than the local stack + # (#42, #43, #44). It is queried up front, before any mutations. + github_server.expect_get_stack_request(42).and_respond( + stack_number=100, pr_numbers=[43, 42, 44] + ) + + # Only used with --restack: the diverged stack is dissolved up front, + # base branches are updated, the amended commit is pushed, and the stack + # is recreated from the local stack. + github_server.expect_unstack_request(100).and_respond() + github_server.expect_update_pr_request( + "PR_id_44", 44, "", base="pr43" + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_43", 43, "", base="pr42" + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_42", 42, "", base="main" + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_44", 44, "three\n", base=None + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_43", 43, "two\n", base=None + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_42", 42, "one\n", base=None + ).and_respond() + github_server.expect_get_username_request().and_respond() + tip = scmutil.revsingle(repo, "desc(three)").hex() + github_server.expect_merge_into_branch(tip).and_respond() + github_server.expect_create_stack_request([42, 43, 44]).and_respond( + stack_number=101 + ) + + return github_server + + +def reposetup(ui, repo): + github_server = setup_mock_github_server(repo) + extensions.wrapfunction( + github_gh_cli, "_make_request", github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) diff --git a/eden/scm/tests/github/mock_extend_stacked_prs.py b/eden/scm/tests/github/mock_extend_stacked_prs.py new file mode 100644 index 0000000000000..540d9fbb5b7ef --- /dev/null +++ b/eden/scm/tests/github/mock_extend_stacked_prs.py @@ -0,0 +1,87 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import mock_run_git_command, MockGitHubServer + +# An extension to mock network requests for `sl pr submit` with +# github.pr-workflow=stacked when a new commit is added on top of a stack +# whose pull requests (#42, #43) were created by a previous submit (see +# mock_create_stacked_prs.py): the new PR #44 should be appended to the +# existing stack on GitHub. + +COMMIT_ONE = "ebe5b8faff36687becb7bdbca1e6a61dac428834" +COMMIT_TWO = "1a67244b0a776bfcc3be6bf811e98c993d78ce47" +COMMIT_THREE = "f4185fef85f10d46b859c30076243068b0f59245" + + +def setup_mock_github_server(ui) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond() + + # PRs #42 and #43 already exist and are up-to-date: their head OIDs match + # the local commits, so only the new commit is pushed. + github_server.expect_get_pr_details_request(42).and_respond( + "PR_id_42", head_ref_oid=COMMIT_ONE + ) + github_server.expect_get_pr_details_request(43).and_respond( + "PR_id_43", head_ref_oid=COMMIT_TWO + ) + + # The next PR number should be 44. + github_server.expect_guess_next_pull_request_number().and_respond( + latest_issue_num=40, latest_pr_num=43 + ) + + # Existing PRs #42 and #43 are already members of native stack #100 (see + # the expect_get_stack_request below), so their base branches must NOT be + # updated before pushing: GitHub rejects base branch changes for pull + # requests that are in a stack (the stack manages bases itself). Hence + # there are no base-update expectations here. + + # The new PR is created directly against the head branch of the pull + # request below it in the stack: the base cannot be corrected afterwards, + # since GitHub rejects base branch changes for PRs in a native stack. + github_server.expect_create_pr_request( + body="", title="three", head="pr44", base="pr43" + ).and_respond(number=44) + github_server.expect_get_pr_details_request(44).and_respond("PR_id_44") + + # Body rewrites leave the base branch untouched (base=None) in the + # "stacked" workflow. + github_server.expect_update_pr_request( + "PR_id_44", 44, "three\n", base=None + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_43", 43, "two\n", base=None + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_42", 42, "one\n", base=None + ).and_respond() + + github_server.expect_get_username_request().and_respond() + github_server.expect_merge_into_branch(COMMIT_THREE).and_respond() + + # #42 is already the bottom of stack #100, and the local stack extends it + # at the top, so #44 is appended to the existing stack. The stack is + # queried up front (before any base updates or pushes). + github_server.expect_get_stack_request(42).and_respond( + stack_number=100, pr_numbers=[42, 43] + ) + github_server.expect_add_to_stack_request(100, [44]).and_respond( + pr_numbers=[42, 43, 44] + ) + + return github_server + + +def uisetup(ui): + mock_github_server = setup_mock_github_server(ui) + extensions.wrapfunction( + github_gh_cli, "_make_request", mock_github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) diff --git a/eden/scm/tests/github/mock_fork_stacked.py b/eden/scm/tests/github/mock_fork_stacked.py new file mode 100644 index 0000000000000..33bea4d63cf97 --- /dev/null +++ b/eden/scm/tests/github/mock_fork_stacked.py @@ -0,0 +1,80 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions, scmutil +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import ( + mock_run_git_command, + MockGitHubServer, + wrap_with_consumption_check, +) +from sapling.ext.github.pull_request_body import title_and_body + +# An extension to mock network requests for `sl pr submit` with +# github.pr-workflow=stacked when the repo is a fork. GitHub requires all +# branches of a stack to be in the same repository, so no native stack is +# created (a warning is printed instead), and chained bases are not possible +# (fork head branches cannot be bases on the upstream repository), so both +# pull requests are created against the upstream default branch. + +UPSTREAM = { + "id": "R_upstream_repo", + "owner": {"id": "upstream_id", "login": "upstream"}, + "name": "test_github_repo", + "isFork": False, + "defaultBranchRef": {"name": "main"}, +} + + +def setup_mock_github_server(repo) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond( + is_fork=True, parent=UPSTREAM + ) + + github_server.expect_guess_next_pull_request_number( + owner="upstream" + ).and_respond() + + prs = [ + (42, "one\n"), + (43, "two\n"), + ] + for num, msg in prs: + title, body = title_and_body(msg) + github_server.expect_create_pr_request( + body=body, + title=title, + head=f"facebook:pr{num}", + base="main", + owner="upstream", + ).and_respond(number=num) + + pr_id = f"PR_id_{num}" + github_server.expect_get_pr_details_request( + num, owner="upstream" + ).and_respond(pr_id) + + # The stacked workflow leaves the base untouched when rewriting the + # body, and omits the stack list footer. + github_server.expect_update_pr_request( + pr_id, num, msg, base=None, owner="upstream" + ).and_respond() + + github_server.expect_get_username_request().and_respond() + tip = scmutil.revsingle(repo, "desc(two)").hex() + github_server.expect_merge_into_branch(tip).and_respond() + + return github_server + + +def reposetup(ui, repo): + github_server = setup_mock_github_server(repo) + extensions.wrapfunction( + github_gh_cli, "_make_request", github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) + wrap_with_consumption_check(github_server, submit, "submit") diff --git a/eden/scm/tests/github/mock_insert_mid_stack.py b/eden/scm/tests/github/mock_insert_mid_stack.py new file mode 100644 index 0000000000000..2124719a5ec54 --- /dev/null +++ b/eden/scm/tests/github/mock_insert_mid_stack.py @@ -0,0 +1,59 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions, scmutil +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import ( + mock_run_git_command, + MockGitHubServer, + wrap_with_consumption_check, +) + +# An extension to mock network requests for `sl pr submit` with +# github.pr-workflow=stacked when a new commit was inserted into the middle +# of a stack whose existing pull requests (#42, #43, #44) are linked into +# native stack #100 on GitHub in the same order. +# +# Even though the existing pull requests match the stack on GitHub, the new +# pull request cannot be appended (stacks can only grow at the top), so +# without --restack the submit must abort before updating or pushing +# anything. (See mock_insert_mid_stack_restack.py for the --restack case.) + + +def setup_mock_github_server(repo) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond() + + commit_one = scmutil.revsingle(repo, "desc(one)").hex() + github_server.expect_get_pr_details_request(42).and_respond( + "PR_id_42", head_ref_oid=commit_one + ) + # #43 and #44 are stale: their commits were rebased on top of the + # inserted commit. + github_server.expect_get_pr_details_request(43).and_respond("PR_id_43") + github_server.expect_get_pr_details_request(44).and_respond("PR_id_44") + + # The next PR number would be 45 (for the inserted commit). + github_server.expect_guess_next_pull_request_number().and_respond( + latest_issue_num=40, latest_pr_num=44 + ) + + # The stack matches the existing pull requests, but the new pull request + # would be inserted below its top, which requires recreating the stack. + github_server.expect_get_stack_request(42).and_respond( + stack_number=100, pr_numbers=[42, 43, 44] + ) + + return github_server + + +def reposetup(ui, repo): + github_server = setup_mock_github_server(repo) + extensions.wrapfunction( + github_gh_cli, "_make_request", github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) + wrap_with_consumption_check(github_server, submit, "submit") diff --git a/eden/scm/tests/github/mock_insert_mid_stack_restack.py b/eden/scm/tests/github/mock_insert_mid_stack_restack.py new file mode 100644 index 0000000000000..e84b4e3ee5a27 --- /dev/null +++ b/eden/scm/tests/github/mock_insert_mid_stack_restack.py @@ -0,0 +1,96 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions, scmutil +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import ( + mock_run_git_command, + MockGitHubServer, + wrap_with_consumption_check, +) + +# The --restack counterpart of mock_insert_mid_stack.py: the diverged stack +# (#42, #43, #44 with a new commit inserted between #42 and #43) is dissolved +# up front, the base branches are re-chained (including through the new pull +# request #45), everything is pushed, and the stack is recreated from the +# local stack. + + +def setup_mock_github_server(repo) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond() + + commit_one = scmutil.revsingle(repo, "desc(one)").hex() + github_server.expect_get_pr_details_request(42).and_respond( + "PR_id_42", head_ref_oid=commit_one + ) + github_server.expect_get_pr_details_request(43).and_respond("PR_id_43") + github_server.expect_get_pr_details_request(44).and_respond("PR_id_44") + + github_server.expect_guess_next_pull_request_number().and_respond( + latest_issue_num=40, latest_pr_num=44 + ) + + github_server.expect_get_stack_request(42).and_respond( + stack_number=100, pr_numbers=[42, 43, 44] + ) + + # The diverged stack is dissolved before any base updates. + github_server.expect_unstack_request(100).and_respond() + + # Base branches are re-chained through the new pull request: #43's base + # becomes the inserted commit's head branch (pr45). + github_server.expect_update_pr_request( + "PR_id_44", 44, "", base="pr43" + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_43", 43, "", base="pr45" + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_42", 42, "", base="main" + ).and_respond() + + # The inserted commit's pull request is created directly against the head + # branch of the pull request below it. + github_server.expect_create_pr_request( + body="", title="insert", head="pr45", base="pr42" + ).and_respond(number=45) + github_server.expect_get_pr_details_request(45).and_respond("PR_id_45") + + # Body rewrites leave the base branch untouched in the stacked workflow. + github_server.expect_update_pr_request( + "PR_id_44", 44, "three\n", base=None + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_43", 43, "two\n", base=None + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_45", 45, "insert\n", base=None + ).and_respond() + github_server.expect_update_pr_request( + "PR_id_42", 42, "one\n", base=None + ).and_respond() + + github_server.expect_get_username_request().and_respond() + tip = scmutil.revsingle(repo, "desc(three)").hex() + github_server.expect_merge_into_branch(tip).and_respond() + + # The stack is recreated to match the local stack, including the inserted + # pull request. + github_server.expect_create_stack_request([42, 45, 43, 44]).and_respond( + stack_number=101 + ) + + return github_server + + +def reposetup(ui, repo): + github_server = setup_mock_github_server(repo) + extensions.wrapfunction( + github_gh_cli, "_make_request", github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) + wrap_with_consumption_check(github_server, submit, "submit") diff --git a/eden/scm/tests/github/mock_restack_too_small.py b/eden/scm/tests/github/mock_restack_too_small.py new file mode 100644 index 0000000000000..d90113eaf5c1a --- /dev/null +++ b/eden/scm/tests/github/mock_restack_too_small.py @@ -0,0 +1,44 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import ( + mock_run_git_command, + MockGitHubServer, + wrap_with_consumption_check, +) + +# An extension to mock network requests for `sl pr submit` with +# github.pr-workflow=stacked when the local stack contains a single open pull +# request (#42) while the stack on GitHub contains two (#42, #43): the local +# and GitHub stacks have diverged, and --restack must refuse to dissolve the +# stack because it could not be recreated afterwards (a stack requires at +# least two pull requests). + + +def setup_mock_github_server(repo) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond() + + # #42's head is stale (the commit was amended), so there is something to + # push, which makes this a "mutating" submit. + github_server.expect_get_pr_details_request(42).and_respond("PR_id_42") + + github_server.expect_get_stack_request(42).and_respond( + stack_number=100, pr_numbers=[42, 43] + ) + + return github_server + + +def reposetup(ui, repo): + github_server = setup_mock_github_server(repo) + extensions.wrapfunction( + github_gh_cli, "_make_request", github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) + wrap_with_consumption_check(github_server, submit, "submit") diff --git a/eden/scm/tests/github/mock_stack_query_failure.py b/eden/scm/tests/github/mock_stack_query_failure.py new file mode 100644 index 0000000000000..841c17d0b1aa1 --- /dev/null +++ b/eden/scm/tests/github/mock_stack_query_failure.py @@ -0,0 +1,46 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions, scmutil +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import ( + mock_run_git_command, + MockGitHubServer, + wrap_with_consumption_check, +) + +# An extension to mock network requests for `sl pr submit` with +# github.pr-workflow=stacked when the stacks API query fails while there are +# local changes to push. The submit must fail closed: without knowing the +# state of the stack on GitHub, base updates and pushes could corrupt it. + + +def setup_mock_github_server(repo) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond() + + for desc, num in [("one", 42), ("insert", 45), ("two", 43)]: + commit = scmutil.revsingle(repo, f"desc({desc})").hex() + github_server.expect_get_pr_details_request(num).and_respond( + f"PR_id_{num}", head_ref_oid=commit + ) + # #44 is stale: its commit was amended. + github_server.expect_get_pr_details_request(44).and_respond("PR_id_44") + + github_server.expect_get_stack_request(42).and_respond_error( + "mock stacks API failure" + ) + + return github_server + + +def reposetup(ui, repo): + github_server = setup_mock_github_server(repo) + extensions.wrapfunction( + github_gh_cli, "_make_request", github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) + wrap_with_consumption_check(github_server, submit, "submit") diff --git a/eden/scm/tests/github/mock_unstack_remnant.py b/eden/scm/tests/github/mock_unstack_remnant.py new file mode 100644 index 0000000000000..ae6054d05c03b --- /dev/null +++ b/eden/scm/tests/github/mock_unstack_remnant.py @@ -0,0 +1,55 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions, scmutil +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import ( + mock_run_git_command, + MockGitHubServer, + wrap_with_consumption_check, +) + +# An extension to mock network requests for `sl pr submit --restack` with +# github.pr-workflow=stacked when dissolving the diverged stack only +# partially succeeds: pull requests that are merged or queued for merge +# cannot be unstacked and are left in place. Recreating the stack is not +# possible while they remain in the old one, so the submit must abort before +# updating or pushing anything. + + +def setup_mock_github_server(repo) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond() + + for desc, num in [("one", 42), ("insert", 45), ("two", 43)]: + commit = scmutil.revsingle(repo, f"desc({desc})").hex() + github_server.expect_get_pr_details_request(num).and_respond( + f"PR_id_{num}", head_ref_oid=commit + ) + # #44 is stale: its commit was amended. + github_server.expect_get_pr_details_request(44).and_respond("PR_id_44") + + # The stack on GitHub has a different order than the local stack. + github_server.expect_get_stack_request(42).and_respond( + stack_number=100, pr_numbers=[43, 42, 45, 44] + ) + + # Dissolving the stack leaves #43 in place (e.g., it is queued for + # merge). + github_server.expect_unstack_request(100).and_respond( + remaining_pr_numbers=[43] + ) + + return github_server + + +def reposetup(ui, repo): + github_server = setup_mock_github_server(repo) + extensions.wrapfunction( + github_gh_cli, "_make_request", github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) + wrap_with_consumption_check(github_server, submit, "submit") diff --git a/eden/scm/tests/github/mock_unstack_remnant_sync.py b/eden/scm/tests/github/mock_unstack_remnant_sync.py new file mode 100644 index 0000000000000..220879e4dce74 --- /dev/null +++ b/eden/scm/tests/github/mock_unstack_remnant_sync.py @@ -0,0 +1,51 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# +# This software may be used and distributed according to the terms of the +# GNU General Public License version 2. + +from sapling import extensions, scmutil +from sapling.ext.github import github_gh_cli, submit +from sapling.ext.github.mock_utils import ( + mock_run_git_command, + MockGitHubServer, + wrap_with_consumption_check, +) + +# Like mock_unstack_remnant.py, but for the case where all local commits are +# up-to-date (nothing to push): dissolving the diverged stack happens during +# the stack sync at the end of the submit, and a partial dissolution is +# reported as a warning (the stack is not recreated). + + +def setup_mock_github_server(repo) -> MockGitHubServer: + github_server = MockGitHubServer() + + github_server.expect_get_repository_request().and_respond() + + for desc, num in [("one", 42), ("two", 43)]: + commit = scmutil.revsingle(repo, f"desc({desc})").hex() + github_server.expect_get_pr_details_request(num).and_respond( + f"PR_id_{num}", head_ref_oid=commit + ) + + # The stack on GitHub has a different order than the local stack. + github_server.expect_get_stack_request(42).and_respond( + stack_number=100, pr_numbers=[43, 42] + ) + + # Dissolving the stack leaves #43 in place (e.g., it is queued for + # merge), so the stack cannot be recreated. + github_server.expect_unstack_request(100).and_respond( + remaining_pr_numbers=[43] + ) + + return github_server + + +def reposetup(ui, repo): + github_server = setup_mock_github_server(repo) + extensions.wrapfunction( + github_gh_cli, "_make_request", github_server.make_request + ) + extensions.wrapfunction(submit, "run_git_command", mock_run_git_command) + wrap_with_consumption_check(github_server, submit, "submit") diff --git a/eden/scm/tests/test-doctest.py b/eden/scm/tests/test-doctest.py index e0f5d40b354df..d8e4ac5cb92f5 100644 --- a/eden/scm/tests/test-doctest.py +++ b/eden/scm/tests/test-doctest.py @@ -32,6 +32,8 @@ def testmod(name, optionflags=0, testtarget=None): testmod("sapling.pathlog") testmod("sapling.ext.github.archive_commit") +testmod("sapling.ext.github.gh_submit") +testmod("sapling.ext.github.github_gh_cli") testmod("sapling.ext.github.github_repo_util") testmod("sapling.ext.github.pr_parser") testmod("sapling.ext.github.pull_request_arg") diff --git a/eden/scm/tests/test-ext-github-pr-submit-closed.t b/eden/scm/tests/test-ext-github-pr-submit-closed.t index cf4244c465667..aad56fd40716b 100644 --- a/eden/scm/tests/test-ext-github-pr-submit-closed.t +++ b/eden/scm/tests/test-ext-github-pr-submit-closed.t @@ -19,3 +19,25 @@ test we don't try updating a closed pr: pushing 1 to https://github.com/facebook/test_github_repo.git warning, not updating #42 because it isn't open hint[unlink-closed-pr]: to create a new PR, disassociate commit(s) using 'sl pr unlink' then re-run 'sl pr submit' + +test chained bases skip a closed pull request: two open pull requests are +stacked on top of the closed #42. #43 must NOT use the closed #42's head +branch as its base (that would break the chain); it falls through to the +default base branch instead. + + $ echo b > b1 + $ sl ci -Aqm "two + > + > Pull Request resolved: https://github.com/facebook/test_github_repo/pull/43" + $ echo c > c1 + $ sl ci -Aqm "three + > + > Pull Request resolved: https://github.com/facebook/test_github_repo/pull/44" + $ sl pr submit --config extensions.pr_submit=$TESTDIR/github/mock_closed_mid_stack.py + updated base for https://github.com/facebook/test_github_repo/pull/44 + updated base for https://github.com/facebook/test_github_repo/pull/43 + pushing 3 to https://github.com/facebook/test_github_repo.git + updated body for https://github.com/facebook/test_github_repo/pull/44 + updated body for https://github.com/facebook/test_github_repo/pull/43 + warning, not updating #42 because it isn't open + hint[unlink-closed-pr]: to create a new PR, disassociate commit(s) using 'sl pr unlink' then re-run 'sl pr submit' diff --git a/eden/scm/tests/test-ext-github-pr-submit-placeholder-issue.t b/eden/scm/tests/test-ext-github-pr-submit-placeholder-issue.t index 8a60df92c32a3..115fa0f01fbb3 100644 --- a/eden/scm/tests/test-ext-github-pr-submit-placeholder-issue.t +++ b/eden/scm/tests/test-ext-github-pr-submit-placeholder-issue.t @@ -23,3 +23,23 @@ test sending pr pushing 1 to https://github.com/facebook/test_github_repo.git created new pull request: https://github.com/facebook/test_github_repo/pull/1 updated body for https://github.com/facebook/test_github_repo/pull/1 + +test the placeholder strategy with a chained-bases workflow on a fork: fork +head branches cannot be used as base branches on the upstream repository, so +both pull requests are created against the upstream default branch + + $ cd .. + $ sl init --git repo2 + $ cd repo2 + $ setconfig github.placeholder-strategy=True + $ setconfig github.pr-workflow=single + $ echo a > a1 + $ sl ci -Aqm one + $ echo a >> a1 + $ sl ci -Aqm two + $ sl pr submit --config extensions.pr_submit=$TESTDIR/github/mock_create_prs_placeholder_fork.py + pushing 2 to https://github.com/facebook/test_github_repo.git + created new pull request: https://github.com/upstream/test_github_repo/pull/42 + created new pull request: https://github.com/upstream/test_github_repo/pull/43 + updated body for https://github.com/upstream/test_github_repo/pull/43 + updated body for https://github.com/upstream/test_github_repo/pull/42 diff --git a/eden/scm/tests/test-ext-github-pr-submit-stacked-placeholder.t b/eden/scm/tests/test-ext-github-pr-submit-stacked-placeholder.t new file mode 100644 index 0000000000000..1695ac966c80b --- /dev/null +++ b/eden/scm/tests/test-ext-github-pr-submit-stacked-placeholder.t @@ -0,0 +1,28 @@ +#require git no-eden no-windows + + $ eagerepo + $ enable github + $ export SL_TEST_GH_URL=https://github.com/facebook/test_github_repo.git + $ . $TESTDIR/git.sh + $ configure github.pr-workflow=stacked + +build up a github repo + + $ sl init --git repo1 + $ cd repo1 + $ setconfig github.placeholder-strategy=True + $ echo a > a1 + $ sl ci -Aqm one + $ echo a >> a1 + $ sl ci -Aqm two + +submitting a stack of 2 commits with the placeholder strategy creates the PRs +(with chained bases) and links them into a native GitHub stack + + $ sl pr submit --config extensions.pr_submit=$TESTDIR/github/mock_create_stacked_prs_placeholder.py + pushing 2 to https://github.com/facebook/test_github_repo.git + created new pull request: https://github.com/facebook/test_github_repo/pull/42 + created new pull request: https://github.com/facebook/test_github_repo/pull/43 + updated body for https://github.com/facebook/test_github_repo/pull/43 + updated body for https://github.com/facebook/test_github_repo/pull/42 + created stack: https://github.com/facebook/test_github_repo/stacks/100 diff --git a/eden/scm/tests/test-ext-github-pr-submit-stacked.t b/eden/scm/tests/test-ext-github-pr-submit-stacked.t new file mode 100644 index 0000000000000..7429e0c92a5a8 --- /dev/null +++ b/eden/scm/tests/test-ext-github-pr-submit-stacked.t @@ -0,0 +1,201 @@ +#require git no-eden no-windows + + $ eagerepo + $ enable github + $ export SL_TEST_GH_URL=https://github.com/facebook/test_github_repo.git + $ . $TESTDIR/git.sh + $ configure github.pr-workflow=stacked + +build up a github repo + + $ sl init --git repo1 + $ cd repo1 + $ echo a > a1 + $ sl ci -Aqm one + $ echo a >> a1 + $ sl ci -Aqm two + +confirm it is a 'github_repo' + $ sl log -r. -T '{github_repo}\n' + True + +submitting a stack of 2 commits creates the PRs (with chained bases, like the +"single" workflow, and without the stack list footer) and links them into a +native GitHub stack + $ sl pr submit --config extensions.pr_submit=$TESTDIR/github/mock_create_stacked_prs.py + pushing 2 to https://github.com/facebook/test_github_repo.git + created new pull request: https://github.com/facebook/test_github_repo/pull/42 + created new pull request: https://github.com/facebook/test_github_repo/pull/43 + updated body for https://github.com/facebook/test_github_repo/pull/43 + updated body for https://github.com/facebook/test_github_repo/pull/42 + created stack: https://github.com/facebook/test_github_repo/stacks/100 + +adding a commit on top and resubmitting appends the new PR to the existing +stack; the bases of #42 and #43 are NOT updated via the API (GitHub rejects +base changes for PRs in a native stack) + $ echo b > b1 + $ sl ci -Aqm three + $ sl pr submit --config extensions.pr_submit=$TESTDIR/github/mock_extend_stacked_prs.py + #42 is up-to-date + #43 is up-to-date + pushing 1 to https://github.com/facebook/test_github_repo.git + created new pull request: https://github.com/facebook/test_github_repo/pull/44 + updated body for https://github.com/facebook/test_github_repo/pull/44 + updated body for https://github.com/facebook/test_github_repo/pull/43 + updated body for https://github.com/facebook/test_github_repo/pull/42 + added #44 to stack #100 + +if the stack on GitHub has diverged from the local stack, warn without +modifying it + $ sl pr submit --config extensions.pr_submit=$TESTDIR/github/mock_diverged_stack.py + #42 is up-to-date + #43 is up-to-date + #44 is up-to-date + no pull requests to update + warning: stack #100 on GitHub (#43, #42, #44) does not match your local stack (#42, #43, #44); not updating it + hint[pr-submit-restack]: use 'sl pr submit --restack' to dissolve the stack on GitHub and recreate it to match your local stack + +with --restack, the diverged stack is dissolved and recreated to match the +local stack + $ sl pr submit --restack --config extensions.pr_submit=$TESTDIR/github/mock_diverged_stack.py + #42 is up-to-date + #43 is up-to-date + #44 is up-to-date + no pull requests to update + recreated stack: https://github.com/facebook/test_github_repo/stacks/101 + +if the stack on GitHub has diverged AND there are local changes to push, +abort before updating any bases or pushing anything + $ echo b >> b1 + $ sl amend + $ sl pr submit --config extensions.pr_submit=$TESTDIR/github/mock_diverged_stack_dirty.py + #42 is up-to-date + #43 is up-to-date + stack #100 on GitHub (#43, #42, #44) does not match your local stack (#42, #43, #44); not updating it + hint[pr-submit-restack]: use 'sl pr submit --restack' to dissolve the stack on GitHub and recreate it to match your local stack + abort: stack on GitHub has diverged from your local stack + [255] + +with --restack, the diverged stack is dissolved up front so that base +branches can be updated, then recreated from the local stack + $ sl pr submit --restack --config extensions.pr_submit=$TESTDIR/github/mock_diverged_stack_dirty.py + #42 is up-to-date + #43 is up-to-date + updated base for https://github.com/facebook/test_github_repo/pull/44 + updated base for https://github.com/facebook/test_github_repo/pull/43 + updated base for https://github.com/facebook/test_github_repo/pull/42 + pushing 1 to https://github.com/facebook/test_github_repo.git + updated body for https://github.com/facebook/test_github_repo/pull/44 + updated body for https://github.com/facebook/test_github_repo/pull/43 + updated body for https://github.com/facebook/test_github_repo/pull/42 + created stack: https://github.com/facebook/test_github_repo/stacks/101 + +inserting a new commit into the middle of the stack diverges from the stack +on GitHub even though the existing PRs are in the same order (a stack can +only grow at the top): without --restack, abort before updating or pushing +anything + $ sl goto -q 'desc(one)' + $ echo c > c1 + $ sl ci -Aqm insert + $ sl rebase -qs 'desc(two)' -d . + $ sl pr submit --config extensions.pr_submit=$TESTDIR/github/mock_insert_mid_stack.py + #42 is up-to-date + new pull requests would be inserted below the top of stack #100 on GitHub (#42, #43, #44), which requires recreating the stack; not updating it + hint[pr-submit-restack]: use 'sl pr submit --restack' to dissolve the stack on GitHub and recreate it to match your local stack + abort: stack on GitHub has diverged from your local stack + [255] + +with --restack, the stack is dissolved up front, the new PR is created with +its base chained into the stack, and the stack is recreated in the new order + $ sl pr submit --restack --config extensions.pr_submit=$TESTDIR/github/mock_insert_mid_stack_restack.py + #42 is up-to-date + updated base for https://github.com/facebook/test_github_repo/pull/44 + updated base for https://github.com/facebook/test_github_repo/pull/43 + updated base for https://github.com/facebook/test_github_repo/pull/42 + pushing 3 to https://github.com/facebook/test_github_repo.git + created new pull request: https://github.com/facebook/test_github_repo/pull/45 + updated body for https://github.com/facebook/test_github_repo/pull/44 + updated body for https://github.com/facebook/test_github_repo/pull/43 + updated body for https://github.com/facebook/test_github_repo/pull/45 + updated body for https://github.com/facebook/test_github_repo/pull/42 + created stack: https://github.com/facebook/test_github_repo/stacks/101 + +if the stacks API query fails while there are local changes to push, fail +closed: without knowing the state of the stack, pushing could corrupt it + $ sl goto -q 'desc(three)' + $ echo b >> b1 + $ sl amend + $ sl pr submit --config extensions.pr_submit=$TESTDIR/github/mock_stack_query_failure.py + #42 is up-to-date + #45 is up-to-date + #43 is up-to-date + warning, could not query stacks for #42: mock stacks API failure + abort: could not determine the state of the stack on GitHub; re-run 'pr submit' to retry + [255] + +if dissolving a diverged stack only partially succeeds (merged or queued +PRs cannot be unstacked), abort rather than proceed against the remnant + $ sl pr submit --restack --config extensions.pr_submit=$TESTDIR/github/mock_unstack_remnant.py + #42 is up-to-date + #45 is up-to-date + #43 is up-to-date + abort: stack #100 was only partially dissolved: #43 could not be unstacked (merged or queued pull requests are left in place) + [255] + +a single open pull request whose stack on GitHub has more members is a +divergence; --restack refuses to dissolve the stack since it could not be +recreated with fewer than two pull requests + $ cd .. + $ sl init --git repo2 + $ cd repo2 + $ echo a > a1 + $ sl ci -Aqm "one + > + > Pull Request resolved: https://github.com/facebook/test_github_repo/pull/42" + $ echo a >> a1 + $ sl amend + $ sl pr submit --config extensions.pr_submit=$TESTDIR/github/mock_restack_too_small.py + stack #100 on GitHub (#42, #43) does not match your local stack (#42); not updating it + hint[pr-submit-restack]: use 'sl pr submit --restack' to dissolve the stack on GitHub and recreate it to match your local stack + abort: stack on GitHub has diverged from your local stack + [255] + $ sl pr submit --restack --config extensions.pr_submit=$TESTDIR/github/mock_restack_too_small.py + abort: --restack would leave stack #100 with fewer than two pull requests; dissolve it on GitHub instead if that is intended + [255] + +a closed stack is treated the same as no stack: the base branch is updated +via the API and no stack operations are attempted + $ sl pr submit --config extensions.pr_submit=$TESTDIR/github/mock_closed_stack.py + updated base for https://github.com/facebook/test_github_repo/pull/42 + pushing 1 to https://github.com/facebook/test_github_repo.git + updated body for https://github.com/facebook/test_github_repo/pull/42 + +if dissolving the stack during the post-submit sync partially fails, warn +without recreating the stack (the pull requests themselves were already +submitted) + $ echo b > b1 + $ sl ci -Aqm "two + > + > Pull Request resolved: https://github.com/facebook/test_github_repo/pull/43" + $ sl pr submit --restack --config extensions.pr_submit=$TESTDIR/github/mock_unstack_remnant_sync.py + #42 is up-to-date + #43 is up-to-date + no pull requests to update + warning, stack #100 was only partially dissolved: #43 could not be unstacked (merged or queued pull requests are left in place) + +stacks are not supported across forks: pull requests are created against the +upstream default branch and a warning is printed instead of creating a stack + $ cd .. + $ sl init --git repo3 + $ cd repo3 + $ echo a > a1 + $ sl ci -Aqm one + $ echo a >> a1 + $ sl ci -Aqm two + $ sl pr submit --config extensions.pr_submit=$TESTDIR/github/mock_fork_stacked.py + pushing 2 to https://github.com/facebook/test_github_repo.git + created new pull request: https://github.com/upstream/test_github_repo/pull/42 + created new pull request: https://github.com/upstream/test_github_repo/pull/43 + updated body for https://github.com/upstream/test_github_repo/pull/43 + updated body for https://github.com/upstream/test_github_repo/pull/42 + warning: GitHub does not support stacks across forks; pull requests were submitted without a stack