Cannot use the lint commit hooks on Windows

RESOLVED FIXED in Firefox 56

Status

Firefox Build System
Lint and Formatting
RESOLVED FIXED
9 months ago
2 months ago

People

(Reporter: mak, Assigned: ahal)

Tracking

Version 3
mozilla56

Firefox Tracking Flags

(firefox56 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

I'm trying to setup the lint commit hooks on Windows 10, but I get errors:

[hooks]
pre-push.lint = python:D:\src\tools\lint\hooks.py:hg
pretxncommit.lint = python:D:\src\tools\lint\hooks.py:hg

error: pretxncommit.lint hook raised an exception: [Error 193] %1 non Þ un'applicazione di Win32 valida
transaction abort!
rollback completed
abort: %1 non Þ un'applicazione di Win32 valida

It's the usual "not a valid win32 application" error.
Does running `D:\src\tools\lint\hooks.py` from the command prompt work? (By work I mean find the interpreter, you'll definitely get errors afterwards).

When you try to execute a python file on Windows, it should automatically find the interpreter:
https://docs.python.org/2.7/faq/windows.html#how-do-i-make-python-scripts-executable

If that's not the case, there may be something wrong with your python installation. It's also possible there's some mercurial weirdness happening here, though the mercurial docs don't mention any problems with hooks on Windows.
The Windows native path clearly doesn't work in the mingw32 shell, I tried:
$ /d/src/tools/lint/hooks.py
warning: 'hooks.py' is not a valid mozlint hooktype
But If I try to use that path in Mercurial.ini I get:
abort: No such file or directory: d:/d/src/tools/lint/hooks.py
I forgot for comment2, it is set like this:
pretxncommit.lint = python:/d/src/tools/lint/hooks.py:hg

I didn't try mozilla build 3.0 yet, could be that helps.
Does the hook rely on the shebang working? that's not going to work on Windows.
I replaced the cmd definition with
    cmd = [sys.executable, os.path.join(topsrcdir, 'mach'), 'lint', '--quiet']
and it seems to work.
I don't know if this change may be fine across all the platforms?
(In reply to Andrew Halberstadt [:ahal] from comment #1)
> When you try to execute a python file on Windows, it should automatically
> find the interpreter:
> https://docs.python.org/2.7/faq/windows.html#how-do-i-make-python-scripts-
> executable

It states "the standard Python installer already associates the .py extension with a file type". This is not the case for us, because we use mozillaBuild that doesn't do python association, instead it creates its own env.
Summary: Cannot use the link commit hooks on Windows → Cannot use the lint commit hooks on Windows
Comment hidden (mozreview-request)
I briefly tested this on Win and Mac, and it seems to work properly.
Comment hidden (mozreview-request)
(Assignee)

Comment 9

9 months ago
mozreview-review
Comment on attachment 8889313 [details]
Bug 1382140 - Cannot use the lint commit hooks on Windows.

https://reviewboard.mozilla.org/r/160392/#review165718

This works properly if the build system virtualenv was also created with sys.executable. But with the commit hook enabled, and doing this:

$ mach clobber
$ ./mach lint -o  # creates build system virtualenv
$ hg commit

I'm getting this error:

calling hook pretxncommit.lint: hghook_pretxncommit_lint.hg
Overwriting /home/ahal/hg/mozilla-central/objdirs/opt/_virtualenv/lib64/python2.7/orig-prefix.txt with new content
New python executable in /home/ahal/hg/mozilla-central/objdirs/opt/_virtualenv/bin/python
Traceback (most recent call last):
  File "/home/ahal/hg/mozilla-central/third_party/python/virtualenv/virtualenv.py", line 2325, in <module>
    main()
  File "/home/ahal/hg/mozilla-central/third_party/python/virtualenv/virtualenv.py", line 711, in main
    symlink=options.symlink and hasattr(os, 'symlink')) # MOZ: Make sure we don't use symlink when we don't have it
  File "/home/ahal/hg/mozilla-central/third_party/python/virtualenv/virtualenv.py", line 924, in create_environment
    site_packages=site_packages, clear=clear, symlink=symlink))
  File "/home/ahal/hg/mozilla-central/third_party/python/virtualenv/virtualenv.py", line 1385, in install_python
    raise e
OSError: [Errno 40] Too many levels of symbolic links

This is because `sys.executable` and `which python` return two different things on my system, I don't think we can rely on using it in the hook. Sorry I haven't been paying attention to this, I just got a new laptop I'm in the process of setting a Windows environment up on, I'll try and reproduce your issue today and see if there's a good way of fixing it.
Attachment #8889313 - Flags: review?(ahalberstadt) → review-
I guess I misunderstood the problem earlier too, it's not that we can't find python, it's that we can find #!/bin/sh (from the top of the mach command). So maybe we just need to imitate `which python` instead of hardcoding sys.executable. I'll do some testing.
Comment hidden (mozreview-request)
Marco, this gets me passed the error, but I see eslint setup errors afterwards.. Though I'm pretty sure I just don't have my system set up properly yet. Please verify this gets everything working for you.
(Reporter)

Comment 13

9 months ago
mozreview-review
Comment on attachment 8889509 [details]
Bug 1382140 - Call 'mach' with python from tools/lint/hooks.py so it works on Windows,

https://reviewboard.mozilla.org/r/160534/#review165790

Yes, WFM!
Attachment #8889509 - Flags: review?(mak77) → review+
Attachment #8889313 - Attachment is obsolete: true
Assignee: nobody → ahalberstadt
Status: NEW → ASSIGNED

Comment 14

9 months ago
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f91d86bb4b2a
Call 'mach' with python from tools/lint/hooks.py so it works on Windows, r=mak

Comment 15

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f91d86bb4b2a
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56

Updated

2 months ago
Product: Testing → Firefox Build System
You need to log in before you can comment on or make changes to this bug.