Closed
Bug 1411004
Opened 7 years ago
Closed 7 years ago
mach clang-format doesn't respect .clang-format-ignore
Categories
(Developer Infrastructure :: Source Code Analysis, defect, P2)
Developer Infrastructure
Source Code Analysis
Tracking
(firefox58 fixed)
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: Sylvestre, Assigned: Sylvestre)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
This is a recent regression, not sure what is going on.
As example:
$ ./mach clang-format -p config/
config/gcc-stl-wrapper.template.h should not be touched
Assignee | ||
Updated•7 years ago
|
Blocks: clang-format
Assignee | ||
Comment 1•7 years ago
|
||
Actually, the command "./mach clang-format -p config/" is fine
but "./mach clang-format -p ./config/" is not
Updated•7 years ago
|
Priority: -- → P2
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → sledru
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8923493 -
Flags: review?(nika)
Assignee | ||
Comment 3•7 years ago
|
||
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8923493 [details]
Bug 1411004 - ./mach clang-format: Better handling of relative paths =mystor
https://reviewboard.mozilla.org/r/194630/#review200032
r=me, but if this doesn't work when running ./mach clang-format outside of topsrcdir I'd love a follow up to fix that :-).
::: tools/mach_commands.py:308
(Diff revision 1)
> for line in open(pathToThirdparty):
> # Remove comments and empty lines
> if line.startswith('#') or len(line.strip()) == 0:
> continue
> - ignored_dir.append(line.rstrip())
> + # The regexp is to make sure we are managing relative paths
> + ignored_dir.append("^[\./]*" + line.rstrip())
So, this doesn't perfectly manage relative paths, it only handles ./foo/bar, foo/bar. ././foo/bar, right?
I'm worried a bit about trying to run clang-format from a directory other than $topsrcdir. Do these path whitelists just stop working if we do that?
Attachment #8923493 -
Flags: review?(nika) → review+
Assignee | ||
Comment 5•7 years ago
|
||
(In reply to Nika Layzell [:mystor] from comment #4)
> So, this doesn't perfectly manage relative paths, it only handles ./foo/bar,
> foo/bar. ././foo/bar, right?
It doesn't indeed.
We discussed about that with Calixte but felt that it wasn't a comment pattern. Are we wrong?
Thanks for the review
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/31f259ee387b
/mach clang-format: Better handling of relative paths r=mystor=mystor
Comment 7•7 years ago
|
||
(In reply to Sylvestre Ledru [:sylvestre] from comment #5)
> It doesn't indeed.
> We discussed about that with Calixte but felt that it wasn't a comment
> pattern. Are we wrong?
>
>
> Thanks for the review
I don't think it's super common to run outside of topsrcdir, but it might be good to at least fail gracefully in that situation.
Comment 8•7 years ago
|
||
Backed out for failing flake8 at tools/mach_commands.py:307:14 | indentation is not a multiple of four:
https://hg.mozilla.org/integration/autoland/rev/19f207add6c8ba25e25273e807a9fff22fa41b84
Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=31f259ee387b61199ae3ea2f92d68fea45d88db2&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=runnable&filter-resultStatus=retry&filter-resultStatus=usercancel
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=141073039&repo=autoland
Flags: needinfo?(sledru)
Comment hidden (mozreview-request) |
Comment 10•7 years ago
|
||
Pushed by sledru@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e1b4f786f584
/mach clang-format: Better handling of relative paths r=mystor=mystor
Comment 11•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Assignee | ||
Updated•7 years ago
|
Flags: needinfo?(sledru)
Assignee | ||
Updated•7 years ago
|
Updated•7 years ago
|
Product: Core → Firefox Build System
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
•