Wrong `BuildingRef` when a prior GitHub action moves HEAD
vchirikov opened this issue · 13 comments
Hi,
I use PR comment commands like /publish with github actions, and as you might know that workflows triggered on issue_comment started from the default branch (master in my case), thus the
works wrong.
I have something like this in workflow:
- name: git checkout / on 'workflow_call'
if: ${{ inputs.ref != '' }}
uses: actions/checkout@v3
with:
clean: false
# branch name
ref: ${{ inputs.ref }}
fetch-depth: 0
...
- name: Generate version variables
id: nbgv
uses: dotnet/nbgv@master
with:
setAllVars: true
And I can't do something like echo "GITHUB_REF=refs/heads/$branch" >> $GITHUB_ENV because:
If you attempt to override the value of one of these default environment variables, the assignment is ignored.
So I can't override the wrong GITHUB_REFvalue and because of that nbgv step generates wrong version info.
Do you have any ideas how to beat this?
cc: @AArnott
It will be nice if we could disable cloud logic at all (in version.json for example)
What do you mean by "disable cloud logic at all"? Are you wanting the nbgv GitHub Action to disregard the environment variables and just return results based on the position of HEAD? That could certainly be done, but CI systems often checkout a detached HEAD, so the publicReleaseRefSpec in version.json wouldn't work properly because that tends to be based on an attached HEAD. That's the chief reason why we look an environment variables in CI systems: it's the only way to know the name of the branch being built.
We could perhaps have an alternate environment variable name that we check first and use that if it's present, so you could define that. But before we go there, would something like this work?
- name: Generate version variables
id: nbgv
uses: dotnet/nbgv@master
with:
setAllVars: true
env:
GITHUB_REF: ${{ inputs.ref }}Maybe that syntax would be honored to set the env var just for that task.
I know you mentioned that echo "GITHUB_REF=refs/heads/$branch" >> $GITHUB_ENV wouldn't work for the reason given in the doc, but maybe this alternate syntax would work? Just a shot.
Maybe it will work, for now as a workaround I use dotnet nbgv from Bullseye target with my own env variables
Ok, now I ran into this in a js repo (without dotnet build project), checked your solution and it doesn't work:
- name: Forward env variables & checkout fixes
id: gh
run: |
echo -n "" > .env
env | grep -E '^(GITHUB_|CI)' | grep -E GITHUB_SHA= --invert-match | grep -E GITHUB_REF= --invert-match >> $GITHUB_ENV
env | grep -E '^(GITHUB_|CI)' | grep -E GITHUB_SHA= --invert-match | grep -E GITHUB_REF= --invert-match >> .env
sha=$(git rev-parse HEAD)
echo "GITHUB_SHA=$sha" >> $GITHUB_ENV
echo "GITHUB_SHA=$sha" >> .env
branch=$(git rev-parse --abbrev-ref HEAD)
echo "GITHUB_REF=refs/heads/$branch" >> $GITHUB_ENV
echo "GITHUB_REF=refs/heads/$branch" >> .env
echo -e "\033[38;5;99;5;16msha: $sha\033[0m"
echo -e "\033[38;5;99;5;16mbranch: $branch\033[0m"
echo -e "\033[38;5;99;5;16minputs.ref: ${{ inputs.ref }}\033[0m"
echo "::set-output name=sha::$sha"
echo "::set-output name=branch::$branch"
echo "::add-matcher::./.github/eslint-compact.json"
echo "::add-matcher::./.github/eslint-stylish.json"
echo "::add-matcher::./.github/tsc.json"
- name: Generate version
id: nbgv
uses: dotnet/nbgv@master
with:
setAllVars: true
env:
GITHUB_REF: refs/heads/${{ steps.gh.outputs.branch }}
GITHUB_SHA: ${{ steps.gh.outputs.sha }}cc: @AArnott
As a workaround we can use github-script action:
- name: Generate version
id: nbgv
uses: actions/github-script@v6
with:
script: |
const path = require('path');
const os = require('os');
process.env.GITHUB_REF='refs/heads/${{ steps.gh.outputs.branch }}';
process.env.GITHUB_SHA='${{ steps.gh.outputs.sha }}';
try {
// install nbgv
let exitCode = await exec.exec('dotnet', ['tool', 'install', '-g', 'nbgv'], { ignoreReturnCode: true });
if (exitCode > 1) {
throw new Error("dotnet tool install failed.");
}
// add .dotnet/tools to the path
core.addPath(path.join(os.homedir(), '.dotnet', 'tools'));
// collect a JSON string of all the version properties.
let versionJson = '';
await exec.exec('nbgv', ['get-version', '-f', 'json'], { listeners: { stdout: (data) => { versionJson += data.toString() } } });
core.setOutput('versionJson', versionJson);
// break up the JSON into individual outputs.
const versionProperties = JSON.parse(versionJson);
for (let name in versionProperties.CloudBuildAllVars) {
// Trim off the leading NBGV_
core.setOutput(name.substring(5), versionProperties.CloudBuildAllVars[name]);
}
await exec.exec('nbgv', ['cloud', '-a']);
// stamp the version on package.json
await exec.exec('npm', ['version', versionProperties.NpmPackageVersion, '--git-tag-version=false', '--allow-same-version']);
}
catch (error) {
core.setFailed(error.message);
}But it's better to add ref/sha inputs to this action.
It will be nice if we could disable cloud logic at all
I could certainly add an input to the nbgv github action (and tool) to disable cloud recognition. Would that do what you want?
Actually I guess that wouldn't work for you. The whole point of using this Action is to set env vars, which requires cloud awareness. What you really want I think is a way to override what the cloud says HEAD is pointing at, right?
@vchirikov Can you please try this in your pipeline? It uses a new environment variable, and instructs the nbgv Action to pull down the newer version of the tool that is still in an open PR. So you wouldn't want to keep this long-term, but it would serve to test the content of dotnet/Nerdbank.GitVersioning#795 and #51.
- name: Generate version
id: nbgv
uses: dotnet/nbgv@addFeed
with:
setAllVars: true
toolVersion: 3.6.20-alpha-gaf2d6026c0
toolFeed: https://pkgs.dev.azure.com/andrewarnott/OSS/_packaging/PublicCI/nuget/v3/index.json
env:
IGNORE_GITHUB_REF: trueI guess I should use IGNORE_GITHUB_REF: true instead of false :)
With this change GitCommit is correct, but "BuildingRef": "refs/heads/master" is wrong.
https://github.com/vchirikov/gh-vstest-logger/runs/7817834798?check_suite_focus=true#step:5:110
Yes, thanks for testing and correcting my mistake. I've edited my comment above for future references to it to be correct. I'll review the branch name being wrong and let you know when I have an update for you to test.
@vchirikov Sorry for the delay. I think the code was all correct, but now that nbgv is included in GitHub Actions agents, the nbgv GitHub Action skipped installing the precise version of the nbgv tool we needed to test. I've pushed a fix for this to the action's addFeed branch that you're using. Can you please re-queue your github workflow and share the results? Does it address the BuildingRef issue?
Looks like works fine, BuildingRef & CommitId are ok with nbgv 3.6.20-alpha-gaf2d6026c0
Wonderful.
You can switch your nbgv action back to sourcing from the v0.4 or the master branch.
You'll have to keep pinning the toolVersion and specifying the toolFeed until 3.6 is stable and pushed to nuget.org. v3.6.40-alpha or later are the official CI builds that contain this fix.
- name: Generate version
id: nbgv
uses: dotnet/nbgv@v0.4
with:
setAllVars: true
toolVersion: 3.6.40-alpha # or later
toolFeed: https://pkgs.dev.azure.com/andrewarnott/OSS/_packaging/PublicCI/nuget/v3/index.json
env:
IGNORE_GITHUB_REF: true