1464381 - KeyError: u"'NIGHTLY_BUILD'" from bug 1464128 when Gecko 62 merges to Beta on 2018-06-14

Status: VERIFIED FIXED

(Firefox Build System :: General, defect)

Description

[Bogdan Tara[:bogdan_tara | bogdant]]

[Tracking Requested - why for this release]:
Central as Beta simulation  - https://treeherder.mozilla.org/#/jobs?repo=try&revision=0328fc58c863d252d26b33c2388a0fdb89329901&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable&selectedJob=180220254

Caused from bug 1464128. 

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=180219181&repo=try&lineNumber=32411

Comment 1

[Nick Alexander :nalexander [he/him]]

I'll definitely address this, but I want to figure out why this isn't doing the right thing before changing anything.  It makes no sense:

NIGHTLY_BUILD is set unconditionally at https://searchfox.org/mozilla-central/rev/ce86c6c0472d5021ef693cf99abaaa0644c89e55/build/moz.configure/init.configure#1043

but we're failing to find it in buildconfig.substs at

https://searchfox.org/mozilla-central/rev/ce86c6c0472d5021ef693cf99abaaa0644c89e55/browser/locales/generate_bookmarks.py#32

That doesn't make any sense to me.  I can, of course, `.get(..., False)` or whatever, but I want to understand why buildconfig.substs appears to be lying to me.

Update from dmajor: apparently this is _not_ set unconditionally.

Comment 2

[Nick Alexander :nalexander [he/him]]

OK, this really does need the `.get(..., False)` work-around.  I'll get to it.

Comment 3

[Nick Alexander :nalexander [he/him]]

Attachment: Bug 1464381 - Fix check for NIGHTLY_BUILD.

This was simply an oversight in the implementation of Bug 1464128.
What's happening is that `set_config` in `moz.configure` is not
unconditional, and NIGHTLY_BUILD is set in local builds and in B and N
builds in automation, so there was no test of the other case, which
promptly fails.  This re-uses a pattern successful in mobile/android
for setting defines.

Review commit: https://reviewboard.mozilla.org/r/246970/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/246970/

Comment 4

[Mike Hommey [:glandium]]

I wonder if we shouldn't do the more general thing of making buildconfig.substs essentially work like moz.build CONFIG, and never throw KeyError.

Comment 5

[Nick Alexander :nalexander [he/him]]

(In reply to Mike Hommey [:glandium] from comment #4)
> I wonder if we shouldn't do the more general thing of making
> buildconfig.substs essentially work like moz.build CONFIG, and never throw
> KeyError.

I think we should, but not in this ticket -- it's follow-up :)

Comment 6

[Kim Moir [:kmoir] ET]

Triaging, assigning to nalexander since he wrote the patches for this bug.

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 8980800 [details]
Bug 1464381 - Fix check for NIGHTLY_BUILD.

https://reviewboard.mozilla.org/r/246970/#review253694

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/eaefe57724c7
Fix check for NIGHTLY_BUILD. r=gps

Comment 9

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/eaefe57724c7

Comment 10

[Raul Gurzau (:RaulG)]

We have verified that this issue is fixed with today's Beta Simulation, link: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ed376ac1d1c4617b98d720c832dded991ef6bd61&selectedJob=180858976

Comment 11

[Nick Alexander :nalexander [he/him]]

(In reply to Raul Gurzau (:RaulGurzau) from comment #10)
> We have verified that this issue is fixed with today's Beta Simulation,
> link:
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=ed376ac1d1c4617b98d720c832dded991ef6bd61&selectedJob=1
> 80858976

Thanks Raul.  Sorry for the run-around!

Comment 12

[Liz Henry (:lizzard)]

Belatedly tracking, so that if this reopens it'll show up on our queries.