Closed Bug 1419182 Opened 3 years ago Closed 3 years ago

compare-mozconfigs cleanup

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set
normal

Tracking

(firefox59 fixed)

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: gps, Assigned: gps)

References

(Blocks 1 open bug)

Details

Attachments

(12 files)

59 bytes, text/x-review-board-request
mshal
: review+
Details
59 bytes, text/x-review-board-request
mshal
: review+
Details
59 bytes, text/x-review-board-request
mshal
: review+
Details
59 bytes, text/x-review-board-request
mshal
: review+
Details
59 bytes, text/x-review-board-request
mshal
: review+
Details
59 bytes, text/x-review-board-request
mshal
: review+
Details
59 bytes, text/x-review-board-request
mshal
: review+
Details
59 bytes, text/x-review-board-request
mshal
: review+
Details
59 bytes, text/x-review-board-request
mshal
: review+
Details
59 bytes, text/x-review-board-request
mshal
: review+
Details
59 bytes, text/x-review-board-request
mshal
: review+
Details
59 bytes, text/x-review-board-request
mshal
: review+
Details
I'm tired of finding dead code in the mozconfig whitelist. I'm going to clean this mess up once and for all.
Assignee: nobody → gps
Status: NEW → ASSIGNED
Comment on attachment 8930274 [details]
Bug 1419182 - Remove unused required keys feature;

https://reviewboard.mozilla.org/r/201400/#review207058

It looks like this readConfig() method is some copy-pasta from build/release/info.py, hence the unused cruft.
Attachment #8930274 - Flags: review+
Attachment #8930274 - Flags: review?(core-build-config-reviews)
Attachment #8930275 - Flags: review?(core-build-config-reviews)
Comment on attachment 8930276 [details]
Bug 1419182 - Don't use mutable default argument value;

https://reviewboard.mozilla.org/r/201404/#review207064
Attachment #8930276 - Flags: review+
Attachment #8930276 - Flags: review?(core-build-config-reviews)
Comment on attachment 8930277 [details]
Bug 1419182 - Remove functionality for downloading mozconfigs;

https://reviewboard.mozilla.org/r/201406/#review207066
Attachment #8930277 - Flags: review+
Attachment #8930277 - Flags: review?(core-build-config-reviews)
Attachment #8930278 - Flags: review?(core-build-config-reviews)
Comment on attachment 8930279 [details]
Bug 1419182 - Pass topsrcdir into compare-mozconfigs.py;

https://reviewboard.mozilla.org/r/201410/#review207070

::: commit-message-5cd60:7
(Diff revision 1)
> +
> +We currently have the logic for compare-mozconfigs spread across
> +2 Python files. I'd like to move the logic into 1 file so we
> +can do nicer things more easily. In preparation for this, change
> +compare-mozconfigs.py to receive an argument that is the path to
> +the topsrcdir. Also, switch to argparse because it is more modern.

What, splitting the argparse switch out into a separate commit would make the patchset too large? :)
Attachment #8930279 - Flags: review+
Attachment #8930279 - Flags: review?(core-build-config-reviews)
Comment on attachment 8930280 [details]
Bug 1419182 - Consolidate mozconfig comparison logic into compare-mozconfigs.py;

https://reviewboard.mozilla.org/r/201412/#review207072

::: build/compare-mozconfig/compare-mozconfigs-wrapper.py:23
(Diff revision 1)
> -        for platform in PLATFORMS:
> -            log.info('Comparing platform %s' % platform)
> -            python_exe = substs['PYTHON']
> -            topsrcdir = substs['top_srcdir']
> +        topsrcdir = substs['top_srcdir']
>  
> -            # construct paths and args for compare-mozconfig
> +        ret = subprocess.call([

Does the wrapper file actually serve any purpose now? It seems like the compare-mozconfigs.py script could just be a python test.
Attachment #8930280 - Flags: review+
Attachment #8930280 - Flags: review?(core-build-config-reviews)
Comment on attachment 8930281 [details]
Bug 1419182 - Remove MOZ_MAKE_FLAGS references from mozconfigs whitelists;

https://reviewboard.mozilla.org/r/201414/#review207074
Attachment #8930281 - Flags: review+
Attachment #8930281 - Flags: review?(core-build-config-reviews)
Comment on attachment 8930282 [details]
Bug 1419182 - Remove last references to PROFILE_GEN_SCRIPT;

https://reviewboard.mozilla.org/r/201416/#review207076
Attachment #8930282 - Flags: review+
Attachment #8930282 - Flags: review?(core-build-config-reviews)
Comment on attachment 8930283 [details]
Bug 1419182 - Remove last reference to CLIENT_PY_ARGS;

https://reviewboard.mozilla.org/r/201418/#review207078
Attachment #8930283 - Flags: review+
Attachment #8930283 - Flags: review?(core-build-config-reviews)
Comment on attachment 8930284 [details]
Bug 1419182 - Remove unreferenced entries in nightly whitelist;

https://reviewboard.mozilla.org/r/201420/#review207080
Attachment #8930284 - Flags: review+
Attachment #8930284 - Flags: review?(core-build-config-reviews)
Comment on attachment 8930285 [details]
Bug 1419182 - Look for extra lines in nightly whitelist;

https://reviewboard.mozilla.org/r/201422/#review207082
Attachment #8930285 - Flags: review+
Attachment #8930285 - Flags: review?(core-build-config-reviews)
Comment on attachment 8930280 [details]
Bug 1419182 - Consolidate mozconfig comparison logic into compare-mozconfigs.py;

https://reviewboard.mozilla.org/r/201412/#review207072

> Does the wrapper file actually serve any purpose now? It seems like the compare-mozconfigs.py script could just be a python test.

You are correct: it can likely be eliminated. This was marginally beyond my scope bloat tolerance for this series. The next person touching this code can worry about it :)
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/23c4884d5316
Remove unused required keys feature; r=mshal
https://hg.mozilla.org/integration/autoland/rev/124bb2bb9b39
Hard code whitelist key; r=mshal
https://hg.mozilla.org/integration/autoland/rev/01ee358a08bf
Don't use mutable default argument value; r=mshal
https://hg.mozilla.org/integration/autoland/rev/3fe392d9fd66
Remove functionality for downloading mozconfigs; r=mshal
https://hg.mozilla.org/integration/autoland/rev/a7a3f0cdc871
Clean up value checking; r=mshal
https://hg.mozilla.org/integration/autoland/rev/489860a7e03a
Pass topsrcdir into compare-mozconfigs.py; r=mshal
https://hg.mozilla.org/integration/autoland/rev/a1040ffa24e2
Consolidate mozconfig comparison logic into compare-mozconfigs.py; r=mshal
https://hg.mozilla.org/integration/autoland/rev/cd573160f87c
Remove MOZ_MAKE_FLAGS references from mozconfigs whitelists; r=mshal
https://hg.mozilla.org/integration/autoland/rev/16aadd824582
Remove last references to PROFILE_GEN_SCRIPT; r=mshal
https://hg.mozilla.org/integration/autoland/rev/728865ad47ef
Remove last reference to CLIENT_PY_ARGS; r=mshal
https://hg.mozilla.org/integration/autoland/rev/5db28a857fdc
Remove unreferenced entries in nightly whitelist; r=mshal
https://hg.mozilla.org/integration/autoland/rev/3c9fb7f0ff92
Look for extra lines in nightly whitelist; r=mshal
Depends on: 1426566
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.