1240116 - error: commit.eslint hook raised an exception: [Error 2] The system cannot find the file specified

Status: RESOLVED FIXED

(Firefox Build System :: Mach Core, enhancement)

Description

[(Away until May 24) Jared Wein [:jaws] (please needinfo? me)]

When using the eslint commit hook (mozeslint)[1], I get the following error:
error: commit.eslint hook raised an exception: [Error 2] The system cannot find the file specified

`mach eslint path/to/filename` does work though.

This is on Windows, eslint is in my path and works by just typing `eslint` in to my terminal.

[1] mozeslint = c:\fx\tools\mercurial\eslintvalidate.py
http://www.oxymoronical.com/blog/2015/12/Running-ESLint-on-commit

Comment 1

[Dave Townsend [:mossop]]

So the problem here is that attempting to run just "eslint" doesn't seem to search the PATH on windows like it does elsewhere. Other python code in the tree uses which.py to deal with that but I can't assume the user has it installed globally. Is it sane to just load python/which/which.py and use that to find eslint?

Comment 2

[Gregory Szorc [:gps]]

Yes, please import which.which and use it for locating binaries. Also, note that executing files with shebangs need to run in a unix environment or be prefixed with the interpreter .exe in order to work, since CreateProcess() doesn't handle shebangs.

Comment 3

[(Away until May 24) Jared Wein [:jaws] (please needinfo? me)]

I fixed this today locally for me. I had to do the following steps:

1) Make sure that npm is up-to-date
2) npm install npm
2a) This failed due to https://github.com/npm/npm/issues/12887#issuecomment-222525339. To fix this I closed the cmd window and opened it again, this time not resizing the window.
3) I then navigated to /c/program files (x86)/nodejs and ran `npm install npm` successfully
4) Change directory back to my fx-team tree, and run `mach eslint`, which fails because it still doesn't find the file
5) Edit my profile file located at mozilla-build/msys/etc/profile to force nodejs in the path. The path reported through Windows' System Properties / Environment Variables didn't match the $PATH variable that is used in the profile script (before it is reassigned).
5a) I used the old-style directory name for the path so there would be no spaces, /c/PROGRA~2/nodejs

This now works for me, whether the window is default size or maximized.

Comment 4

[(Away until May 24) Jared Wein [:jaws] (please needinfo? me)]

Actually this was not fixed. I only tested with `mach eslint path/to/filename`, which worked in comment #0, but the commit hook still doesn't work.

Comment 6

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

This is caused by the fact that we need to run the local copy of eslint... the global version will not have access to the necessary plugins.

We just need to set the paths in tools/mercurial/eslintvalidate.py.

Comment 7

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

We can't get the binaries using which because this is a local node module... luckily, it is still easy to obtain the correct path.

Comment 8

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: fix-eslint-thing.diff

Comment 9

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Attachment: Use os.path.dirname

Comment 10

[(Away until May 24) Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8777140 [details] [diff] [review]
Use os.path.dirname

Review of attachment 8777140 [details] [diff] [review]:
-----------------------------------------------------------------

I applied your patch and made a new commit with some trailing whitespace at the end of a line but got the following error:

jared@jaws-win10 /c/fx
$ hg qnew temp
error: commit.eslint hook raised an exception: [Error 193] %1 is not a valid Win32 application

Comment 11

[(Away until May 24) Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8777140 [details] [diff] [review]
Use os.path.dirname

Review of attachment 8777140 [details] [diff] [review]:
-----------------------------------------------------------------

I applied your patch and made a new commit with some trailing whitespace at the end of a line but got the following error:

jared@jaws-win10 /c/fx
$ hg qnew temp
error: commit.eslint hook raised an exception: [Error 193] %1 is not a valid Win32 application

::: tools/mercurial/eslintvalidate.py
@@ +41,5 @@
> +            return
> +
> +        dir = os.path.join(basepath, "tools", "lint", "eslint", "node_modules", ".bin")
> +
> +        output = check_output([os.path.join(dir, "eslint"),

The error is happening here. os.path.join(dir, "eslint") returns "c:\fx\tools\lint\eslint\node_modules\.bin\eslint".

I looked in "C:\fx\tools\lint\eslint\node_modules\.bin" and it has both a `eslint` and a `eslint.cmd`. I guess we need to use the .cmd version on Windows?

Comment 12

[(Away until May 24) Jared Wein [:jaws] (please needinfo? me)]

Attachment: Updated patch

I fixed your patch to work on my system. I grant review to your changes, will you bless my tweak? You can keep assignee status on the bug since you did 99% of the work. Thanks!

Comment 13

[Mike Ratcliffe [:miker] [:mratcliffe] [:mikeratcliffe]]

Comment on attachment 8777203 [details] [diff] [review]
Updated patch

Review of attachment 8777203 [details] [diff] [review]:
-----------------------------------------------------------------

I should have thought about Windows... thanks for the tweak!

I was sick of this warning and didn't realize that the mozeslint plugin wasn't working until I looked into it. Now that it works I kinda like it.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7b6b67d2e31d
error: commit.eslint hook raised an exception. r=jaws

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7b6b67d2e31d