cli icon indicating copy to clipboard operation
cli copied to clipboard

`pr create` prompts for branch when pushed, if cloned with `-- --depth=1 --single-branch`

Open AaronMoat opened this issue 1 year ago • 6 comments

Describe the bug

gh pr create seems to always unconditionally ask where to push the branch, even if it's already been pushed, if the repo has been partially cloned.

gh --version
gh version 2.53.0 (2024-07-17)
https://github.com/cli/cli/releases/tag/v2.53.0

Steps to reproduce the behavior

See the logs for the long details, but

  1. gh repo clone <repo> -- --depth=1 --single-branch
  2. Make a commit on a branch
  3. git push -u origin branch
  4. gh pr create --fill
  5. It prompts you for where to push your branch

Expected vs actual behavior

I would expect not to receive a prompt if the branch has already been pushed.

Logs

pr-create-bug.log

AaronMoat avatar Jul 22 '24 01:07 AaronMoat

Thank you for opening this issue, @AaronMoat! 🙇 Through initial triaging, I think all of this goes back to https://github.com/cli/cli/pull/704 and how we're determining the tracking branch.

Walking through the code

Whether the user is prompted is really based on whether we can accurately and confidently determine the head repo:

https://github.com/cli/cli/blob/b05bddc210a1e45a25b76f9b320239e4c36f03ee/pkg/cmd/pr/create/create.go#L589-L641

https://github.com/cli/cli/blob/b05bddc210a1e45a25b76f9b320239e4c36f03ee/pkg/cmd/pr/create/create.go#L465-L504

https://github.com/cli/cli/blob/b05bddc210a1e45a25b76f9b320239e4c36f03ee/git/client.go#L189-L211

Doing some local debugging, I see a single resolved ref, which causes the logic to determine the tracking branch to exit nil thus falling into asking where to push to. 🤔

andyfeller avatar Jul 29 '24 22:07 andyfeller

@AaronMoat : I think the issue here lies in the additional arguments you're using to clone the repository, which prevents your local repository of pulling remote references needed to detect the branch has been pushed:

gh repo clone <repo> -- --depth=1 --single-branch

These additional options you are passing cause git not to update the local repository with the remote references needed to detect if the branch is pushed up or not.

Local testing

Doing some dirty debugging below, it appears that the remote references that gh is using are not available due to the additional arguments you are providing. Essentially, git cannot determine the local branch has been pushed up by the limit around history and branches.

$ gh repo clone tinyfists/actions-experiments actions-experiments-1 -- --depth=1 --single-branch
$ cd actions-experiments
$ git checkout -b 2nd-test
$ vim whatever.md
$ git add .
$ git commit
$ git push
$ ~/Documents/workspace/cli/cli/bin/gh pr create --fill
Executing git command: /opt/homebrew/bin/git remote -v
Executing git command: /opt/homebrew/bin/git config --get-regexp ^remote\..*\.gh-resolved$
Executing git command: /opt/homebrew/bin/git symbolic-ref --quiet HEAD
Executing git command: /opt/homebrew/bin/git status --porcelain
Determining tracking branch
Reading branch config, head branch 2nd-test
Executing git command: /opt/homebrew/bin/git config --get-regexp ^branch\.2nd-test\.(remote|merge)$
Remote name origin
Resolving refs [HEAD refs/remotes/origin/2nd-test refs/remotes/origin/2nd-test]
Executing git command: /opt/homebrew/bin/git show-ref --verify -- HEAD refs/remotes/origin/2nd-test refs/remotes/origin/2nd-test
ShowRefs output line: 8b21e7104953142b32d256e9139a72bcc6605582 HEAD
Resolved refs [{Hash:8b21e7104953142b32d256e9139a72bcc6605582 Name:HEAD}]
Tracking refs [refs/remotes/origin/2nd-test refs/remotes/origin/2nd-test]

? Where should we push the '2nd-test' branch?  [Use arrows to move, type to filter]
> tinyfists/actions-experiments
  Create a fork of tinyfists/actions-experiments
  Skip pushing the branch
  Cancel

References

andyfeller avatar Jul 30 '24 18:07 andyfeller

@AaronMoat : With everything above, I don't know if this is really a bug in gh as I don't think there's any other way to detect local/remote branch situations. That said, I'd appreciate any insight you can offer to see if there is anything that could be done:

  1. What are the main motivations for these custom git clone arguments used in this case?
  2. Does this limitation due to these arguments make sense?

Thanks!

andyfeller avatar Jul 30 '24 18:07 andyfeller

Who the the fuck are you!! Stop hacking my account

Fuck You !!

On Mon, Jul 29, Reiwa 6 at 5:41 PM Andy Feller @.***> wrote:

Thank you for opening this issue, @AaronMoat https://github.com/AaronMoat! 🙇 Through initial triaging, I think all of this goes back to #704 https://github.com/cli/cli/pull/704 and how we're determining the tracking branch. Walking through the code

Whether the user is prompted is really based on whether we can accurately and confidently determine the head repo:

https://github.com/cli/cli/blob/b05bddc210a1e45a25b76f9b320239e4c36f03ee/pkg/cmd/pr/create/create.go#L589-L641

https://github.com/cli/cli/blob/b05bddc210a1e45a25b76f9b320239e4c36f03ee/pkg/cmd/pr/create/create.go#L465-L504

https://github.com/cli/cli/blob/b05bddc210a1e45a25b76f9b320239e4c36f03ee/git/client.go#L189-L211

Doing some local debugging, I see a single resolved ref, which causes the logic to determine the tracking branch to exit nil thus falling into asking where to push to. 🤔

— Reply to this email directly, view it on GitHub https://github.com/cli/cli/issues/9348#issuecomment-2257128854, or unsubscribe https://github.com/notifications/unsubscribe-auth/BKFE4M3D727JX3TB6UAQTXLZO3AKDAVCNFSM6AAAAABLHLYMQKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJXGEZDQOBVGQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

sarahbyrd3 avatar Jul 30 '24 18:07 sarahbyrd3

@andyfeller thanks for the detailed analysis!

I have been using this for a clone, modify, pr flow for some automated batch changes - so for larger repositories it's quite inefficient to do a full clone. However your explanation makes sense and I think it makes sense not to call this a bug.

I can try with other arguments to wrangle it!

That being said, I wonder whether the following make sense?

  • Should the cli warn you? I'm unsure
  • Should there be options to force an option in non-interactive scenarios?

AaronMoat avatar Jul 30 '24 22:07 AaronMoat

I have been using this for a clone, modify, pr flow for some automated batch changes - so for larger repositories it's quite inefficient to do a full clone.

That makes sense.

That being said, I wonder whether the following make sense?

  • Should the cli warn you? I'm unsure
  • Should there be options to force an option in non-interactive scenarios?

I'm unsure we would incorporate such complex scenario discovery within the code base in all truth, not unless this was significantly impactful for a majority of gh users. 🤔

What logic might gh leverage in this scenario?

Again, the problem is that the repository is cloned in a particular way that makes later interactions more difficult. So would we need to assess the state of the repository when working with certain commands like gh pr create? 🤔

andyfeller avatar Oct 07 '24 14:10 andyfeller