diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 2ec9102b6..8d53b708c 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -5,6 +5,8 @@ - fixed: `git machete discover` no longer produces a different branch tree depending on which worktree it is run from; the fresh-branch recency ranking now aggregates HEAD reflogs across all worktrees rather than only the current one (reported by @jasonoura, contributed by @earfman) - fixed: when run from a branch being slid out, `git machete slide-out` no longer checks out that branch's new parent if a child branch is going to be checked out right afterwards anyway for the rebase/merge +- fixed: pull/merge request-reading commands (`anno-prs`, `checkout-prs`, `retarget-pr`, `restack-pr`, `update-pr-descriptions` and their GitLab counterparts) + now address the base/target repository - the one that actually hosts the PR/MR - when the `machete.{github,gitlab}.base*` git config keys are set, rather than the head/source repository ## New in git-machete 3.44.1 diff --git a/docs/man/git-machete.1 b/docs/man/git-machete.1 index 116071d4e..990f688ed 100644 --- a/docs/man/git-machete.1 +++ b/docs/man/git-machete.1 @@ -332,7 +332,8 @@ For example, \fBgit config machete.github.domain git.example.org\fP The name of the git remote (as in \fBgit remote\fP) that git\-machete pushes the head branch to. Unless both \fBmachete.github.organization\fP and \fBmachete.github.repository\fP are set, this remote\(aqs URL is also inspected to derive the GitHub organization and repository that the pull request resides in. -The pull request is operated on through the GitHub API, which addresses that organization/repository rather than a git remote. +Unless the \fBmachete.github.base*\fP keys below point elsewhere, the pull request is operated on through the GitHub API, +which addresses that organization/repository rather than a git remote. By default (when this key is unset), if exactly one remote\(aqs URL corresponds to GitHub, that remote is selected automatically; set this key to disambiguate when more than one remote points to GitHub. For example, \fBgit config machete.github.remote origin\fP @@ -348,6 +349,10 @@ For example, \fBgit config machete.github.repository git\-machete\fP .B \fBmachete.github.baseRemote\fP Like \fBmachete.github.remote\fP, but used to locate the base repository that the pull request targets, which may differ from the head repository (for example, the base in an upstream repository and the head in a fork). +Setting this key is what makes the PR\-reading/\-modifying commands (\fBanno\-prs\fP, \fBcheckout\-prs\fP, \fBretarget\-pr\fP, +\fBrestack\-pr\fP, \fBupdate\-pr\-descriptions\fP) address that base repository rather than the head one. +\fBcreate\-pr\fP does not need it: it infers the base repository from the base branch\(aqs tracking remote, +so it already targets the correct base (even one in a separate fork/upstream repository) even when this key is unset. Defaults to \fBmachete.github.remote\fP when unset. For example, \fBgit config machete.github.baseRemote upstream\fP .TP @@ -411,7 +416,8 @@ For example, \fBgit config machete.gitlab.domain git.example.org\fP The name of the git remote (as in \fBgit remote\fP) that git\-machete pushes the source branch to. Unless both \fBmachete.gitlab.namespace\fP and \fBmachete.gitlab.project\fP are set, this remote\(aqs URL is also inspected to derive the GitLab namespace and project that the merge request resides in. -The merge request is operated on through the GitLab API, which addresses that namespace/project rather than a git remote. +Unless the \fBmachete.gitlab.base*\fP keys below point elsewhere, the merge request is operated on through the GitLab API, +which addresses that namespace/project rather than a git remote. By default (when this key is unset), if exactly one remote\(aqs URL corresponds to GitLab, that remote is selected automatically; set this key to disambiguate when more than one remote points to GitLab. For example, \fBgit config machete.gitlab.remote origin\fP @@ -427,6 +433,10 @@ For example, \fBgit config machete.gitlab.project hello\-world\fP .B \fBmachete.gitlab.baseRemote\fP Like \fBmachete.gitlab.remote\fP, but used to locate the target project that the merge request targets, which may differ from the source project (for example, the target in an upstream project and the source in a fork). +Setting this key is what makes the MR\-reading/\-modifying commands (\fBanno\-mrs\fP, \fBcheckout\-mrs\fP, \fBretarget\-mr\fP, +\fBrestack\-mr\fP, \fBupdate\-mr\-descriptions\fP) address that target project rather than the source one. +\fBcreate\-mr\fP does not need it: it infers the target project from the target branch\(aqs tracking remote, +so it already targets the correct project (even one in a separate fork/upstream project) even when this key is unset. Defaults to \fBmachete.gitlab.remote\fP when unset. For example, \fBgit config machete.gitlab.baseRemote upstream\fP .TP @@ -1212,7 +1222,8 @@ For example, \fBgit config machete.github.domain git.example.org\fP The name of the git remote (as in \fBgit remote\fP) that git\-machete pushes the head branch to. Unless both \fBmachete.github.organization\fP and \fBmachete.github.repository\fP are set, this remote\(aqs URL is also inspected to derive the GitHub organization and repository that the pull request resides in. -The pull request is operated on through the GitHub API, which addresses that organization/repository rather than a git remote. +Unless the \fBmachete.github.base*\fP keys below point elsewhere, the pull request is operated on through the GitHub API, +which addresses that organization/repository rather than a git remote. By default (when this key is unset), if exactly one remote\(aqs URL corresponds to GitHub, that remote is selected automatically; set this key to disambiguate when more than one remote points to GitHub. For example, \fBgit config machete.github.remote origin\fP @@ -1228,6 +1239,10 @@ For example, \fBgit config machete.github.repository git\-machete\fP .B \fBmachete.github.baseRemote\fP Like \fBmachete.github.remote\fP, but used to locate the base repository that the pull request targets, which may differ from the head repository (for example, the base in an upstream repository and the head in a fork). +Setting this key is what makes the PR\-reading/\-modifying commands (\fBanno\-prs\fP, \fBcheckout\-prs\fP, \fBretarget\-pr\fP, +\fBrestack\-pr\fP, \fBupdate\-pr\-descriptions\fP) address that base repository rather than the head one. +\fBcreate\-pr\fP does not need it: it infers the base repository from the base branch\(aqs tracking remote, +so it already targets the correct base (even one in a separate fork/upstream repository) even when this key is unset. Defaults to \fBmachete.github.remote\fP when unset. For example, \fBgit config machete.github.baseRemote upstream\fP .TP @@ -1507,7 +1522,8 @@ For example, \fBgit config machete.gitlab.domain git.example.org\fP The name of the git remote (as in \fBgit remote\fP) that git\-machete pushes the source branch to. Unless both \fBmachete.gitlab.namespace\fP and \fBmachete.gitlab.project\fP are set, this remote\(aqs URL is also inspected to derive the GitLab namespace and project that the merge request resides in. -The merge request is operated on through the GitLab API, which addresses that namespace/project rather than a git remote. +Unless the \fBmachete.gitlab.base*\fP keys below point elsewhere, the merge request is operated on through the GitLab API, +which addresses that namespace/project rather than a git remote. By default (when this key is unset), if exactly one remote\(aqs URL corresponds to GitLab, that remote is selected automatically; set this key to disambiguate when more than one remote points to GitLab. For example, \fBgit config machete.gitlab.remote origin\fP @@ -1523,6 +1539,10 @@ For example, \fBgit config machete.gitlab.project hello\-world\fP .B \fBmachete.gitlab.baseRemote\fP Like \fBmachete.gitlab.remote\fP, but used to locate the target project that the merge request targets, which may differ from the source project (for example, the target in an upstream project and the source in a fork). +Setting this key is what makes the MR\-reading/\-modifying commands (\fBanno\-mrs\fP, \fBcheckout\-mrs\fP, \fBretarget\-mr\fP, +\fBrestack\-mr\fP, \fBupdate\-mr\-descriptions\fP) address that target project rather than the source one. +\fBcreate\-mr\fP does not need it: it infers the target project from the target branch\(aqs tracking remote, +so it already targets the correct project (even one in a separate fork/upstream project) even when this key is unset. Defaults to \fBmachete.gitlab.remote\fP when unset. For example, \fBgit config machete.gitlab.baseRemote upstream\fP .TP diff --git a/docs/source/git-config-keys/github.access.rst b/docs/source/git-config-keys/github.access.rst index 33bc17c8c..7116a96b3 100644 --- a/docs/source/git-config-keys/github.access.rst +++ b/docs/source/git-config-keys/github.access.rst @@ -6,7 +6,8 @@ The name of the git remote (as in ``git remote``) that git-machete pushes the head branch to. Unless both ``machete.github.organization`` and ``machete.github.repository`` are set, this remote's URL is also inspected to derive the GitHub organization and repository that the pull request resides in. - The pull request is operated on through the GitHub API, which addresses that organization/repository rather than a git remote. + Unless the ``machete.github.base*`` keys below point elsewhere, the pull request is operated on through the GitHub API, + which addresses that organization/repository rather than a git remote. By default (when this key is unset), if exactly one remote's URL corresponds to GitHub, that remote is selected automatically; set this key to disambiguate when more than one remote points to GitHub. For example, ``git config machete.github.remote origin`` @@ -22,6 +23,10 @@ ``machete.github.baseRemote`` Like ``machete.github.remote``, but used to locate the base repository that the pull request targets, which may differ from the head repository (for example, the base in an upstream repository and the head in a fork). + Setting this key is what makes the PR-reading/-modifying commands (``anno-prs``, ``checkout-prs``, ``retarget-pr``, + ``restack-pr``, ``update-pr-descriptions``) address that base repository rather than the head one. + ``create-pr`` does not need it: it infers the base repository from the base branch's tracking remote, + so it already targets the correct base (even one in a separate fork/upstream repository) even when this key is unset. Defaults to ``machete.github.remote`` when unset. For example, ``git config machete.github.baseRemote upstream`` diff --git a/docs/source/git-config-keys/gitlab.access.rst b/docs/source/git-config-keys/gitlab.access.rst index 7bf752b62..c6d4c10e9 100644 --- a/docs/source/git-config-keys/gitlab.access.rst +++ b/docs/source/git-config-keys/gitlab.access.rst @@ -6,7 +6,8 @@ The name of the git remote (as in ``git remote``) that git-machete pushes the source branch to. Unless both ``machete.gitlab.namespace`` and ``machete.gitlab.project`` are set, this remote's URL is also inspected to derive the GitLab namespace and project that the merge request resides in. - The merge request is operated on through the GitLab API, which addresses that namespace/project rather than a git remote. + Unless the ``machete.gitlab.base*`` keys below point elsewhere, the merge request is operated on through the GitLab API, + which addresses that namespace/project rather than a git remote. By default (when this key is unset), if exactly one remote's URL corresponds to GitLab, that remote is selected automatically; set this key to disambiguate when more than one remote points to GitLab. For example, ``git config machete.gitlab.remote origin`` @@ -22,6 +23,10 @@ ``machete.gitlab.baseRemote`` Like ``machete.gitlab.remote``, but used to locate the target project that the merge request targets, which may differ from the source project (for example, the target in an upstream project and the source in a fork). + Setting this key is what makes the MR-reading/-modifying commands (``anno-mrs``, ``checkout-mrs``, ``retarget-mr``, + ``restack-mr``, ``update-mr-descriptions``) address that target project rather than the source one. + ``create-mr`` does not need it: it infers the target project from the target branch's tracking remote, + so it already targets the correct project (even one in a separate fork/upstream project) even when this key is unset. Defaults to ``machete.gitlab.remote`` when unset. For example, ``git config machete.gitlab.baseRemote upstream`` diff --git a/git_machete/client/with_code_hosting.py b/git_machete/client/with_code_hosting.py index 1364d4ad9..d8cd3c265 100644 --- a/git_machete/client/with_code_hosting.py +++ b/git_machete/client/with_code_hosting.py @@ -433,7 +433,8 @@ def create_pull_request( def restack_pull_request(self, *, opt_update_related_descriptions: bool) -> None: spec = self.code_hosting_spec head = self._git.get_current_branch() - _, org_repo_remote = self._init_code_hosting_client(branch_used_for_tracking_data=head) + _, org_repo_remote = self._init_code_hosting_client( + branch_used_for_tracking_data=head, base_branch_used_for_tracking_data=self.parent_of(head)) pr: Optional[PullRequest] = self.__get_sole_pull_request_for_head(head, ignore_if_missing=False) assert pr is not None @@ -538,7 +539,8 @@ def retarget_pull_request(self, *, opt_branch: Optional[LocalBranchShortName], head: ManagedBranchName = self.expect_in_managed_branches(opt_branch or self._git.get_current_branch()) spec = self.code_hosting_spec if self.__code_hosting_client is None: - self._init_code_hosting_client(branch_used_for_tracking_data=head) + self._init_code_hosting_client( + branch_used_for_tracking_data=head, base_branch_used_for_tracking_data=self.parent_of(head)) pr: Optional[PullRequest] = self.__get_sole_pull_request_for_head( head, ignore_if_missing=opt_ignore_if_missing) @@ -605,7 +607,8 @@ def __derive_org_repo_and_remote( keys = spec.git_config_keys if is_base: remote_key, org_key, repo_key = keys.base_remote, keys.base_organization, keys.base_repository - # The base remote falls back to the (non-base) remote; the base organization/repository have no such fallback. + # The base remote falls back to the (non-base) remote; the base organization/repository have no such fallback + # (`create_pull_request` relies on that to detect a fork base). remote_from_config = self._config.code_hosting_base_remote(keys) or self._config.code_hosting_remote(keys) org_from_config = self._config.code_hosting_base_organization(keys) repo_from_config = self._config.code_hosting_base_repository(keys) @@ -691,16 +694,45 @@ def __derive_org_repo_and_remote( f'{spec.git_config_keys.for_locating_repo_message()}\n') def _init_code_hosting_client(self, - branch_used_for_tracking_data: Optional[LocalBranchShortName] = None + branch_used_for_tracking_data: Optional[LocalBranchShortName] = None, + base_branch_used_for_tracking_data: Optional[LocalBranchShortName] = None ) -> Tuple[str, OrganizationAndRepositoryAndRemote]: if self.__code_hosting_client is not None: raise UnexpectedMacheteException("Code hosting client has already been initialized.") domain = self.__derive_code_hosting_domain() - org_repo_remote = self.__derive_org_repo_and_remote( + # PR-reading/-modifying commands must talk to the repository that *hosts* the PRs, i.e. the base repository. + # In a fork workflow the base (upstream) repository differs from the head (fork) repository that holds the branches, + # so the code hosting client is created against the base repository, while the returned head remote is still what + # callers use to fetch/push branches. + # The base repository is located in two ways: explicit machete..base* config keys take precedence (honored for + # every PR-reading/-modifying command); otherwise, when a base branch is given (retarget/restack), the base repository + # is inferred from that branch's tracking remote, exactly like create_pull_request does. Inference is best-effort: if it + # cannot be resolved unambiguously (e.g. the base branch has no tracking data among several candidate remotes), we fall + # back to the head repository, preserving the pre-inference behavior. When neither applies, the base repository resolves + # to the head one, so this is a no-op for the common (non-fork) case. + head_org_repo_remote = self.__derive_org_repo_and_remote( domain=domain, branch_used_for_tracking_data=branch_used_for_tracking_data) + keys = self.code_hosting_spec.git_config_keys + base_config_present = ( + self._config.code_hosting_base_remote(keys) is not None or + self._config.code_hosting_base_organization(keys) is not None or + self._config.code_hosting_base_repository(keys) is not None) + base_org_repo_remote = head_org_repo_remote + if base_config_present: + base_org_repo_remote = self.__derive_org_repo_and_remote( + domain=domain, + branch_used_for_tracking_data=base_branch_used_for_tracking_data or branch_used_for_tracking_data, + is_base=True) + elif base_branch_used_for_tracking_data is not None: + try: + base_org_repo_remote = self.__derive_org_repo_and_remote( + domain=domain, branch_used_for_tracking_data=base_branch_used_for_tracking_data, is_base=True) + except MacheteException: + # Ambiguous inference with no explicit base* config to honor -> fall back to the head repository. + pass self.code_hosting_client = self.code_hosting_spec.create_client( - domain=domain, organization=org_repo_remote.organization, repository=org_repo_remote.repository) - return domain, org_repo_remote + domain=domain, organization=base_org_repo_remote.organization, repository=base_org_repo_remote.repository) + return domain, head_org_repo_remote START_GIT_MACHETE_GENERATED_COMMENT = '' END_GIT_MACHETE_GENERATED_COMMENT = '' diff --git a/git_machete/generated_docs.py b/git_machete/generated_docs.py index 434181d99..334675081 100644 --- a/git_machete/generated_docs.py +++ b/git_machete/generated_docs.py @@ -253,7 +253,8 @@ The name of the git remote (as in `git remote`) that git-machete pushes the head branch to. Unless both `machete.github.organization` and `machete.github.repository` are set, this remote's URL is also inspected to derive the GitHub organization and repository that the pull request resides in. - The pull request is operated on through the GitHub API, which addresses that organization/repository rather than a git remote. + Unless the `machete.github.base*` keys below point elsewhere, the pull request is operated on through the GitHub API, + which addresses that organization/repository rather than a git remote. By default (when this key is unset), if exactly one remote's URL corresponds to GitHub, that remote is selected automatically; set this key to disambiguate when more than one remote points to GitHub. For example, `git config machete.github.remote origin` @@ -269,6 +270,10 @@ `machete.github.baseRemote` Like `machete.github.remote`, but used to locate the base repository that the pull request targets, which may differ from the head repository (for example, the base in an upstream repository and the head in a fork). + Setting this key is what makes the PR-reading/-modifying commands (`anno-prs`, `checkout-prs`, `retarget-pr`, + `restack-pr`, `update-pr-descriptions`) address that base repository rather than the head one. + `create-pr` does not need it: it infers the base repository from the base branch's tracking remote, + so it already targets the correct base (even one in a separate fork/upstream repository) even when this key is unset. Defaults to `machete.github.remote` when unset. For example, `git config machete.github.baseRemote upstream` @@ -319,7 +324,8 @@ The name of the git remote (as in `git remote`) that git-machete pushes the source branch to. Unless both `machete.gitlab.namespace` and `machete.gitlab.project` are set, this remote's URL is also inspected to derive the GitLab namespace and project that the merge request resides in. - The merge request is operated on through the GitLab API, which addresses that namespace/project rather than a git remote. + Unless the `machete.gitlab.base*` keys below point elsewhere, the merge request is operated on through the GitLab API, + which addresses that namespace/project rather than a git remote. By default (when this key is unset), if exactly one remote's URL corresponds to GitLab, that remote is selected automatically; set this key to disambiguate when more than one remote points to GitLab. For example, `git config machete.gitlab.remote origin` @@ -335,6 +341,10 @@ `machete.gitlab.baseRemote` Like `machete.gitlab.remote`, but used to locate the target project that the merge request targets, which may differ from the source project (for example, the target in an upstream project and the source in a fork). + Setting this key is what makes the MR-reading/-modifying commands (`anno-mrs`, `checkout-mrs`, `retarget-mr`, + `restack-mr`, `update-mr-descriptions`) address that target project rather than the source one. + `create-mr` does not need it: it infers the target project from the target branch's tracking remote, + so it already targets the correct project (even one in a separate fork/upstream project) even when this key is unset. Defaults to `machete.gitlab.remote` when unset. For example, `git config machete.gitlab.baseRemote upstream` @@ -840,7 +850,8 @@ The name of the git remote (as in `git remote`) that git-machete pushes the head branch to. Unless both `machete.github.organization` and `machete.github.repository` are set, this remote's URL is also inspected to derive the GitHub organization and repository that the pull request resides in. - The pull request is operated on through the GitHub API, which addresses that organization/repository rather than a git remote. + Unless the `machete.github.base*` keys below point elsewhere, the pull request is operated on through the GitHub API, + which addresses that organization/repository rather than a git remote. By default (when this key is unset), if exactly one remote's URL corresponds to GitHub, that remote is selected automatically; set this key to disambiguate when more than one remote points to GitHub. For example, `git config machete.github.remote origin` @@ -856,6 +867,10 @@ `machete.github.baseRemote` Like `machete.github.remote`, but used to locate the base repository that the pull request targets, which may differ from the head repository (for example, the base in an upstream repository and the head in a fork). + Setting this key is what makes the PR-reading/-modifying commands (`anno-prs`, `checkout-prs`, `retarget-pr`, + `restack-pr`, `update-pr-descriptions`) address that base repository rather than the head one. + `create-pr` does not need it: it infers the base repository from the base branch's tracking remote, + so it already targets the correct base (even one in a separate fork/upstream repository) even when this key is unset. Defaults to `machete.github.remote` when unset. For example, `git config machete.github.baseRemote upstream` @@ -1060,7 +1075,8 @@ The name of the git remote (as in `git remote`) that git-machete pushes the source branch to. Unless both `machete.gitlab.namespace` and `machete.gitlab.project` are set, this remote's URL is also inspected to derive the GitLab namespace and project that the merge request resides in. - The merge request is operated on through the GitLab API, which addresses that namespace/project rather than a git remote. + Unless the `machete.gitlab.base*` keys below point elsewhere, the merge request is operated on through the GitLab API, + which addresses that namespace/project rather than a git remote. By default (when this key is unset), if exactly one remote's URL corresponds to GitLab, that remote is selected automatically; set this key to disambiguate when more than one remote points to GitLab. For example, `git config machete.gitlab.remote origin` @@ -1076,6 +1092,10 @@ `machete.gitlab.baseRemote` Like `machete.gitlab.remote`, but used to locate the target project that the merge request targets, which may differ from the source project (for example, the target in an upstream project and the source in a fork). + Setting this key is what makes the MR-reading/-modifying commands (`anno-mrs`, `checkout-mrs`, `retarget-mr`, + `restack-mr`, `update-mr-descriptions`) address that target project rather than the source one. + `create-mr` does not need it: it infers the target project from the target branch's tracking remote, + so it already targets the correct project (even one in a separate fork/upstream project) even when this key is unset. Defaults to `machete.gitlab.remote` when unset. For example, `git config machete.gitlab.baseRemote upstream` diff --git a/tests/mockers_github.py b/tests/mockers_github.py index a161f948a..7497a63b3 100644 --- a/tests/mockers_github.py +++ b/tests/mockers_github.py @@ -14,6 +14,7 @@ def mock_pr_json(head: str, base: str, number: int, repo_id: int = 1, + base_repo_id: Optional[int] = None, user: str = 'some_other_user', html_url: str = 'www.github.com', body: Optional[str] = '# Summary', @@ -23,7 +24,9 @@ def mock_pr_json(head: str, base: str, number: int, return { 'head': {'ref': head, 'repo': {'id': repo_id}}, 'user': {'login': user}, - 'base': {'ref': base}, + # `base.repo.id` records which repository hosts the PR; `None` (the default) means "served for any repository", + # so the vast majority of tests that don't care about fork/base targeting keep working unchanged. + 'base': {'ref': base, 'repo': {'id': base_repo_id}}, 'number': str(number), 'html_url': html_url, 'title': 'PR title', @@ -113,6 +116,12 @@ def url_with_query_params(**new_params: Any) -> str: new_query_string: str = urlencode({**query_params, **new_params}) return parsed_url._replace(query=new_query_string).geturl() + def find_repo_id_by_org_and_repo(org: str, repo: str) -> Optional[int]: + for repo_id, repo_data in github_api_state.repositories.items(): + if repo_data['owner']['login'] == org and repo_data['name'] == repo: + return repo_id + return None + def handle_get() -> "MockAPIResponse": if url_path_matches('/repositories/[0-9]+'): repo_no = int(url_segments[-1]) @@ -120,14 +129,22 @@ def handle_get() -> "MockAPIResponse": return MockAPIResponse(HTTPStatus.OK, github_api_state.repositories[repo_no]) raise error_404() elif url_path_matches('/repos/*/*/pulls'): + # The `/repos/{org}/{repo}/pulls` endpoint lists PRs hosted by the base repository `{org}/{repo}`, + # so filter out PRs whose base repository (if declared) differs from the one being queried. + requested_base_repo_id: Optional[int] = find_repo_id_by_org_and_repo(url_segments[-3], url_segments[-2]) + + def hosted_by_requested_repo(pull: Dict[str, Any]) -> bool: + base_repo_id = pull['base'].get('repo', {}).get('id') + return base_repo_id is None or base_repo_id == requested_base_repo_id + full_head_name: Optional[str] = query_params.get('head') if full_head_name: head: str = full_head_name.split(':')[1] - prs = github_api_state.get_open_pulls_by_head(head) + prs = [pull for pull in github_api_state.get_open_pulls_by_head(head) if hosted_by_requested_repo(pull)] # If no matching PRs are found, the real GitHub returns 200 OK with an empty JSON array - not 404. return MockAPIResponse(HTTPStatus.OK, prs) else: - pulls = github_api_state.get_open_pulls() + pulls = [pull for pull in github_api_state.get_open_pulls() if hosted_by_requested_repo(pull)] page_str = query_params.get('page') page = int(page_str) if page_str else 1 per_page = int(query_params['per_page']) diff --git a/tests/mockers_gitlab.py b/tests/mockers_gitlab.py index 07c0a83c9..255128451 100644 --- a/tests/mockers_gitlab.py +++ b/tests/mockers_gitlab.py @@ -21,6 +21,7 @@ def mock_mr_json(head: str, base: str, number: int, repo_id: int = 1, + base_repo_id: Optional[int] = None, user: str = 'some_other_user', html_url: str = 'www.gitlab.com', body: Optional[str] = '# Summary', @@ -30,6 +31,9 @@ def mock_mr_json(head: str, base: str, number: int, return { 'source_branch': head, 'source_project_id': repo_id, + # `project_id` records the target project that hosts the MR; `None` (the default) means "served for any project", + # so the vast majority of tests that don't care about fork/target targeting keep working unchanged. + 'project_id': base_repo_id, 'target_branch': base, 'author': {'username': user}, 'iid': str(number), @@ -127,6 +131,15 @@ def url_with_query_params(**new_params: Any) -> str: new_query_string: str = urlencode({**query_params, **new_params}) return parsed_url._replace(query=new_query_string).geturl() + def find_project_id(id_or_path: str) -> Optional[int]: + if id_or_path.isdigit(): + return int(id_or_path) + full_path = urllib.parse.unquote(id_or_path) + for project_id, project in gitlab_api_state.projects.items(): + if project['namespace']['full_path'] == full_path: + return project_id + return None + def handle_get() -> "MockAPIResponse": if url_path_matches('/projects/[0-9]+'): repo_no = int(url_segments[-1]) @@ -140,13 +153,21 @@ def handle_get() -> "MockAPIResponse": return MockAPIResponse(HTTPStatus.OK, project) raise error_404() elif url_path_matches('/projects/*/merge_requests'): + # The `/projects/{id}/merge_requests` endpoint lists MRs hosted by the target project `{id}`, + # so filter out MRs whose target project (if declared) differs from the one being queried. + requested_target_project_id: Optional[int] = find_project_id(url_segments[-2]) + + def hosted_by_requested_project(mr: Dict[str, Any]) -> bool: + target_project_id = mr.get('project_id') + return target_project_id is None or target_project_id == requested_target_project_id + head: Optional[str] = query_params.get('source_branch') if head: - mrs = gitlab_api_state.get_open_mrs_by_head(head) + mrs = [mr for mr in gitlab_api_state.get_open_mrs_by_head(head) if hosted_by_requested_project(mr)] # If no matching MRs are found, the real GitLab returns 200 OK with an empty JSON array - not 404. return MockAPIResponse(HTTPStatus.OK, mrs) else: - mrs = gitlab_api_state.get_open_mrs() + mrs = [mr for mr in gitlab_api_state.get_open_mrs() if hosted_by_requested_project(mr)] page_str = query_params.get('page') page = int(page_str) if page_str else 1 per_page_str = query_params.get('per_page') diff --git a/tests/test_github_anno_prs.py b/tests/test_github_anno_prs.py index 87883ba6a..7efeaf1ef 100644 --- a/tests/test_github_anno_prs.py +++ b/tests/test_github_anno_prs.py @@ -4,6 +4,7 @@ from tests.cli_runner import assert_failure, assert_success, launch_command, rewrite_branch_layout_file from tests.git_repository import (add_remote, amend_commit, check_out, commit, create_repo, create_repo_with_remote, delete_branch, new_branch, push, remove_remote, reset_to, set_git_config_key, wait_to_bump_commit_timestamp) +from tests.mockers_code_hosting import mock_from_url from tests.mockers_github import MockGitHubAPIState, mock_github_token_for_domain_fake, mock_pr_json, mock_urlopen @@ -193,6 +194,67 @@ def test_github_anno_prs_no_remotes(self) -> None: remove_remote() assert_failure(["github", "anno-prs"], "No remotes defined for this repository (see git remote)") + def test_github_anno_prs_targets_base_repo(self, mocker: MockerFixture) -> None: + # In a fork workflow the PRs are hosted by the base (upstream) repository, not the head (fork) repository. + # Reading commands must therefore honor `machete.github.baseRemote` and query the base repository for PRs. + self.patch_symbol(mocker, 'git_machete.code_hosting.OrganizationAndRepository.from_url', mock_from_url) + self.patch_symbol(mocker, 'git_machete.github.GitHubToken.for_domain', mock_github_token_for_domain_fake) + repositories = { + 1: {'owner': {'login': 'example-org'}, 'name': 'example-repo', + 'clone_url': 'https://github.com/example-org/example-repo.git'}, + 2: {'owner': {'login': 'example-org'}, 'name': 'example-repo-1', + 'clone_url': 'https://github.com/example-org/example-repo-1.git'}, + } + github_api_state = MockGitHubAPIState( + repositories, + mock_pr_json(number=1, head='feature', base='develop', repo_id=1, base_repo_id=2, user='github_user'), + mock_pr_json(number=2, head='develop', base='master', repo_id=1, base_repo_id=2, user='github_user')) + self.patch_symbol(mocker, 'urllib.request.urlopen', mock_urlopen(github_api_state)) + + # `origin` (-> example-org/example-repo) is the head/fork remote holding the branches, + # while `upstream` (-> example-org/example-repo-1) is the base remote that hosts the PRs. + create_repo_with_remote() + upstream_path = create_repo("remote-1", bare=True, switch_dir_to_new_repo=False) + add_remote("upstream", upstream_path) + + new_branch("master") + commit() + push() + new_branch("develop") + commit() + push() + new_branch("feature") + commit() + push() + rewrite_branch_layout_file("master\n\tdevelop\n\t\tfeature") + + # Without any base config the client targets the head repository (origin), which does not host these PRs. + launch_command("github", "anno-prs") + assert_success( + ["status"], + """ + master + | + o-develop + | + o-feature * + """ + ) + + # `machete.github.baseRemote` points reading commands at the base repository that actually hosts the PRs. + set_git_config_key("machete.github.baseRemote", "upstream") + launch_command("github", "anno-prs") + assert_success( + ["status"], + """ + master + | + o-develop PR #2 + | + o-feature * PR #1 + """ + ) + def test_github_anno_prs_multiple_non_origin_github_remotes(self) -> None: create_repo() add_remote("origin-1", "https://github.com/tester/repo_sandbox-1.git") diff --git a/tests/test_github_checkout_prs.py b/tests/test_github_checkout_prs.py index d36b03e58..695ce2776 100644 --- a/tests/test_github_checkout_prs.py +++ b/tests/test_github_checkout_prs.py @@ -602,3 +602,62 @@ def test_github_checkout_prs_main_to_main_pr(self, mocker: MockerFixture) -> Non 'Checking for open GitHub PRs... OK\n' 'PR #2 checked out at local branch fix-10341\n' ) + + def test_github_checkout_prs_targets_base_repo(self, mocker: MockerFixture) -> None: + # In a fork workflow the PRs are hosted by the base (upstream) repository, not the head (fork) repository. + # checkout-prs must therefore honor `machete.github.baseRemote` and query the base repository for PRs. + self.patch_symbol(mocker, 'git_machete.code_hosting.OrganizationAndRepository.from_url', mock_from_url) + self.patch_symbol(mocker, 'git_machete.github.GitHubToken.for_domain', mock_github_token_for_domain_fake) + repositories = { + 1: {'owner': {'login': 'example-org'}, 'name': 'example-repo', + 'clone_url': 'https://github.com/example-org/example-repo.git'}, + 2: {'owner': {'login': 'example-org'}, 'name': 'example-repo-1', + 'clone_url': 'https://github.com/example-org/example-repo-1.git'}, + } + github_api_state = MockGitHubAPIState( + repositories, + mock_pr_json(number=1, head='feature', base='develop', repo_id=1, base_repo_id=2, user='github_user'), + mock_pr_json(number=2, head='develop', base='master', repo_id=1, base_repo_id=2, user='github_user')) + self.patch_symbol(mocker, 'urllib.request.urlopen', mock_urlopen(github_api_state)) + + # `origin` (-> example-org/example-repo) is the head/fork remote holding the branches, + # while `upstream` (-> example-org/example-repo-1) is the base remote that hosts the PRs. + create_repo_with_remote() + upstream_path = create_repo("remote-1", bare=True, switch_dir_to_new_repo=False) + add_remote("upstream", upstream_path) + + new_branch("master") + commit() + push() + new_branch("develop") + commit() + push() + new_branch("feature") + commit() + push() + + # Without any base config the client targets the head repository (origin), which does not host these PRs. + assert_success( + ['github', 'checkout-prs', '--all'], + 'Checking for open GitHub PRs... OK\n' + 'Warn: currently there are no pull requests opened in repository example-org/example-repo\n' + ) + + # `machete.github.baseRemote` points checkout-prs at the base repository that actually hosts the PRs. + set_git_config_key("machete.github.baseRemote", "upstream") + assert_success( + ['github', 'checkout-prs', '--all'], + 'Checking for open GitHub PRs... OK\n' + 'PR #2 checked out at local branch develop\n' + 'PR #1 checked out at local branch feature\n' + ) + assert_success( + ["status"], + """ + master + | + o-develop PR #2 + | + o-feature * PR #1 + """ + ) diff --git a/tests/test_github_restack_pr.py b/tests/test_github_restack_pr.py index aea59c2ab..970a42151 100644 --- a/tests/test_github_restack_pr.py +++ b/tests/test_github_restack_pr.py @@ -4,7 +4,8 @@ from tests.base_test import BaseTest from tests.cli_runner import assert_failure, assert_success, rewrite_branch_layout_file -from tests.git_repository import amend_commit, commit, create_repo_with_remote, new_branch, push, reset_to, set_git_config_key +from tests.git_repository import (add_remote, amend_commit, commit, create_repo, create_repo_with_remote, new_branch, push, reset_to, + set_git_config_key) from tests.mockers import fixed_author_and_committer_date_in_past from tests.mockers_code_hosting import mock_from_url from tests.mockers_github import MockGitHubAPIState, mock_github_token_for_domain_fake, mock_pr_json, mock_urlopen @@ -331,3 +332,46 @@ def test_github_restack_pr_branch_diverged_and_older(self, mocker: MockerFixture assert pr is not None assert pr['draft'] is False assert pr['base']['ref'] == 'master' + + def test_github_restack_pr_infers_base_repo_from_parent_tracking(self, mocker: MockerFixture) -> None: + # In a fork workflow the PR is hosted by the base (upstream) repository, not the head (fork) repository. + # Even with no base* config keys, restack-pr infers the base repository from the parent branch's tracking remote + # (just like create-pr does), so it queries the repository that actually hosts the PR. + self.patch_symbol(mocker, 'git_machete.github.GitHubToken.for_domain', mock_github_token_for_domain_fake) + self.patch_symbol(mocker, 'git_machete.code_hosting.OrganizationAndRepository.from_url', mock_from_url) + repositories = { + 1: {'owner': {'login': 'example-org'}, 'name': 'example-repo', + 'clone_url': 'https://github.com/example-org/example-repo.git'}, + 2: {'owner': {'login': 'example-org'}, 'name': 'example-repo-1', + 'clone_url': 'https://github.com/example-org/example-repo-1.git'}, + } + github_api_state = MockGitHubAPIState( + repositories, + mock_pr_json(number=1, head='feature', base='master', repo_id=1, base_repo_id=2, user='github_user')) + self.patch_symbol(mocker, 'urllib.request.urlopen', mock_urlopen(github_api_state)) + + # `origin` (-> example-org/example-repo) is the head/fork remote holding the branches, while + # `upstream` (-> example-org/example-repo-1) is the base remote that hosts the PR. The parent branch (develop) + # tracks `upstream`, so the base repository is inferred from it - no machete.github.base* key is set. + create_repo_with_remote() + upstream_path = create_repo("remote-1", bare=True, switch_dir_to_new_repo=False) + add_remote("upstream", upstream_path) + + new_branch("master") + commit() + push() + new_branch("develop") + commit() + push(remote="upstream") + new_branch("feature") + commit() + push() + rewrite_branch_layout_file("master\n\tdevelop\n\t\tfeature") + + assert_success( + ['github', 'restack-pr'], + "Switching base branch of PR #1 to develop... OK\n" + ) + pr = github_api_state.get_pull_by_number(1) + assert pr is not None + assert pr['base']['ref'] == 'develop' diff --git a/tests/test_github_retarget_pr.py b/tests/test_github_retarget_pr.py index bb61d75ee..70b74c4a7 100644 --- a/tests/test_github_retarget_pr.py +++ b/tests/test_github_retarget_pr.py @@ -487,3 +487,115 @@ def test_github_retarget_pr_root_branch(self, mocker: MockerFixture) -> None: "Branch master does not have a parent branch (it is a root) even though there is an open PR #15 to root.\n" "Consider modifying the branch layout file (git machete edit) so that master is a child of root." ) + + def test_github_retarget_pr_infers_base_repo_from_parent_tracking(self, mocker: MockerFixture) -> None: + # In a fork workflow the PR is hosted by the base (upstream) repository, not the head (fork) repository. + # Even with no base* config keys, retarget-pr infers the base repository from the parent branch's tracking remote + # (just like create-pr does), so it queries the repository that actually hosts the PR. + self.patch_symbol(mocker, 'git_machete.code_hosting.OrganizationAndRepository.from_url', mock_from_url) + self.patch_symbol(mocker, 'git_machete.github.GitHubToken.for_domain', mock_github_token_for_domain_fake) + repositories = { + 1: {'owner': {'login': 'example-org'}, 'name': 'example-repo', + 'clone_url': 'https://github.com/example-org/example-repo.git'}, + 2: {'owner': {'login': 'example-org'}, 'name': 'example-repo-1', + 'clone_url': 'https://github.com/example-org/example-repo-1.git'}, + } + github_api_state = MockGitHubAPIState( + repositories, + mock_pr_json(number=1, head='feature', base='master', repo_id=1, base_repo_id=2, user='github_user')) + self.patch_symbol(mocker, 'urllib.request.urlopen', mock_urlopen(github_api_state)) + + # `origin` (-> example-org/example-repo) is the head/fork remote holding the branches, while + # `upstream` (-> example-org/example-repo-1) is the base remote that hosts the PR. The parent branch (develop) + # tracks `upstream`, so the base repository is inferred from it - no machete.github.base* key is set. + create_repo_with_remote() + upstream_path = create_repo("remote-1", bare=True, switch_dir_to_new_repo=False) + add_remote("upstream", upstream_path) + + new_branch("master") + commit() + push() + new_branch("develop") + commit() + push(remote="upstream") + new_branch("feature") + commit() + push() + rewrite_branch_layout_file("master\n\tdevelop\n\t\tfeature") + + assert_success( + ['github', 'retarget-pr'], + "Switching base branch of PR #1 to develop... OK\n" + ) + pr1 = github_api_state.get_pull_by_number(1) + assert pr1 is not None + assert pr1['base']['ref'] == 'develop' + + def test_github_retarget_pr_surfaces_base_config_error(self, mocker: MockerFixture) -> None: + # When a base* config key is set but cannot be resolved (here it points at a nonexistent remote), retarget-pr must + # surface the misconfiguration rather than silently falling back to the head repository. + self.patch_symbol(mocker, 'git_machete.code_hosting.OrganizationAndRepository.from_url', mock_from_url) + + create_repo_with_remote() + new_branch("master") + commit() + push() + new_branch("feature") + commit() + push() + rewrite_branch_layout_file("master\n\tfeature") + + set_git_config_key("machete.github.baseRemote", "nonexistent") + assert_failure( + ['github', 'retarget-pr'], + "machete.github.baseRemote git config key points to nonexistent remote, but such remote does not exist") + + def test_github_retarget_pr_targets_base_repo(self, mocker: MockerFixture) -> None: + # In a fork workflow the PR is hosted by the base (upstream) repository, not the head (fork) repository. + # retarget-pr must therefore honor `machete.github.baseRemote` and query the base repository for the PR. + self.patch_symbol(mocker, 'git_machete.code_hosting.OrganizationAndRepository.from_url', mock_from_url) + self.patch_symbol(mocker, 'git_machete.github.GitHubToken.for_domain', mock_github_token_for_domain_fake) + repositories = { + 1: {'owner': {'login': 'example-org'}, 'name': 'example-repo', + 'clone_url': 'https://github.com/example-org/example-repo.git'}, + 2: {'owner': {'login': 'example-org'}, 'name': 'example-repo-1', + 'clone_url': 'https://github.com/example-org/example-repo-1.git'}, + } + github_api_state = MockGitHubAPIState( + repositories, + mock_pr_json(number=1, head='feature', base='master', repo_id=1, base_repo_id=2, user='github_user')) + self.patch_symbol(mocker, 'urllib.request.urlopen', mock_urlopen(github_api_state)) + + # `origin` (-> example-org/example-repo) is the head/fork remote holding the branches, + # while `upstream` (-> example-org/example-repo-1) is the base remote that hosts the PR. + create_repo_with_remote() + upstream_path = create_repo("remote-1", bare=True, switch_dir_to_new_repo=False) + add_remote("upstream", upstream_path) + + new_branch("master") + commit() + push() + new_branch("develop") + commit() + push() + new_branch("feature") + commit() + push() + rewrite_branch_layout_file("master\n\tdevelop\n\t\tfeature") + + # Without any base config the client targets the head repository (origin), which does not host the PR. + assert_failure( + ['github', 'retarget-pr'], + "No PRs in example-org/example-repo have feature as its head branch" + ) + + # `machete.github.baseRemote` points retarget-pr at the base repository that actually hosts the PR; + # the PR's stale base (master) is then retargeted to feature's actual parent (develop). + set_git_config_key("machete.github.baseRemote", "upstream") + assert_success( + ['github', 'retarget-pr'], + "Switching base branch of PR #1 to develop... OK\n" + ) + pr1 = github_api_state.get_pull_by_number(1) + assert pr1 is not None + assert pr1['base']['ref'] == 'develop' diff --git a/tests/test_gitlab_anno_mrs.py b/tests/test_gitlab_anno_mrs.py index a128ca872..84a461aaa 100644 --- a/tests/test_gitlab_anno_mrs.py +++ b/tests/test_gitlab_anno_mrs.py @@ -4,6 +4,7 @@ from tests.cli_runner import assert_failure, assert_success, launch_command, rewrite_branch_layout_file from tests.git_repository import (add_remote, amend_commit, check_out, commit, create_repo, create_repo_with_remote, delete_branch, new_branch, push, remove_remote, reset_to, set_git_config_key, wait_to_bump_commit_timestamp) +from tests.mockers_code_hosting import mock_from_url from tests.mockers_gitlab import MockGitLabAPIState, mock_gitlab_token_for_domain_fake, mock_mr_json, mock_urlopen @@ -193,6 +194,62 @@ def test_gitlab_anno_mrs_no_remotes(self) -> None: remove_remote() assert_failure(["gitlab", "anno-mrs"], "No remotes defined for this repository (see git remote)") + def test_gitlab_anno_mrs_targets_base_project(self, mocker: MockerFixture) -> None: + # In a fork workflow the MRs are hosted by the target (upstream) project, not the source (fork) project. + # Reading commands must therefore honor `machete.gitlab.baseRemote` and query the target project for MRs. + self.patch_symbol(mocker, 'git_machete.code_hosting.OrganizationAndRepository.from_url', mock_from_url) + self.patch_symbol(mocker, 'git_machete.gitlab.GitLabToken.for_domain', mock_gitlab_token_for_domain_fake) + # Project id 2 (-> example-org/example-repo) is the source/fork project holding the branches, + # while project id 3 (-> example-org/example-repo-1) is the target project that hosts the MRs. + gitlab_api_state = MockGitLabAPIState.with_mrs( + mock_mr_json(number=1, head='feature', base='develop', repo_id=2, base_repo_id=3, user='gitlab_user'), + mock_mr_json(number=2, head='develop', base='master', repo_id=2, base_repo_id=3, user='gitlab_user')) + self.patch_symbol(mocker, 'urllib.request.urlopen', mock_urlopen(gitlab_api_state)) + + # `origin` (-> example-org/example-repo) is the source/fork remote holding the branches, + # while `upstream` (-> example-org/example-repo-1) is the base remote that hosts the MRs. + create_repo_with_remote() + upstream_path = create_repo("remote-1", bare=True, switch_dir_to_new_repo=False) + add_remote("upstream", upstream_path) + + new_branch("master") + commit() + push() + new_branch("develop") + commit() + push() + new_branch("feature") + commit() + push() + rewrite_branch_layout_file("master\n\tdevelop\n\t\tfeature") + + # Without any base config the client targets the source project (origin), which does not host these MRs. + launch_command("gitlab", "anno-mrs") + assert_success( + ["status"], + """ + master + | + o-develop + | + o-feature * + """ + ) + + # `machete.gitlab.baseRemote` points reading commands at the target project that actually hosts the MRs. + set_git_config_key("machete.gitlab.baseRemote", "upstream") + launch_command("gitlab", "anno-mrs") + assert_success( + ["status"], + """ + master + | + o-develop MR !2 + | + o-feature * MR !1 + """ + ) + def test_gitlab_anno_mrs_multiple_non_origin_gitlab_remotes(self) -> None: create_repo() add_remote("origin-1", "https://gitlab.com/tester/repo_sandbox-1.git") diff --git a/tests/test_gitlab_retarget_mr.py b/tests/test_gitlab_retarget_mr.py index 17d56e451..cb9030c24 100644 --- a/tests/test_gitlab_retarget_mr.py +++ b/tests/test_gitlab_retarget_mr.py @@ -505,3 +505,46 @@ def test_gitlab_retarget_mr_root_branch(self, mocker: MockerFixture) -> None: "Branch master does not have a parent branch (it is a root) even though there is an open MR !15 to root.\n" "Consider modifying the branch layout file (git machete edit) so that master is a child of root." ) + + def test_gitlab_retarget_mr_infers_target_project_from_parent_tracking(self, mocker: MockerFixture) -> None: + # In a fork workflow the MR is hosted by the target (upstream) project, not the source (fork) project. + # Even with no base* config keys, retarget-mr infers the target project from the parent branch's tracking remote + # (just like create-mr does), so it queries the project that actually hosts the MR. + self.patch_symbol(mocker, 'git_machete.code_hosting.OrganizationAndRepository.from_url', mock_from_url) + self.patch_symbol(mocker, 'git_machete.gitlab.GitLabToken.for_domain', mock_gitlab_token_for_domain_fake) + projects = { + 2: {'id': 2, 'namespace': {'full_path': 'example-org/example-repo'}, 'name': 'example-repo', + 'http_url_to_repo': 'https://gitlab.com/example-org/example-repo.git'}, + 3: {'id': 3, 'namespace': {'full_path': 'example-org/example-repo-1'}, 'name': 'example-repo-1', + 'http_url_to_repo': 'https://gitlab.com/example-org/example-repo-1.git'}, + } + gitlab_api_state = MockGitLabAPIState( + projects, + mock_mr_json(number=1, head='feature', base='master', repo_id=2, base_repo_id=3, user='gitlab_user')) + self.patch_symbol(mocker, 'urllib.request.urlopen', mock_urlopen(gitlab_api_state)) + + # `origin` (-> example-org/example-repo) is the source/fork remote holding the branches, while + # `upstream` (-> example-org/example-repo-1) is the base remote that hosts the MR. The parent branch (develop) + # tracks `upstream`, so the target project is inferred from it - no machete.gitlab.base* key is set. + create_repo_with_remote() + upstream_path = create_repo("remote-1", bare=True, switch_dir_to_new_repo=False) + add_remote("upstream", upstream_path) + + new_branch("master") + commit() + push() + new_branch("develop") + commit() + push(remote="upstream") + new_branch("feature") + commit() + push() + rewrite_branch_layout_file("master\n\tdevelop\n\t\tfeature") + + assert_success( + ['gitlab', 'retarget-mr'], + "Switching target branch of MR !1 to develop... OK\n" + ) + mr = gitlab_api_state.get_mr_by_number(1) + assert mr is not None + assert mr['target_branch'] == 'develop'