Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: support finding pr branch via git config branch keys #806

Merged

Conversation

ldelossa
Copy link
Contributor

@ldelossa ldelossa commented Jan 17, 2025

Fixes #799

When the gh cli tool checks out a branch created from a fork of the origin it places remote tracking info directly in git config at a key derived from branch.{branch_name} prefix.

Octo did not consider this when checking if the repository is checked out to the branch pending merge into the origin.

Update the in_pr_branch function to also check if the current PR has a git configuration which provides details of the upstream fork branch.

See the following man git-config entries for more details:

       branch.<name>.remote
           When on branch <name>, it tells git fetch and git push which remote to fetch from/push to. The remote to push to may
           be overridden with remote.pushDefault (for all branches). The remote to push to, for the current branch, may be
           further overridden by branch.<name>.pushRemote. If no remote is configured, or if you are not on any branch and there
           is more than one remote defined in the repository, it defaults to origin for fetching and remote.pushDefault for
           pushing. Additionally, . (a period) is the current local repository (a dot-repository), see branch.<name>.merge's
           final note below.

       branch.<name>.pushRemote
           When on branch <name>, it overrides branch.<name>.remote for pushing. It also overrides remote.pushDefault for
           pushing from branch <name>. When you pull from one place (e.g. your upstream) and push to another place (e.g. your
           own publishing repository), you would want to set remote.pushDefault to specify the remote to push to for all
           branches, and use this option to override it for a specific branch.

       branch.<name>.merge
           Defines, together with branch.<name>.remote, the upstream branch for the given branch. It tells git fetch/git
           pull/git rebase which branch to merge and can also affect git push (see push.default). When in branch <name>, it
           tells git fetch the default refspec to be marked for merging in FETCH_HEAD. The value is handled like the remote part
           of a refspec, and must match a ref which is fetched from the remote given by "branch.<name>.remote". The merge
           information is used by git pull (which at first calls git fetch) to lookup the default branch for merging. Without
           this option, git pull defaults to merge the first refspec fetched. Specify multiple values to get an octopus merge.
           If you wish to setup git pull so that it merges into <name> from another branch in the local repository, you can
           point branch.<name>.merge to the desired branch, and use the relative path setting . (a period) for
           branch.<name>.remote.

Describe what this PR does / why we need it

Does this pull request fix one issue?

Describe how you did it

Describe how to verify it

Special notes for reviews

Checklist

  • Passing tests and linting standards
  • Documentation updates in README.md and doc/octo.txt

@github-actions github-actions bot added the bug Something wrong in the code label Jan 17, 2025
@ldelossa ldelossa force-pushed the ldelossa/fix-local-fs-gh-pr-checkout branch 4 times, most recently from 9ce0c36 to bc0a71a Compare January 17, 2025 16:24
@wd60622
Copy link
Collaborator

wd60622 commented Jan 21, 2025

Will take a look tomorrow

Copy link
Collaborator

@wd60622 wd60622 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my point of view, looks good. One suggestion for readibility

Fixes pwntester#799

When the gh cli tool checks out a branch created from a fork of the
origin it places remote tracking info directly in git config at a key
derived from branch.{branch_name} prefix.

Octo did not consider this when checking if the repository is checked
out to the branch pending merge into the origin.

Update the `in_pr_branch` function to also check if the current PR has
a git configuration which provides details of the upstream fork branch.

See the following `man git-config` entries for more details:
```
       branch.<name>.remote
           When on branch <name>, it tells git fetch and git push which remote to fetch from/push to. The remote to push to may
           be overridden with remote.pushDefault (for all branches). The remote to push to, for the current branch, may be
           further overridden by branch.<name>.pushRemote. If no remote is configured, or if you are not on any branch and there
           is more than one remote defined in the repository, it defaults to origin for fetching and remote.pushDefault for
           pushing. Additionally, . (a period) is the current local repository (a dot-repository), see branch.<name>.merge's
           final note below.

       branch.<name>.pushRemote
           When on branch <name>, it overrides branch.<name>.remote for pushing. It also overrides remote.pushDefault for
           pushing from branch <name>. When you pull from one place (e.g. your upstream) and push to another place (e.g. your
           own publishing repository), you would want to set remote.pushDefault to specify the remote to push to for all
           branches, and use this option to override it for a specific branch.

       branch.<name>.merge
           Defines, together with branch.<name>.remote, the upstream branch for the given branch. It tells git fetch/git
           pull/git rebase which branch to merge and can also affect git push (see push.default). When in branch <name>, it
           tells git fetch the default refspec to be marked for merging in FETCH_HEAD. The value is handled like the remote part
           of a refspec, and must match a ref which is fetched from the remote given by "branch.<name>.remote". The merge
           information is used by git pull (which at first calls git fetch) to lookup the default branch for merging. Without
           this option, git pull defaults to merge the first refspec fetched. Specify multiple values to get an octopus merge.
           If you wish to setup git pull so that it merges into <name> from another branch in the local repository, you can
           point branch.<name>.merge to the desired branch, and use the relative path setting . (a period) for
           branch.<name>.remote.

```

Signed-off-by: ldelossa <[email protected]>
@ldelossa ldelossa force-pushed the ldelossa/fix-local-fs-gh-pr-checkout branch from ce14d32 to 8d388a7 Compare January 22, 2025 14:59
@ldelossa
Copy link
Contributor Author

@wd60622

I handled your feedback. Also rebased to latest HEAD, so the merge commit is gone here.

@clobrano
Copy link

It seems to fix the problem for me too
Thanks!

Copy link
Collaborator

@wd60622 wd60622 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for adjustment and pr!

@wd60622 wd60622 merged commit 9f90ec9 into pwntester:master Jan 22, 2025
3 checks passed
ldelossa added a commit to ldelossa/octo.nvim that referenced this pull request Jan 27, 2025
A follow up to: pwntester#806

I overlooked that the `gh cli` command may also store checked out local
branch's upstream branch in a format 'refs/pull/{pr_number}/head'.

Update the `get_upstream_branch_from_config` function to consider this
and check if the current branch's upstream configuration matches the pr
datamodel being evaluated.

Signed-off-by: ldelossa <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something wrong in the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Could not start a review of GitHub PR
3 participants