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

getLocalRef can return the wrong ref when there are multiple branches with a similar name #90

Closed
raidenhasegawa opened this issue Jun 9, 2021 · 4 comments · Fixed by #91
Labels
bug Something isn't working

Comments

@raidenhasegawa
Copy link

Behavior and Cause

When you have an action triggered by a push to a certain branch, say my_branch, and another similarly named branch exists, say another/my_branch, getLocalRef will return another/my_branch instead of my_branch as it shows up first in the list of matching refs pulled by getLocalRef:

const output = (await exec('git', ['show-ref', shortName], {ignoreReturnCode: true})).stdout 
const refs = output.split(/\r?\n/g) 
                   .map(l =>; l.match(/refs\/.*$/)?.[0] ?? '')
                   .filter(l =>; l !== '')

Prefixing with this format{some_prefix}/{branch_name} causes some trouble with show-ref as they show up as refs for {branch_name}. Annoyingly, some bots will autogenerate branches with this pattern, e.g., changeset actions has a bot that does such a thing (see example below).

Example

This was caused by the changeset bot:
image

Comment

I'm not sure what the right solution is here, I'd be happy to work together to figure it out but I'm not an expert in actions or git in general. It does appear to be an undesirable behavior that can get by you rather silently. I think you can solve this in some cases by explicitly passing the ref in the inputs but does it make sense to fail if say there remoteRef.length > 1 in getLocalRef?

@dorny dorny added the bug Something isn't working label Jun 14, 2021
@dorny
Copy link
Owner

dorny commented Jun 14, 2021

Hello,

Thanks for reporting the issue.
Using only fully qualified refs would be the best as it would be completely unambiguous.
However, users are used to working with short names so I implement this behavior in this action as well.
I will look into the issue and post here update as soon as I will figure out something.

dorny added a commit that referenced this issue Jun 14, 2021
git show-ref will return all branches where end segment matches the input. This cause issues when there are both 'someBranch' and 'somePrefix/someBranch' branches. This fix ensures the correct ref is returned by explicitly matching segments after common parts (e.g. refs/heads).
@dorny dorny closed this as completed in #91 Jun 14, 2021
dorny added a commit that referenced this issue Jun 14, 2021
@dorny
Copy link
Owner

dorny commented Jun 14, 2021

The fix turns out to be pretty simple. I change the code so it explicitly matches what follows after refs/heads or refs/remotes/origin against the input.

Please verify if it works for you now and reopen the issue if there is still some issue.

@raidenhasegawa
Copy link
Author

This is great, @dorny. Just ran the action with v2.10.2 and this corrected the behavior I was seeing with v2.10.1. Thanks!

@raidenhasegawa
Copy link
Author

So this worked well for me using a short name, but it does break when I use a fully qualified ref. I think that the filter logic here breaks down when shortName is fully qualified, e.g., if shortName == 'refs/remotes/origin/{branch_name}, then match[1] == '{branch_name}' when match !== null.

I think that this can be fixed by modifying getShortName to also strip 'refs/remotes/origin' from the ref.

lykahb pushed a commit to lykahb/paths-filter that referenced this issue Mar 11, 2022
git show-ref will return all branches where end segment matches the input. This cause issues when there are both 'someBranch' and 'somePrefix/someBranch' branches. This fix ensures the correct ref is returned by explicitly matching segments after common parts (e.g. refs/heads).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants