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

Can't find node in PATH, trying to find a node binary on your system #18

Closed
EmilienLeroy opened this issue Nov 27, 2019 · 14 comments · Fixed by #45
Closed

Can't find node in PATH, trying to find a node binary on your system #18

EmilienLeroy opened this issue Nov 27, 2019 · 14 comments · Fixed by #45
Labels
bug Something isn't working enhancement New feature or request

Comments

@EmilienLeroy
Copy link

Version of the Action
v2.3.0

Describe the bug
I use husky to run lint before commit. An error appear when the action is call, i think is due to the pre commit.

To Reproduce
This my configuration :

name: Build

on:
  push:
    branches:
    - master

jobs:
  build:

    runs-on: ubuntu-latest

    strategy:
      matrix:
        node-version: [10.x]

    steps:
    - uses: actions/checkout@v1
    - name: Use Node.js ${{ matrix.node-version }}
      uses: actions/setup-node@v1
      with:
        node-version: ${{ matrix.node-version }}
    - name: install
      run: npm install
      
    - name: build
      run: npm run build
      
    - name: Commit changed files
      uses: stefanzweifel/[email protected]
      with:
        commit_message: 📦 Build dist
        branch: master
      env:
        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
        
    - name: Push changes
      uses: ad-m/github-push-action@master
      with:
        github_token: ${{ secrets.GITHUB_TOKEN }}

And i run this during the pre commit :

  "lint-staged": {
    "{src,test}/**/*.ts": [
      "prettier --write",
      "git add"
    ]
  },

Expected behavior
Run the pre commit before commit without error.

Screenshots
https://github.com/Kamiapp-fr/kami-collection/runs/322604011
image

@stefanzweifel
Copy link
Owner

Hi Emilien,

Thanks for the detailed issue!
The problem occours, as the Action run on the alpine/git:1.0.7 docker image where node is not available. (Source)

I first have to test, if and how we could switch to a Docker image that supports both git and node. As husky is quite a popular pre-commit solution I think it would make sense to support it in the Action.

In the meantime you should be able to disable the hooks by passing --no-verify to the Action (if that is something that would work in your workflow)

    - name: Commit changed files
      uses: stefanzweifel/[email protected]
      with:
        commit_message: 📦 Build dist
        branch: master
+       commit_options: '--no-verify'
      env:
        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

@stefanzweifel stefanzweifel added bug Something isn't working enhancement New feature or request labels Nov 27, 2019
@EmilienLeroy
Copy link
Author

Thanks for your answers,

I have made a test without husky and it work perferctly : https://github.com/Kamiapp-fr/kami-collection/commit/2fc3c2a5775a3ab70e3528f4309f1c72c8ad2181/checks?check_suite_id=331010381
image

@stefanzweifel
Copy link
Owner

Great!
I will see what I can do to add support for husky. Will keep you updated through this issue.

zachowj added a commit to zachowj/node-red-contrib-home-assistant-websocket that referenced this issue Jan 1, 2020
@zhangyoufu
Copy link
Contributor

zhangyoufu commented Feb 8, 2020

Please consider rewrite this action using js, so that CLI tools in the host environment can be used.

As easy as a .js file as entry and a shell script. See https://github.com/ad-m/github-push-action

@stefanzweifel
Copy link
Owner

A rewrite in JS/TS has been on my mind lately. (Would like to have a test suite to make sure everything works as expected).

Thanks for the link. Will checkout if we can do something similar in this Action.

@stefanzweifel
Copy link
Owner

I've made some first attempts of an easy rewrite in JS as mentioned by @zhangyoufu here: https://github.com/stefanzweifel/git-auto-commit-action/blob/86f0c11c06a83f9b92c87e0111b1b8f9037a77bd/index.js

However, using the updated Action in a demo results in a broken Workflow (See log here)


My bandwith is quite limited right know and I think the Action works for 80% of the usecases. I've written more thought on a JavaScript rewrite in #44.
If someone in this thread has the time and energy to start working for it, go for it. PRs are most welcomed.

I might revisit the idea of a JS rewrite in 2-3 months.

@zhangyoufu
Copy link
Contributor

However, using the updated Action in a demo results in a broken Workflow (See log here)

You're invoking sh instead of bash in index.js.

const main = async () => {
await exec('sh', [path.join(__dirname, './entrypoint.sh')]);
};

I think change [[ ... ]] to [ ... ] in entrypoint.sh will make this script POSIX shell-compliant.

_git_is_dirty() {
[[ -n "$(git status -s)" ]]
}

@stefanzweifel
Copy link
Owner

@zhangyoufu Thanks. Will give it a try later this week.

@stefanzweifel
Copy link
Owner

Thanks @zhangyoufu for the help. Your suggestion solved the issues.
I've prepared a PR, which changes the Action to use node12 instead of Docker. See #45.

I would appreciated it if any of you would test the changes in your projects before I merge and release a new major version.

Update your workflow files to use the PR branch like this.

- - uses: stefanzweifel/[email protected]
+ - uses: stefanzweifel/git-auto-commit-action@refactor/switch-to-js

Please leave your feedback in the PR #45.

@EmilienLeroy
Copy link
Author

I have tried with the js version. Husky work well but i get an error for the commit.
https://github.com/Kamiapp-fr/kami-collection/runs/464095052
image

My config file:
https://github.com/Kamiapp-fr/kami-collection/blob/test/pre-commit/.github/workflows/build.yml

name: Build

on:
  push:
    branches:
    - master
    - test/pre-commit

jobs:
  build:

    runs-on: ubuntu-latest

    strategy:
      matrix:
        node-version: [10.x]

    steps:
    - uses: actions/checkout@v1
    - name: Use Node.js ${{ matrix.node-version }}
      uses: actions/setup-node@v1
      with:
        node-version: ${{ matrix.node-version }}
    - name: install
      run: npm install
      
    - name: build
      run: npm run build
      
    - name: Commit changed files
      uses: stefanzweifel/git-auto-commit-action@refactor/switch-to-js
      with:
        commit_message: 📦 Build dist
        branch: test/pre-commit
      env:
        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
        
    - name: Push changes
      uses: ad-m/github-push-action@master
      with:
        github_token: ${{ secrets.GITHUB_TOKEN }}

@stefanzweifel
Copy link
Owner

The error comes from a breaking change we made in v3.0.0 of this Action.
I've modified your workflow file below:

  • actions/checkout@v2 is required
  • GITHUB_TOKEN is no longer required
name: Build

on:
  push:
    branches:
    - master
    - test/pre-commit

jobs:
  build:

    runs-on: ubuntu-latest

    strategy:
      matrix:
        node-version: [10.x]

    steps:
-    - uses: actions/checkout@v1
+    - uses: actions/checkout@v2
    - name: Use Node.js ${{ matrix.node-version }}
      uses: actions/setup-node@v1
      with:
        node-version: ${{ matrix.node-version }}
    - name: install
      run: npm install
      
    - name: build
      run: npm run build
      
    - name: Commit changed files
      uses: stefanzweifel/git-auto-commit-action@refactor/switch-to-js
      with:
        commit_message: 📦 Build dist
        branch: test/pre-commit
-      env:
-        GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
        
    - name: Push changes
      uses: ad-m/github-push-action@master
      with:
        github_token: ${{ secrets.GITHUB_TOKEN }}

@EmilienLeroy
Copy link
Author

It's work better with a good configuration ;)
All is okey for me, husky and the commit.
https://github.com/Kamiapp-fr/kami-collection/runs/464144510?check_suite_focus=true

Thanks you for your help 😄

@stefanzweifel
Copy link
Owner

@EmilienLeroy Great! I will merge this PR and release a new major version (4.0.0) very soon.
You probably have to update your config again, as the PR branch will go away.

I keep you updated through this issue.

@stefanzweifel
Copy link
Owner

@EmilienLeroy v4.0.0 has been released: https://github.com/stefanzweifel/git-auto-commit-action/releases/tag/v4.0.0

Update your workflow file to use the latest version.

    - name: Commit changed files
-      uses: stefanzweifel/git-auto-commit-action@refactor/switch-to-js
+      uses: stefanzweifel/[email protected]
      with:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants