Attempting to setup eslint using |mach eslint -s| from bug 1203520, and I'm getting the attached error log. Windows 10, latest tree, NodeJS v4.1.1 (NPM v2.14.4).

I am surprised by this because we trap WindowsError.

Change to using set literal syntax r?=pbrosset

Bug 1211223 - Error running eslint setup on Windows r?=pbro Seems like these changes were dropped when I experienced hg corruption.

I have applied both changes locally and I'm still getting the same error. If I add some logs, I can see that the npmPath mach is trying to use to install eslint is 'c:\Program Files\nodejs\npm' which is correct, but calling this from inside the Mozilla Build shell thing still fails with: 'WindowsError: [Error 193] %1 is not a valid Win32 application'.

Comment on attachment 8670192 [details] MozReview Request: Change to using set literal syntax r?=pbrosset Redirecting to :gps who, I think, did the original review of the mach command.

Comment on attachment 8670193 [details] MozReview Request: Bug 1211223 - Error running eslint setup on Windows r?=pbro Redirecting to :gps who, I think, did the original review of the mach command.

See comment 5, I don't think these patches fix the problem. Also, on IRC this week several people have been reporting that ./mach eslint has been broken on both mac and win. Mike, I think an email to a few mailing-lists about using the --setup option would be useful (or quickly fix bug 1211483 maybe).

(In reply to Patrick Brosset [:pbrosset] [:pbro] from comment #8) > See comment 5, I don't think these patches fix the problem. > Also, on IRC this week several people have been reporting that ./mach eslint > has been broken on both mac and win. Mike, I think an email to a few > mailing-lists about using the --setup option would be useful (or quickly fix > bug 1211483 maybe). This is awkward because it works fine on both my virtual and physical Windows 10 boxes so I am completely unable to reproduce the issue... it should just work. I am starting to suspect that this could be due to using old versions of node and npm so I need to look into that. It could also be that people are not running the command from the root of the project. Of course, fixing bug 1211483 would make sense if ./mach eslint --setup always worked for everybody.

Comment on attachment 8670192 [details] MozReview Request: Change to using set literal syntax r?=pbrosset https://reviewboard.mozilla.org/r/21339/#review19533 ::: python/mach_commands.py:269 (Diff revision 1) > - return { > + return list({ [ ] is Python's list literal syntax (just like JS). return [ ... ]

Comment on attachment 8670193 [details] MozReview Request: Bug 1211223 - Error running eslint setup on Windows r?=pbro https://reviewboard.mozilla.org/r/21341/#review19535 ::: python/mach_commands.py:318 (Diff revision 1) > - except subprocess.CalledProcessError: > + except (subprocess.CalledProcessError, WindowsError): What is raising WindowsError? It is extremely rare to see WindowsError references outside low-level ctypes/ffi code calling Win32 APIs.

(In reply to Gregory Szorc [:gps] (PTO until Nov 3) from comment #11) > Comment on attachment 8670193 [details] > MozReview Request: Bug 1211223 - Error running eslint setup on Windows > r?=pbro > > https://reviewboard.mozilla.org/r/21341/#review19535 > > ::: python/mach_commands.py:318 > (Diff revision 1) > > - except subprocess.CalledProcessError: > > + except (subprocess.CalledProcessError, WindowsError): > > What is raising WindowsError? It is extremely rare to see WindowsError > references outside low-level ctypes/ffi code calling Win32 APIs. It is because we are within the MING32 shell... Attempting to run e.g. npm may not work if environment variables first need to be set via npm.cmd It appears that this patch doesn't fix the issue for everybody... I suspect outdated npm or node versions could be the culprit.

Just noticed that Blair included his version info: - NodeJS v4.1.1 - NPM v2.14.4 I will try to replicate the issue with that setup.

Can somebody send the output of ./mach eslint -s with this patch?

Sorry, try this one.

Works on Win10

This should fix it.

Comment on attachment 8676909 [details] [diff] [review] eslint.patch Review of attachment 8676909 [details] [diff] [review]: ----------------------------------------------------------------- Tested locally, this fixes the issue for me on Windows 10! So that's great, we need to get this in quickly. I'm not a peer for this however, so I don't think I can R+ it. :gps has been reviewing your earlier patches, but it looks like he is on PTO this week and the next, so transferring this to :glandium.

Comment on attachment 8676909 [details] [diff] [review] eslint.patch Review of attachment 8676909 [details] [diff] [review]: ----------------------------------------------------------------- ::: python/mach_commands.py @@ +287,5 @@ > + except which.WhichError: > + pass > + else: > + try: > + appPath = which.which(filename) You might as well directly `return which.which(filename)`

https://reviewboard.mozilla.org/r/21341/#review19535 > What is raising WindowsError? It is extremely rare to see WindowsError references outside low-level ctypes/ffi code calling Win32 APIs. It is because we are within the MINGW32 shell... attempting to run e.g. npm may raise a WindowsError if environment variables first need to be set via npm.cmd

https://reviewboard.mozilla.org/r/21337/#review20887 Already shipped