1571309 - Support linting files outside of the srcdir with `mach lint`

Status: RESOLVED WONTFIX

(Developer Infrastructure :: Lint and Formatting, enhancement, P5)

Description

[alta88]

I have extension code in:
../mozilla/extensions/myextension/bootstratp.js
and would like to run lint from the m-c repo:
../mozilla/builds/mozilla-central/mach lint ../../extensions/myextension/bootstrap.js
to lint using all the mozilla rules, of course. There is also an eslintrc here:
../mozilla/extensions/.eslintrc

The problem is that apparently lint looks for a ~/.mozbuild while that directory is in
../mozilla/builds/.mozbuild
which works fine to build firefox, but since there's no homedir file, lint can't find node and fails, while also doing some sort of bootstrap which creates ~/.mozbuild/gecko-strings.

Lint should look up .mozbuild where it's configured for the rest of the build system and not the homedir.

Comment 1

[Mark Banner (:standard8)]

We already publish the core rules for ESLint via eslint-plugin-mozilla. Why not just use that?

Here's an example: https://github.com/protz/thunderbird-conversations. Note that it is run via npm test and it is capable of being run on Travis as well.

Also, looking at the code, I can't see where Lint or at least ESLint would be getting the wrong directory from - it only uses .mozbuild for looking for npm/node executables, and the first thing it looks at there is get_state_dir which is where everything else gets its mozbuild directory from.

Comment 2

[alta88]

I don't want to have a node_modules in /extensions, I would like to only use the tools in the mozilla-central repo. I changed the /extensions/.eslintrc to match the thunderbird-conversations .eslintrc you linked to and ran:
>../mozilla/builds/mozilla-central/mach lint ../../extensions/myextension/bootstrap.js
the result was:

>error: problem with lint setup, skipping android-api-lint, android-checkstyle, android-findbugs, android-lint, android-test ✖ 0 problems (0 errors, 0 warnings)

If I configure the editor to issue a lint command on the (known bad) bootstrap.js file, the result is:

./mach lint "/mnt/DATA/projects/mozilla/extensions/AttachmentCount/bootstrap.js" (in directory: /mnt/DATA/projects/mozilla/extensions/AttachmentCount/../../builds/mozilla-central)
Could not find Node.js executable later than 8.11.
Executing `mach bootstrap --no-system-changes` should
install a compatible version in ~/.mozbuild on most platforms.
error: problem with lint setup, skipping android-api-lint, android-checkstyle, android-findbugs, android-lint, android-test, eslint
? 0 problems (0 errors, 0 warnings)
Compilation finished successfully.

And if there were no ~/.mozbuild, one will be created. So something clearly wants a .mozbuild in the homedir.
Thanks for any further advice.

Comment 3

[Mark Banner (:standard8)]

(In reply to alta88 from comment #2)

I don't want to have a node_modules in /extensions, I would like to only use the tools in the mozilla-central repo.

I don't see why having some extra setup in your repo is an issue (you can also ignore the node_modules so you don't have to check it in if you're using a repo). You're welcome to try using the ones directly in mozilla-central, just remember that we're not expecting them to be used like that and we're probably not going to support running it in that fashion.

I changed the /extensions/.eslintrc to match the thunderbird-conversations .eslintrc you linked to and ran:
>../mozilla/builds/mozilla-central/mach lint ../../extensions/myextension/bootstrap.js

Now I think about it, I don't know what ESLint will make of relative directories outside of the main directory. mach lint also doesn't seem to be aware of relative directories.

If I configure the editor to issue a lint command on the (known bad) bootstrap.js file, the result is:

Could not find Node.js executable later than 8.11.
Executing `mach bootstrap --no-system-changes` should
install a compatible version in ~/.mozbuild on most platforms.

And if there were no ~/.mozbuild, one will be created. So something clearly wants a .mozbuild in the homedir.
Thanks for any further advice.

The error message is just saying ~/.mozbuild when in actual fact it is also looking in the environment variable location - MOZBUILD_STATE_PATH. I suspect you haven't got node installed anywhere on your system.

./mach bootstrap --no-system-changes will install node into that directory (either the home one or by the env var). We prefer that because then we know we should be getting a specific version of node to use.

However, we do currently (though it may change in future) support node being installed in the path.

If you install node, run bootstrap or make sure its available in the path, then it might get you a bit further.

Comment 4

[alta88]

(In reply to Mark Banner (:standard8) from comment #3)

(In reply to alta88 from comment #2)

I don't want to have a node_modules in /extensions, I would like to only use the tools in the mozilla-central repo.

I don't see why having some extra setup in your repo is an issue (you can also ignore the node_modules so you don't have to check it in if you're using a repo). You're welcome to try using the ones directly in mozilla-central, just remember that we're not expecting them to be used like that and we're probably not going to support running it in that fashion.

It's an issue because the distro node_modules aren't necessarily anywhere near current with the m-c repo shipped version.

I changed the /extensions/.eslintrc to match the thunderbird-conversations .eslintrc you linked to and ran:
>../mozilla/builds/mozilla-central/mach lint ../../extensions/myextension/bootstrap.js

Now I think about it, I don't know what ESLint will make of relative directories outside of the main directory. mach lint also doesn't seem to be aware of relative directories.

It makes no difference in either case whether the paths are absolute.

If I configure the editor to issue a lint command on the (known bad) bootstrap.js file, the result is:

Could not find Node.js executable later than 8.11.
Executing `mach bootstrap --no-system-changes` should
install a compatible version in ~/.mozbuild on most platforms.

And if there were no ~/.mozbuild, one will be created. So something clearly wants a .mozbuild in the homedir.
Thanks for any further advice.

The error message is just saying ~/.mozbuild when in actual fact it is also looking in the environment variable location - MOZBUILD_STATE_PATH.

Well, I think you can see why a misleading message would cause confusion.. The env var is indeed set to the correct .mozbuild location.

I suspect you haven't got node installed anywhere on your system.

I used to and removed it, too many issues what with conflicts between tb and fx and the extensions dir.

./mach bootstrap --no-system-changes will install node into that directory (either the home one or by the env var). We prefer that because then we know we should be getting a specific version of node to use.

Yes, it honors the env var and puts node there. Then why, as I said, is ~/.mozbuild/gecko-strings being created, and by whom?

However, we do currently (though it may change in future) support node being installed in the path.

If you install node, run bootstrap or make sure its available in the path, then it might get you a bit further.

Sure, and then I'd get conflicts, so why wouldn't I prefer to use the m-c one? Why can't mach lint look for node in the right .mozbuild - because it clearly doesn't when asking to lint a file that's not a descendant of mozilla-central dir.

Comment 5

[Mark Banner (:standard8)]

(In reply to alta88 from comment #4)

It's an issue because the distro node_modules aren't necessarily anywhere near current with the m-c repo shipped version.

The distro node_modules aren't really anything to do with this - they are the global install of node/npm. In any case, you should be able to update those with npm install -g. Generally though, its best to install in a local scope to the directory as then you always get exactly the right versions and it then doesn't matter if different repos need different versions.

It makes no difference in either case whether the paths are absolute.

Ok, so this doesn't seem to be supported by mach lint. That command does some checking to test which files to lint and which linters to run as a result, I suspect it is seeing they're outside the directory and just ignoring them.

Then why, as I said, is ~/.mozbuild/gecko-strings being created, and by whom?

That's used for one of the linters. Try searching mozilla-central for gecko-strings and you'll soon get an idea.

Sure, and then I'd get conflicts, so why wouldn't I prefer to use the m-c one? Why can't mach lint look for node in the right .mozbuild - because it clearly doesn't when asking to lint a file that's not a descendant of mozilla-central dir.

I've just checked via code inspection and testing and mach lint is respecting the MOZBUILD_STATE_PATH.

If it isn't working for you, you'll need to debug why these aren't working:

• https://searchfox.org/mozilla-central/rev/e62c920f7f6463239c6634113f8a8351e263b936/tools/lint/eslint/__init__.py#61
• https://searchfox.org/mozilla-central/rev/0ffa9e372df56c95547fed9c3433ddec4fbf6f11/python/mozbuild/mozbuild/nodeutil.py#69

Comment 6

[Andrew Halberstadt [:ahal]]

Haven't read the discussions, but this request will break a very fundamental assumption about mach lint, namely that there is a root (topsrcdir in this case), and all files being linted live under it. I expect all sorts of things to not work if this assumption gets broken. The mozbuild issue you filed for is just an extension of this assumption, we use the state dir associated with topsrcdir. So what you are really asking for here is to be able lint files that live outside of topsrcdir.

I guess I would accept a patch if it is well crafted and doesn't pose any risk to the common use case, but this will require substantial effort. Therefore I'm inclined to WONTFIX. If you are really invested in this and want to give it a shot yourself, feel free to re-open.