Open Bug 774381 Opened 8 years ago Updated 2 years ago

Add moz.build style checking tool

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(Not tracked)

People

(Reporter: gps, Unassigned)

References

(Blocks 1 open bug)

Details

We are removing all rules and other make-isms from Makefile.in's so they are easier to parse. See bug 769378 and others in the bug 774049 umbrella.

We agree that we should have some kind of tool people can use to verify that Makefile.in's conform to the conventions.

We will likely eventually enforce compliance. That may or may not be within scope for this bug. When we implement that, I think we should go with a progressive approach. We'll maintain a whitelist of existing Makefile.in that aren't in compliance. If an existing file goes out of compliance, that will be fatal. If an existing file goes into compliance, it is removed from the whitelist. Once all files are in compliance, we remove the whitelist.

Assigning to me because I already have some code around this. I'm sure most of the bikeshedding will be about the exact rules we choose to enforce.
Makefile.in are dead. Long live moz.build files!

Let's adapt this for moz.build files. What can we do here? 4 space indent is the obvious one. Anything else? Maybe detect assignments of strings to lists? e.g. DIRS += 'foo' (which results in ['f', 'o', 'o']).
No longer blocks: 774049
Summary: Add Makefile.in style checking tool → Add moz.build style checking tool
(In reply to Gregory Szorc [:gps] from comment #1)
> Maybe detect assignments of strings to
> lists? e.g. DIRS += 'foo' (which results in ['f', 'o', 'o']).

I think this one should be detected during evaluation, fwiw.
Agreed. Obvious errors should just be evaluation errors. The tool proposed in this bug should just be a style checker.
Another would be assigning things in conditionals that don't need to be. e.g.

if CONFIG['ENABLE_TESTS']:
    DIRS += ['tests']

should warn because this should be:

TEST_DIRS += ['tests']
I'm not actively working on this.
Assignee: gps → nobody
I have some patches written in bug that create some Python tests that run as part of |make check| which iterate over every moz.build file and ensure we can evaluate them. We could expand this test to start covering things like style checking.
(In reply to Gregory Szorc [:gps] from comment #6)
> I have some patches written in bug that create some Python tests...

Which bug?
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.