Error using {push_files}

[cloudmatt]

Summary

Whenever I pass the {push_files} argument to a run: in the pre-push section I just get this error message that says error replacing {push_files}: exit status 128

Steps to reproduce

Here's the relevant sections my lefthook.yaml file:

---
skip_output:
  - meta       # Skips lefthook version printing
  - success    # Skips successful steps printing
pre-push:
  parallel: true
  commands:
    shellcheck:
      tags: sast
      glob: "*.sh"
      run: scripts/shellcheck.sh {push_files}

Contents of shellscheck.sh though I'm sure it doesn't matter because it happens no matter what I use but adding for completeness sake:

#!/usr/bin/env bash
set -euo pipefail

if ! command -v shellcheck > /dev/null; then
  echo "WARN: shellcheck is not installed. Installing....."
  if [[ "$(uname)" == "Darwin" ]]; then
    if ! command -v brew > /dev/null; then
      echo "ERROR: Homebrew isn't installed. Please see https://docs.brew.sh/Installation for instructions."
      exit 1
    else
      brew install -q shellcheck
    fi
  else
    echo "ERROR: Non macOS operating systems not supported at this time. Feel free to open a PR and extend this check :)"
  fi
fi

for file in "$@"
do
  shellcheck -o all "${file}"
done

Expected results

I expect it to not error? lol. I know the file i'm testing should cause an error to return from my script. I've also tried other scripts in the run directive and the same results.. anytime I reference the {push_files} I get that same error.

Actual results

I make adjustments to a *.sh file and when I try to push up the change to remote up I get the error [snipped branch name details]:

➜  git push origin BRANCH_NAME
error replacing {push_files}: exit status 128
shellcheck: (SKIP. ERROR)

refs/heads/style/BRANCH_NAME branch_sha refs/heads/style/BRANCH_NAME branch_sha

SUMMARY: (done in 0.09 seconds)

Possible Solution

🤷🏻

[cloudmatt]

Well, I guess I know more golang that I thought... this line is causing the error. Primary branch names are no longer called master and are usually called main but could also be called any other thing. Should this be configurable?

for now i've just switched to using to work around it:

    shellcheck:
      tags: sast
      files: git diff --name-only HEAD @{push} || git diff --name-only HEAD main
      glob: "*.sh"
      run: scripts/shellcheck.sh {files}

[mrexox]

Hi! Thank you for the issue. I knew that someday it would shoot. I think this command should be changed to get the diff from the first parent of the current branch. However this could break it for other people using the tool if I put it to the fix version. I think this can be done for the next minor/major release of lefthook. What do you think?

[cloudmatt]

Hi! Thank you for the issue. I knew that someday it would shoot. I think this command should be changed to get the diff from the first parent of the current branch. However this could break it for other people using the tool if I put it to the fix version. I think this can be done for the next minor/major release of lefthook. What do you think?

I think including it in the next major release would make sense to me, that way folks can opt-out or give time to fix any issues it may have with their current setup.

[blalor]

Just ran into this when pushing the PR to a repo that (wait for it) uses the pre-push hook. 😆 I'd love to see this fixed, or made configurable. Maybe the upstream ref should be targeted instead of a hard-coded (or even configurable) branch? Something like this might provide a more reasonable default?

[quoc-huynh-cosee]

Any updates on this?