Skip to content

Fix pr create when branch name contains slashes #10859

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.

Sign up for GitHub

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

Merged
williammartin merged 1 commit into from
Apr 24, 2025
Merged

Fix pr create when branch name contains slashes #10859

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions acceptance/testdata/pr/pr-create-guesses-remote-from-sha-with-branch-name-slash.txtar
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
skip 'it creates a fork owned by the user running the test'

env REPO=${SCRIPT_NAME}-${RANDOM_STRING}
env FORK=${REPO}-fork

# Use gh as a credential helper
exec gh auth setup-git

# Get the current username for the fork owner
exec gh api user --jq .login
stdout2env USER

# Create a repository with a file so it has a default branch
exec gh repo create ${ORG}/${REPO} --add-readme --private

# Defer repo cleanup
defer gh repo delete --yes ${ORG}/${REPO}

# Create a user fork of repository. This will be owned by USER.
exec gh repo fork ${ORG}/${REPO} --fork-name ${FORK}
sleep 5

# Defer repo cleanup of fork
defer gh repo delete --yes ${USER}/${FORK}

# Retrieve fork repository information
exec gh repo view ${USER}/${FORK} --json id --jq '.id'
stdout2env FORK_ID

exec gh repo clone ${USER}/${FORK}
cd ${FORK}

# Prepare a branch to commit
exec git checkout -b feature/branch
exec git commit --allow-empty -m 'Upstream Commit'
# Push without setting an upstream (-u or config)
exec git push upstream feature/branch

# Prepare an additional commit
exec git commit --allow-empty -m 'Fork Commit'
# Push without setting an upstream (-u or config)
exec git push origin feature/branch

# Create the PR
exec gh pr create --title 'Feature Title' --body 'Feature Body'
stdout https://${GH_HOST}/${ORG}/${REPO}/pull/1

# Check the PR is indeed created
exec gh pr view ${USER}:feature/branch --json headRefName,headRepository,baseRefName,isCrossRepository
stdout {"baseRefName":"main","headRefName":"feature/branch","headRepository":{"id":"${FORK_ID}","name":"${FORK}"},"isCrossRepository":true}
2 changes: 2 additions & 0 deletions acceptance/testdata/pr/pr-create-guesses-remote-from-sha.txtar
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
skip 'it creates a fork owned by the user running the test'

env REPO=${SCRIPT_NAME}-${RANDOM_STRING}
env FORK=${REPO}-fork

Expand Down
46 changes: 46 additions & 0 deletions acceptance/testdata/pr/pr-create-remote-ref-with-branch-name-slash.txtar
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
skip 'it creates a fork owned by the user running the test'

# Setup environment variables used for testscript
env REPO=${SCRIPT_NAME}-${RANDOM_STRING}
env FORK=${REPO}-fork

# Use gh as a credential helper
exec gh auth setup-git

# Get the current username for the fork owner
exec gh api user --jq .login
stdout2env USER

# Create a repository to act as upstream with a file so it has a default branch
exec gh repo create ${ORG}/${REPO} --add-readme --private

# Defer repo cleanup of upstream
defer gh repo delete --yes ${ORG}/${REPO}

# Create a user fork of repository. This will be owned by USER.
exec gh repo fork ${ORG}/${REPO} --fork-name ${FORK}
sleep 5

# Defer repo cleanup of fork
defer gh repo delete --yes ${USER}/${FORK}

# Retrieve fork repository information
exec gh repo view ${USER}/${FORK} --json id --jq '.id'
stdout2env FORK_ID

# Clone the repo
exec gh repo clone ${USER}/${FORK}
cd ${FORK}

# Prepare a branch where changes are pulled from the upstream default branch but pushed to fork
exec git checkout -b feature/branch
exec git commit --allow-empty -m 'Empty Commit'
exec git push -u origin feature/branch

# Create the PR spanning upstream and fork repositories
exec gh pr create --title 'Feature Title' --body 'Feature Body'
stdout https://${GH_HOST}/${ORG}/${REPO}/pull/1

# Assert that the PR was created with the correct head repository and refs
exec gh pr view --json headRefName,headRepository,baseRefName,isCrossRepository
stdout {"baseRefName":"main","headRefName":"feature/branch","headRepository":{"id":"${FORK_ID}","name":"${FORK}"},"isCrossRepository":true}
49 changes: 45 additions & 4 deletions git/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,15 +518,56 @@ func (r RemoteTrackingRef) String() string {

// ParseRemoteTrackingRef parses a string of the form "refs/remotes/<remote>/<branch>" into
// a RemoteTrackingBranch struct. If the string does not match this format, an error is returned.
//
// For now, we assume that refnames are of the format "<remote>/<branch>", where
// the remote is a single path component, and branch may have many path components e.g.
// "origin/my/branch" is valid as: {Remote: "origin", Branch: "my/branch"}
// but "my/origin/branch" would parse incorrectly as: {Remote: "my", Branch: "origin/branch"}
// I don't believe there is a way to fix this without providing the list of remotes to this function.
//
// It becomes particularly confusing if you have something like:
//
// ```
// [remote "foo"]
// url = https://github.com/williammartin/test-repo.git
// fetch = +refs/heads/*:refs/remotes/foo/*
// [remote "foo/bar"]
// url = https://github.com/williammartin/test-repo.git
// fetch = +refs/heads/*:refs/remotes/foo/bar/*
// [branch "bar/baz"]
// remote = foo
// merge = refs/heads/bar/baz
// [branch "baz"]
// remote = foo/bar
// merge = refs/heads/baz
// ```
//
// These @{push} refs would resolve identically:
//
// ```
// ➜ git rev-parse --symbolic-full-name baz@{push}
// refs/remotes/foo/bar/baz

// ➜ git rev-parse --symbolic-full-name bar/baz@{push}
// refs/remotes/foo/bar/baz
// ```
//
// When using this ref, git assumes it means `remote: foo` `branch: bar/baz`.
func ParseRemoteTrackingRef(s string) (RemoteTrackingRef, error) {
parts := strings.Split(s, "/")
if len(parts) != 4 || parts[0] != "refs" || parts[1] != "remotes" {
prefix := "refs/remotes/"
if !strings.HasPrefix(s, prefix) {
return RemoteTrackingRef{}, fmt.Errorf("remote tracking branch must have format refs/remotes/<remote>/<branch> but was: %s", s)
}

refName := strings.TrimPrefix(s, prefix)
refNameParts := strings.SplitN(refName, "/", 2)
if len(refNameParts) != 2 {
return RemoteTrackingRef{}, fmt.Errorf("remote tracking branch must have format refs/remotes/<remote>/<branch> but was: %s", s)
}

return RemoteTrackingRef{
Remote: parts[2],
Branch: parts[3],
Remote: refNameParts[0],
Branch: refNameParts[1],
}, nil
}

Expand Down
27 changes: 26 additions & 1 deletion git/client_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1151,13 +1151,38 @@ func TestRemoteTrackingRef(t *testing.T) {
wantError error
}{
{
name: "valid remote tracking ref",
name: "valid remote tracking ref without slash in branch name",
remoteTrackingRef: "refs/remotes/origin/branchName",
wantRemoteTrackingRef: RemoteTrackingRef{
Remote: "origin",
Branch: "branchName",
},
},
{
name: "valid remote tracking ref with slash in branch name",
remoteTrackingRef: "refs/remotes/origin/branch/name",
wantRemoteTrackingRef: RemoteTrackingRef{
Remote: "origin",
Branch: "branch/name",
},
},
// TODO: Uncomment when we support slashes in remote names
// {
// name: "valid remote tracking ref with slash in remote name",
// remoteTrackingRef: "refs/remotes/my/origin/branchName",
// wantRemoteTrackingRef: RemoteTrackingRef{
// Remote: "my/origin",
// Branch: "branchName",
// },
// },
// {
// name: "valid remote tracking ref with slash in remote name and branch name",
// remoteTrackingRef: "refs/remotes/my/origin/branch/name",
// wantRemoteTrackingRef: RemoteTrackingRef{
// Remote: "my/origin",
// Branch: "branch/name",
// },
// },
{
name: "incorrect parts",
remoteTrackingRef: "refs/remotes/origin",
Expand Down
Loading