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

BASE_SHA default set incorrectly #51

Closed
GeReV opened this issue Aug 1, 2022 · 1 comment · Fixed by #52
Closed

BASE_SHA default set incorrectly #51

GeReV opened this issue Aug 1, 2022 · 1 comment · Fixed by #52

Comments

@GeReV
Copy link

GeReV commented Aug 1, 2022

The code path which attempts to set a default value on BASE_SHA (

process.stdout.write('\n');
process.stdout.write(`WARNING: Unable to find a successful workflow run on 'origin/${mainBranchName}'\n`);
process.stdout.write(`We are therefore defaulting to use HEAD~1 on 'origin/${mainBranchName}'\n`);
process.stdout.write('\n');
process.stdout.write(`NOTE: You can instead make this a hard error by setting 'error-on-no-successful-workflow' on the action in your workflow.\n`);
BASE_SHA = execSync(`git rev-parse HEAD~1`, { encoding: 'utf-8' });
) seems to be using an incorrect Git query.

The printout states that "We are therefore defaulting to use HEAD~1 on 'origin/${mainBranchName}".
However, following statement on line 50: BASE_SHA = execSync(`git rev-parse HEAD~1`, { encoding: 'utf-8' }); queries for HEAD~1 on the current branch, instead of specifying the main branch.

(My assumption here that the printed statement is the expected outcome when the action trigger is push and no previous successful run exists).

In addition, if the intention was indeed using the main branch, it seems that using git rev-parse origin/${mainBranchName} or the previously set git merge-base origin/${mainBranchName} HEAD would be preferred.

My workflow configuration for this action is:

name: NX Lint
on:
  push:
jobs:
  main:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v2
        with:
          # We need to fetch all branches and commits so that Nx affected has a base to compare against.
          fetch-depth: 0
      - uses: actions/setup-node@v2
        with:
          node-version: 16
          cache: 'yarn'
      - uses: nrwl/nx-set-shas@v2
        with:
          main-branch-name: master
      - run: yarn install --frozen-lockfile
      - run: yarn nx affected --target=lint --parallel=3
@meeroslav
Copy link
Collaborator

Hey @GeReV,

thank you for reporting this. Indeed, the wording here is not 100% clear.

The assumption (and intended usage of this action) is to run the action or pull request to the main branch or push on the main.

Therefore if you're already in the else branch on

if (eventName === 'pull_request') {
BASE_SHA = execSync(`git merge-base origin/${mainBranchName} HEAD`, { encoding: 'utf-8' });
} else {
the assumption is that the event is running on the main branch. Of course, this might not be the case if you configured your workflow to be other than (e.g. using tags, listening to events on release, develop etc. branches):

on:
  push:
    branches:
      - main
  pull_request:

In practice, it should not make a difference, but using git rev-parse origin/${mainBranchName}~1 would be more accurate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants