Closed
Bug 1434982
Opened 6 years ago
Closed 6 years ago
no-undef errors on ChromeUtils.import of module into global scope
Categories
(Developer Infrastructure :: Lint and Formatting, defect)
Tracking
(Not tracked)
RESOLVED
WORKSFORME
People
(Reporter: ato, Unassigned)
References
Details
Following the integration of https://bugzilla.mozilla.org/show_bug.cgi?id=1431533 in central, I get 443 no-undef errors when linting testing/marionette: > % ./mach lint -funix testing/marionette/ | wc -l > 443 Examples: > % ./mach lint -funix testing/marionette > testing/marionette/accessibility.js:13:16: no-undef error: 'Log' is not defined. > testing/marionette/accessibility.js:23:1: no-undef error: 'XPCOMUtils' is not defined. > testing/marionette/accessibility.js:176:11: no-undef error: 'Services' is not defined. > testing/marionette/accessibility.js:185:7: no-undef error: 'Services' is not defined. > testing/marionette/action.js:362:5: no-undef error: 'assert' is not defined. They arise from code such as this: > ChromeUtils.import("resource://gre/modules/Log.jsm"); > const logger = Log.repository.getLogger("Marionette");
Reporter | ||
Comment 1•6 years ago
|
||
It’s unclear to me how the ES job on TC passes when "./mach lint" locally does not?
Depends on: 1431533
Flags: needinfo?(standard8)
Reporter | ||
Comment 2•6 years ago
|
||
I see the same problem in browser/base/content and other modules. Perhaps https://bugzilla.mozilla.org/show_bug.cgi?id=1431533 fixed the ES job running on TC, but it appears "./mach lint" picks up its rules in a slightly different way?
Flags: needinfo?(kmaglione+bmo)
Comment 3•6 years ago
|
||
This is working fine for me locally. I'm wondering if you repository hasn't updated tools/lint/eslint/eslint-plugin-mozilla somehow, or maybe you've got a hard copy of it in node_modules? Maybe try nuking topsrcdir/node_modules and running `./mach eslint --setup` then lint again.
Flags: needinfo?(standard8)
Comment 4•6 years ago
|
||
As Mark said, it sounds like `mach lint` is using an outdated version of the plugin. It might be worth filing a bug if that command doesn't handle updates automatically, the way that `mach eslint` does.
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: needinfo?(kmaglione+bmo)
Resolution: --- → WORKSFORME
Comment 5•6 years ago
|
||
I should clarify a bit what is likely going on here: - package.json lists file references to eslint-plugin-mozilla and eslint-plugin-spidermonkey-js, i.e. the in-tree versions. - Doing an `npm install` (which is basically what `./mach eslint --setup` does) should install into node_modules all the normal modules, but symlink the file references (eslint-plugin-mozilla & eslint-plugin-spidermonkey-js) - Then, when you update the tree, the in-tree versions automatically update & due to the symlinks, the command gets the right version. Note that `./mach lint` basically calls through to `./mach eslint` for eslint - they're pretty much the same. I believe all supported versions of npm (from around v3 onwards iirc) support symlinking, obviously some older Window disk formats don't, but I think most newer ones should be fine.
Reporter | ||
Comment 6•6 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #3) > I'm wondering if you repository hasn't updated > tools/lint/eslint/eslint-plugin-mozilla somehow, or maybe you've > got a hard copy of it in node_modules? > > Maybe try nuking topsrcdir/node_modules and running `./mach eslint > --setup` then lint again. Nuking $(topsrcdir)/node_modules did the trick. The npm version I have installed (on Linux, which supports symlinking) is 3.10.10. Obviously I didn’t check whether eslint-plugin-mozilla was symlinked before I nuked node_modules, but if there are systems that don’t support this (pre 3.x npm and Windows), shouldn’t the build system have to take care of updating the files automatically?
Flags: needinfo?(standard8)
Comment 7•6 years ago
|
||
We currently have a min version for node of 6.9.1, which shipped with npm 3.10.8 by default. Getting an earlier version would likely be difficult, so I'm not too worried about that case (I think npm 2 might have supported symlinks as well, but can't remember atm). For Windows, we used to have the support but removed it, partially due to the added complexity. I haven't heard any complaints, so I'm generally assuming most of our developers are on modernish systems.
Flags: needinfo?(standard8)
Updated•6 years ago
|
Product: Testing → Firefox Build System
Updated•5 years ago
|
Version: Version 3 → 3 Branch
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•