GITHUB_REF is sometimes problematic for `pull_request` triggers
Closed this issue · 7 comments
Per comment report on #16:
I seem to be having an issue when using this action in a PR where GITHUB_REF is being populated incorrectly with a
pull_requesttrigger https://stackoverflow.com/questions/58033366/how-to-get-the-current-branch-within-github-actions.My suggestion would be to let the user pass this in as an optional parameter, as that way they could use something like
BRANCH_NAME: ${{ github.head_ref || github.ref_name }}to avoid such issues.I tried a call to
- run: echo "GITHUB_REF=${{ github.head_ref }}" >> $GITHUB_ENVbefore running this action, but this variable seems sticky and I was unable to work around it.I ended up just reverting to
PUSH_ALL_REFS: "true"
To briefly summarize my current understanding of the problem:
When the push_all_refs input is set to "false", the git push command that is run by the action will specify "${GITHUB_REF}" as the push target.
However, it seems that there may be an undesired behavior (or an opportunity for a better behavior or user configuration) here on pull requests because on pull requests the ref is refs/pull/<pr_number>/merge but the user may not want to mirror this ref in the case of pull requests or may want to mirror the source branch instead on such triggers.
@cameronrutherford thank you for raising this concern. Can you help me understand this better by explaining the precise problem that occurs when the mirror action is run on a pull request trigger?
What is the unexpected or undesirable behavior? What do you believe the expected or desirable behavior should be?
@cameronrutherford thank you for raising this concern. Can you help me understand this better by explaining the precise problem that occurs when the mirror action is run on a pull request trigger?
What is the unexpected or undesirable behavior? What do you believe the expected or desirable behavior should be?
I think the issue description sufficiently describes the unexpected behaviour. refs/pull/<pr_number>/merge is what GITHUB_REF is populated within pull requests, whereas I would expect that to have the source branch name.
Since this is consistent with GitHub actions and their environment variables, I think having a way to specify the remote ref that is used in the action would be sufficient, as this would also add extra functionality to the action.
Something to keep in mind though is that the PR ref is different from the source branch ref. The PR ref contains the merged contents of the source and target branch. Consider also that because pull requests can originate from other repositories, the source branch may not even be part of the same repository! In which case, using the source branch ref may (at best) fail or (at worst) perform an erroneous action.
If you want to mirror the source branch only, I think the resolution should be that the action should be configured to run on push instead of pull_request. Would that resolve the issue @cameronrutherford ?
I'm actually not even sure if there's a context in which mirroring PR refs makes any sense or if any servers would even accept that ref if you try to push it (I've actually never tried it).
If trying to mirror PR refs is a futile effort, maybe the right thing to do would be to detect if the action is running in the context of a PR (or fork?) and explicitly fail the action with a message indicating it should only be configured to run for push events 🤔
Ah, I didn't really consider the case for larger repositories where people are creating PRs from other forks. I think your suggestion of only running on push should be sufficient.
Maybe there is something else that can be discussed in this issue, but I would guess that #32 can be closed.
Noteably just running on push isn't quite adequate because when a PR is created, the PR ref still is what populates GITHUB_REF, and so I think the approach of gracefully erroring might be best.
The ref is now configurable with #35 If there's some other ref behavior that's desired, users can configure this.