Closed Bug 1341404 Opened 3 years ago Closed 3 years ago

add a --disable-optimize --enable-debug build to automation

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox54 wontfix, firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox54 --- wontfix
firefox55 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(3 files, 3 obsolete files)

(I guess this isn't really Build Config, more of a Taskcluster thing, but the person responsible will probably be a build person, so filing it here...)

Various patches can break --disable-optimize --enable-debug builds, which many developers use for their local builds.  We should be building those configurations in automation (but not testing them!) so we don't break people's workflows unexpectedly.
Duplicate of this bug: 1367751
I will try adding this next week unless somebody beats me to it.
Given how slow debug builds are on Mac, it would also be real nice to fix bug 1350582 - at least for Mac - at the same time. Any chance of that?
See Also: → 1350582
My intention was to do this for linux64 at a minimum; adding a linux64 build is quite cheap (AWS).

I think our Windows builds (at least in Taskcluster) run on virtualized hardware (AWS), so they're probably cheap as well.  If we do Mac, I *think* we would need to do cross-compiled Mac builds (which run on AWS) so as to not make our native Mac build problems worse than they already are.
Duplicate of this bug: 1350582
Attached patch add linux64 noopt debug builds (obsolete) — Splinter Review
A number of developers find it convenient to build with
--disable-optimize --enable-debug for an improved debugging experience.
We don't currently have a configuration in CI that ensures this
combination of options works, so various changes break builds with this
configuration every so often.  We should test such configurations to
ensure they build to provide a smooth experience for developers.

These are the Linux changes; separate patches for Mac and Windows builds will
follow.

Bikeshed to be painted: Should the treeherder symbol be something more
evocative than 'B'?
Attachment #8873470 - Flags: review?(dustin)
Attached patch add macosx64 noopt debug builds (obsolete) — Splinter Review
The Mac changes.  Cross builds only, which should be good enough for our
purposes here.
Attachment #8873471 - Flags: review?(dustin)
Attached patch add win32/64 noopt debug builds (obsolete) — Splinter Review
The Windows builds, in all their cut-and-paste glory.
Attachment #8873472 - Flags: review?(dustin)
These build OK on try (granted, using an older central base), with one small typo fix in the win32 config:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7e14585d36cceb4ac97180f9ac0780886c1bc288
Attachment #8873472 - Flags: review?(dustin) → review+
Attachment #8873471 - Flags: review?(dustin) → review+
Attachment #8873470 - Flags: review?(dustin) → review+
All r+'d just looking at the taskcluster/ci changes - mozharness and mozconfig are not something I can review.
A number of developers find it convenient to build with
--disable-optimize --enable-debug for an improved debugging experience.
We don't currently have a configuration in CI that ensures this
combination of options works, so various changes break builds with this
configuration every so often.  We should test such configurations to
ensure they build to provide a smooth experience for developers.

Reposting to ask for mshal's review.
Attachment #8873579 - Flags: review?(mshal)
Attachment #8873579 - Flags: review+
Attachment #8873470 - Attachment is obsolete: true
Reposting to ask for mshal's review.
Attachment #8873580 - Flags: review?(mshal)
Attachment #8873580 - Flags: review+
Attachment #8873471 - Attachment is obsolete: true
Reposting to ask for mshal's review.  Also includes the win32 typo fix
previously mentioned.
Attachment #8873581 - Flags: review?(mshal)
Attachment #8873581 - Flags: review+
Attachment #8873472 - Attachment is obsolete: true
Comment on attachment 8873579 [details] [diff] [review]
add linux64 noopt debug builds

>diff --git a/browser/config/mozconfigs/linux64/noopt-debug b/browser/config/mozconfigs/linux64/noopt-debug
>new file mode 100644
>index 0000000..f1a32b7
>--- /dev/null
>+++ b/browser/config/mozconfigs/linux64/noopt-debug
>@@ -0,0 +1,21 @@
>+# Developers often build with these options for a better debugging experience.
>+ac_add_options --enable-debug
>+ac_add_options --disable-optimize

Do you need to leave off --enable-dmd and --enable-verify-mar for this build to work? If they can be turned on, I think it would be easier to maintain if this file just did:

ac_add_options --disable-optimize
. "$topsrcdir/browser/config/mozconfigs/linux64/debug"

That way we don't have to keep it consistent with the base debug mozconfig.
Attachment #8873579 - Flags: review?(mshal) → review+
Comment on attachment 8873580 [details] [diff] [review]
add macosx64 noopt debug builds

Similar mozconfig question here.
Attachment #8873580 - Flags: review?(mshal) → review+
Comment on attachment 8873581 [details] [diff] [review]
add win32/64 noopt debug builds

And here too :)
Attachment #8873581 - Flags: review?(mshal) → review+
(In reply to Michael Shal [:mshal] from comment #14)
> >@@ -0,0 +1,21 @@
> >+# Developers often build with these options for a better debugging experience.
> >+ac_add_options --enable-debug
> >+ac_add_options --disable-optimize
> 
> Do you need to leave off --enable-dmd and --enable-verify-mar for this build
> to work? If they can be turned on, I think it would be easier to maintain if
> this file just did:
> 
> ac_add_options --disable-optimize
> . "$topsrcdir/browser/config/mozconfigs/linux64/debug"
> 
> That way we don't have to keep it consistent with the base debug mozconfig.

We could certainly do that.  I think my only concern would be if somebody added --enable-optimize or similar to the debug mozconfig we'd silently start testing something different.  I don't know how likely this is, of course, but it's a possibility, and people aren't likely to think too hard about checking for uses of the debug mozconfig versus common files from build/ or config/.  WDYT?
Flags: needinfo?(mshal)
(In reply to Nathan Froyd [:froydnj] from comment #17)
> We could certainly do that.  I think my only concern would be if somebody
> added --enable-optimize or similar to the debug mozconfig we'd silently
> start testing something different.  I don't know how likely this is, of
> course, but it's a possibility, and people aren't likely to think too hard
> about checking for uses of the debug mozconfig versus common files from
> build/ or config/.  WDYT?

Would there be any issues if we added --disable-optimize at the end instead?

. "$topsrcdir/browser/config/mozconfigs/linux64/debug"
ac_add_options --disable-optimize
Flags: needinfo?(mshal)
I think the only concern there would be people putting things into mozconfig.override or whatever the file that we include last and being surprised that an --enable-optimize there is being overriden.  But that sounds like an edge case, yeah?
Flags: needinfo?(mshal)
Yeah, I don't know why someone would want to build the non-optimization debug build but override the --disable-optimize flag.
Flags: needinfo?(mshal)
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6e538d5add9a
add linux64 noopt debug builds; r=dustin,mshal
https://hg.mozilla.org/integration/mozilla-inbound/rev/a33e62ddf282
add macosx64 noopt debug builds; r=dustin,mshal
https://hg.mozilla.org/integration/mozilla-inbound/rev/f890b5d57fe5
add win32/64 noopt debug builds; r=dustin,mshal
Why is "generate-build-stats" commented out in the mozharness configs? I recently did a lot of work to ensure we're collecting build metrics everywhere. I think we should do it here too.
(In reply to Gregory Szorc [:gps] from comment #22)
> Why is "generate-build-stats" commented out in the mozharness configs? I
> recently did a lot of work to ensure we're collecting build metrics
> everywhere. I think we should do it here too.

Reason #1 is because I believe it was commented out in the mozharness config I cargo-culted from.

But after thinking about it, I decided this makes sense.  After all, this is a smoketest build, and monitoring performance on it seems like a waste; we could collect the stats, sure, but I'm very doubtful that anybody's going to look at them, or they're going to drive decisions.

I guess the one thing they could be good for is tracking debug-only compilation time over time, and complaining to compiler developers if that metric gets worse.  But that would require looking at them, for which see above.  WDYT?  If you think it's worth it in some abstract sense, please file a new bug and assign it to me, and I'll fix it next week.
Flags: needinfo?(gps)
Depends on: 1370129
Blocks: 1370315
I filed bug 1370315.
Flags: needinfo?(gps)
54 RC build is released. Mark 54 won't fix.
Product: Core → Firefox Build System
Assignee: nobody → nfroyd
You need to log in before you can comment on or make changes to this bug.