Closed Bug 1426566 Opened 6 years ago Closed 6 years ago

Debug and DevEdition builds are going to permafail when Gecko 59 merges to Beta on 2018-01-11 due to test_compare_mozconfigs faliures

Categories

(Firefox Build System :: General, defect, P1)

Product:
Firefox Build System
Component:
General
Type:
defect

Tracking

(firefox-esr52 unaffected, firefox57 unaffected, firefox58 unaffected, firefox59blocking verified)

Status:
VERIFIED FIXED
Milestone:
mozilla59
Tracking Flags:
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- unaffected
firefox59 blocking verified

People

(Reporter: RyanVM, Assigned: gps)

References

Details

Attachments

(4 files, 1 obsolete file)

Bug 1426566 - Make -v an alias to --verbose for `mach python-test`;
59 bytes, text/x-review-board-request
froydnj
: review+
Details
Bug 1426566 - Remove wrapper to compare-mozconfigs;
59 bytes, text/x-review-board-request
froydnj
: review+
Details
Bug 1426566 - Remove --enable-profiling from nightly mozconfigs;
59 bytes, text/x-review-board-request
froydnj
: review+
Details
Bug 1426566 - Stop removing --enable-profiling during uplift;
59 bytes, text/x-review-board-request
froydnj
: review+
Details 
[Tracking Requested - why for this release]: Permafailing builds on the next merge day

Looks like it doesn't like the removal of the --enable-profiling line from the nightly mozconfigs which is done by our merge day automation.

https://treeherder.mozilla.org/logviewer.html#?job_id=152738647&repo=try

ERROR:__main__:extra line in nightly whitelist: ac_add_options --enable-profiling

Patch to reproduce with:
https://hg.mozilla.org/try/rev/d69a5517d881095f95b5599cc40342d57be49792

Tom Prince reminds me that I can just remove that entry from the whitelist as a workaround for my Try pushes, but I'm not sure what we want to do for a longer-term fix.
Flags: needinfo?(gps)
Tracking for 59, so that we'll remember to follow up in January before the merge. 
This one looks like a blocker for dev edition 59.
tracking-firefox59: ? → blocking
I think that changing the whitelist is the correct thing to do. The whitelist will be correctly indicating that, on the beta branch, we don't expect the setting of `--enable-profiling` to differ between the nightly and beta configs.
I'll tackle this, since I likely regressed it (and it is high priority).
Assignee: nobody → gps
Status: NEW → ASSIGNED
Flags: needinfo?(gps)
Priority: -- → P1
Comment on attachment 8940361 [details]
Bug 1426566 - Make -v an alias to --verbose for `mach python-test`;

https://reviewboard.mozilla.org/r/210628/#review216366
Attachment #8940361 - Flags: review+
Comment on attachment 8940362 [details]
Bug 1426566 - Remove wrapper to compare-mozconfigs;

https://reviewboard.mozilla.org/r/210630/#review216372

Much abstraction, such value.
Attachment #8940362 - Flags: review+
Comment on attachment 8940363 [details]
Bug 1426566 - Only check for nightly lines on trunk revisions;

https://reviewboard.mozilla.org/r/210632/#review216374

Fine by me.

Can we in some way reference https://searchfox.org/mozilla-central/source/build/moz.configure/init.configure#990, which is "authoritative".  Maybe we should just write these flags into milestone.txt so that we don't proliferate this terrible testing logic yet further?  Follow-up, of course.
Attachment #8940363 - Flags: review+
Attachment #8940361 - Flags: review?(core-build-config-reviews)
Attachment #8940362 - Flags: review?(core-build-config-reviews)
Attachment #8940363 - Flags: review?(core-build-config-reviews)
Comment on attachment 8940363 [details]
Bug 1426566 - Only check for nightly lines on trunk revisions;

https://reviewboard.mozilla.org/r/210632/#review216374

Yes. We're already importing `buildconfig` in this file. So we can look at subst to resolve the situation here. I updated the patch. Please re-review.
(In reply to Tom Prince [:tomprince] from comment #2)
> I think that changing the whitelist is the correct thing to do. The
> whitelist will be correctly indicating that, on the beta branch, we don't
> expect the setting of `--enable-profiling` to differ between the nightly and
> beta configs.

huh? --enable-profiling is *not* supposed to be set on beta.
Comment on attachment 8940363 [details]
Bug 1426566 - Only check for nightly lines on trunk revisions;

Looks good on Try!
Attachment #8940363 - Flags: feedback+
Comment on attachment 8940363 [details]
Bug 1426566 - Only check for nightly lines on trunk revisions;

https://reviewboard.mozilla.org/r/210632/#review216444

lgtm after updating the commit message.

::: commit-message-a1fd0:11
(Diff revision 3)
> +
> +The central_to_beta.py mozharness merge day config rewrites various
> +nightly mozconfigs. This rewriting causes entries in the whitelist
> +to not be present, causing compare-mozconfigs.py to fail.
> +
> +We work around this by only running the additional content checks

This is no longer correct.
Attachment #8940363 - Flags: review+
Attachment #8940363 - Flags: review?(core-build-config-reviews)
(In reply to Mike Hommey [back on Jan 9] [:glandium] from comment #13)
> (In reply to Tom Prince [:tomprince] from comment #2)
> > I think that changing the whitelist is the correct thing to do. The
> > whitelist will be correctly indicating that, on the beta branch, we don't
> > expect the setting of `--enable-profiling` to differ between the nightly and
> > beta configs.
> 
> huh? --enable-profiling is *not* supposed to be set on beta.

That is what I was saying. To be clear, what I was suggesting is:

On mozilla-central:
- `nightly` mozconfig has `--enable-profilling`
- `beta` mozconfig does not have `--enable-profilling`
- whitelist indicates that `--enable-pofiling` is something that nightly-mozconfig should have that isn't also in beta-mozconfig

On mozilla-beta:
- `nightly` mozconfig does not have `--enable-profiling`
- `beta` mozconfig does not have `--enable-profiling`
- (*) whitelist does not indicate that `--enable-profiling` be something that nightly-mozconfig should have that is in't also in beta-mozconfig

This differs from the current situation by changing to (*). gps' solution instead just doesn't check the whitelist sometimes.
Comment on attachment 8940363 [details]
Bug 1426566 - Only check for nightly lines on trunk revisions;

https://reviewboard.mozilla.org/r/210632/#review216446

::: build/compare-mozconfig/compare-mozconfigs.py:139
(Diff revisions 1 - 3)
>          # present. But because we rewrite mozconfigs as part of the
> -        # central_to_beta.py merge day script, we only do this on trunk.
> +        # central_to_beta.py merge day script, we only do this on nightly
> +        # builds.
>          # FUTURE Remove conditional execution once bug 1372697 addresses
>          # rewriting of mozconfigs.
> -        if is_trunk:
> +        if is_nightly:

It [looks](https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&selectedJob=154409765) like there are nightlies built from mozilla-beta, so I think there will still be failures from this there.
Note that for --enable-profiling specifically, the build system has the right defaults depending on the milestone (https://dxr.mozilla.org/mozilla-central/source/js/moz.configure#226-232), so we should just remove all --enable-profiling lines from mozconfigs. We should do that more generally, btw.
IOW, if the whole purpose of this patch series is to deal with --enable-profiling only, I think it would be best to just remove --enable-profiling and not add exceptions to the whitelist.
Attachment #8940363 - Attachment is obsolete: true
Comment on attachment 8941173 [details]
Bug 1426566 - Remove --enable-profiling from nightly mozconfigs;

https://reviewboard.mozilla.org/r/211436/#review217228
Attachment #8941173 - Flags: review+
Comment on attachment 8941174 [details]
Bug 1426566 - Stop removing --enable-profiling during uplift;

https://reviewboard.mozilla.org/r/211438/#review217230
Attachment #8941174 - Flags: review+
Comment on attachment 8940361 [details]
Bug 1426566 - Make -v an alias to --verbose for `mach python-test`;

https://reviewboard.mozilla.org/r/210628/#review217232
Attachment #8940361 - Flags: review+
Comment on attachment 8940362 [details]
Bug 1426566 - Remove wrapper to compare-mozconfigs;

https://reviewboard.mozilla.org/r/210630/#review217234

I am going to assume that the code in compare-mozconfigs-wrapper.py was appropriately moved over.
Attachment #8940362 - Flags: review+
Attachment #8940361 - Flags: review?(core-build-config-reviews)
Attachment #8940362 - Flags: review?(core-build-config-reviews)
Attachment #8941173 - Flags: review?(core-build-config-reviews)
Attachment #8941174 - Flags: review?(core-build-config-reviews)
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a8da2ba97f8f
Make -v an alias to --verbose for `mach python-test`; r=froydnj,nalexander
https://hg.mozilla.org/integration/autoland/rev/90a62bca5999
Remove wrapper to compare-mozconfigs; r=froydnj,nalexander
https://hg.mozilla.org/integration/autoland/rev/9e220db2bf10
Remove --enable-profiling from nightly mozconfigs; r=froydnj
https://hg.mozilla.org/integration/autoland/rev/3e88948848f8
Stop removing --enable-profiling during uplift; r=froydnj
Status: RESOLVED → VERIFIED
status-firefox59: fixed → verified
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: