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)

defect

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
Blocks: clang-format
Actually, the command "./mach clang-format -p config/" is fine but "./mach clang-format -p ./config/" is not
Priority: -- → P2
Assignee: nobody → sledru
Attachment #8923493 - Flags: review?(nika)
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+
(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
(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.
Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/e1b4f786f584 /mach clang-format: Better handling of relative paths r=mystor=mystor
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Flags: needinfo?(sledru)
Product: Core → Firefox Build System
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: