Closed
Bug 1467234
Opened 6 years ago
Closed 6 years ago
Linting a directory behaves differently from a file (at least for cpp-virtual-final)
Categories
(Developer Infrastructure :: Lint and Formatting, enhancement)
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1494069
People
(Reporter: Mardak, Assigned: ahal)
References
Details
Attachments
(2 obsolete files)
In bug 1440421, we ended up adding a cpp-virtual-final.yml exclude of `- '**/node_modules'` because `- browser/extensions/activity-stream/node_modules` didn't seem to work.
With the other bug (which mirrors activity-stream's git repository into mozilla-central), one could develop activity stream directly from mozilla-central by first running `npm install` from `browser/extensions/activity-stream`
Turns out excluding the full path was actually working, but ./mach lint <parent directory> wasn't working.
Below are the results of setting various exclude values in tools/lint/cpp-virtual-final.yml then running on the directory vs file:
$ ./mach lint browser/extensions/activity-stream/
$ ./mach lint browser/extensions/activity-stream/node_modules/nan/nan.h
```
exclude: - browser/extensions/activity-stream/node_modules
✖ 1 problem (0 errors, 1 warning)
✖ 0 problems (0 errors, 0 warnings)
```
```
exclude: - browser/extensions/activity-stream
✖ 0 problems (0 errors, 0 warnings)
✖ 0 problems (0 errors, 0 warnings)
```
```
exclude: - node_modules
✖ 0 problems (0 errors, 0 warnings)
✖ 1 problem (0 errors, 1 warning)
```
```
exclude: - '**/node_modules'
✖ 0 problems (0 errors, 0 warnings)
✖ 0 problems (0 errors, 0 warnings)
```
Assignee | ||
Comment 1•6 years ago
|
||
Thanks for filing, that investigation is really useful! The path filtering logic is too complicated right now.
Assignee | ||
Comment 2•6 years ago
|
||
I actually ran into this as well, and came up with a fix.
However I have several major(ish) mozlint changes in the process of landing. I'd like to get those finished and shore up some tests before landing the fix for this. But hopefully I'll have this done soon.
Assignee: nobody → ahal
Status: NEW → ASSIGNED
Assignee | ||
Comment 3•6 years ago
|
||
This will make it easier to add new test cases in the future. It'll
also make it easier to print debug, as only the output for each
specific test case will be printed on failure.
Assignee | ||
Comment 4•6 years ago
|
||
Currently pathutils.filterpaths computes some extra paths that the underlying linter
should try to exclude if it can. It sets these on lintargs['exclude'], and relies on
the fact that it was passed by reference to work.
However, it seems like pytest does some magic under the hood that prevents this value
from being propagated back. It will also fail if 'filterpaths' was invoked in a
subprocess.
It's much simpler and cleaner to pass this value back directly, rather than rely on a
reference.
Depends on D5862
Updated•6 years ago
|
Attachment #9009108 -
Attachment is obsolete: true
Updated•6 years ago
|
Attachment #9009109 -
Attachment is obsolete: true
Assignee | ||
Comment 5•6 years ago
|
||
This is going to be fixed by bug 1494069
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Updated•6 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
•