1749681 - `mach: line 6: from: command not found` when committing files for which clang and/or js format wants to run

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, defect)

Description

[:Gijs (he/him)]

My hgrc has:

[extensions]
...
clang-format = path\to\home\.mozbuild\version-control-tools\hgext\clang-format
js-format = path\to\home\.mozbuild\version-control-tools\hgext\js-format

Amending a commit with C++ files with hg amend on a mozilla-build shell, I get:

mach: line 6: from: command not found
mach: line 8: import: command not found
mach: line 9: import: command not found
mach: line 10: import: command not found
mach: line 11: from: command not found
mach: line 14: syntax error near unexpected token `('
mach: line 14: `def load_mach(dir_path, mach_path):'

This is from central from Sunday, https://hg.mozilla.org/mozilla-central/rev/b81970e39db4 .

Comment 1

[:Gijs (he/him)]

This also appears to happen for pure JS patches.

Comment 3

[:Gijs (he/him)]

Per the dupe,

(In reply to Masatoshi Kimura [:emk] from comment #0)

Reverting changes to mach in https://hg.mozilla.org/mozilla-central/rev/cd1960daa9e237b9f57e6997de49cf1ebca6a7b3 fixed the issue.

Comment 4

[:Gijs (he/him)]

AIUI Mitchell is on PTO. Glandium, can you take a look?

I am still able to run ./mach clang-format successfully from a mozillabuild shell, but the hg hook complains as noted. Masayuki, is this the same for you?

Comment 5

[Masatoshi Kimura [:emk]]

Attachment: Bug 1749681 - Re-introduce "valid POSIX shell and valid Python" trick

Comment 6

[Masatoshi Kimura [:emk]]

Reverting patch for the record.

Comment 7

[:Gijs (he/him)]

(In reply to :Gijs (he/him) from comment #4)

I am still able to run ./mach clang-format successfully from a mozillabuild shell, but the hg hook complains as noted. Masayuki, is this the same for you?

Gah, I am very sorry, I meant to ask :emk this.

Comment 8

[Masatoshi Kimura [:emk]]

(In reply to :Gijs (he/him) from comment #4)

I am still able to run ./mach clang-format successfully from a mozillabuild shell, but the hg hook complains as noted. Masayuki, is this the same for you?

Yes, the msys shell understands shebang and run the patched mach as a python script. Apparently hg hooks always run mach as a shell script. I don't know why.

Comment 9

[Masatoshi Kimura [:emk]]

./mach clang-format works, but sh ./mach clang-format does not:

$ ./mach clang-format

$ sh ./mach clang-format
./mach: line 6: from: command not found
./mach: line 8: import: command not found
./mach: line 9: import: command not found
./mach: line 10: import: command not found
./mach: line 11: from: command not found
./mach: line 14: syntax error near unexpected token `('
./mach: line 14: `def load_mach(dir_path, mach_path):'

$ 

Maybe here:
https://searchfox.org/version-control-tools/rev/d056e7eb20a0684542d963d00c4d93601f3127a4/hgext/clang-format/__init__.py#56-57
https://searchfox.org/version-control-tools/rev/d056e7eb20a0684542d963d00c4d93601f3127a4/hgext/js-format/__init__.py#51-52

Comment 10

[Masatoshi Kimura [:emk]]

Changing [b'sh', b'mach'] to [b'python3', b'mach'] works.

Comment 11

[Masatoshi Kimura [:emk]]

[b'python3', b'mach'] works with or without cd1960daa9e2. I think we should update version-control-tools.

Comment 12

[:Gijs (he/him)]

(In reply to Masatoshi Kimura [:emk] from comment #11)

[b'python3', b'mach'] works with or without cd1960daa9e2. I think we should update version-control-tools.

:sheehan, does this seem right and can you help with this if needed?

Comment 13

[Connor Sheehan [:sheehan]]

Yes, this seems reasonable. I'll post a patch.

Comment 14

[Connor Sheehan [:sheehan]]

Attachment: hooks/format: call `mach` with `python3` directly instead of through `sh` (Bug 1749681) r?mhentges,#firefox-build-system-reviewers

Previously mach was both valid shell and valid Python. After all the recent
mach work, the need to have mach run as a shell script is gone and the
hack has been removed, leaving mach as a pure-Python script. Nice!

We took advantage of this hack when running the commit hook extensions
on Windows, though I can't remember exactly why. In any case we can move
to calling mach via python3 directly instead of passing through sh.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/b9d4895f4dac
hooks/format: call mach with python3 directly instead of through sh r=firefox-build-system-reviewers,glandium