Make spidermonkey builds use debug by default

RESOLVED FIXED

Status

P3
normal
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: paul.biggar, Assigned: catlee)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
Created attachment 515716 [details] [diff] [review]
Change --disable-debug to --enable-debug

A few build errors crept into the nomethodjit builds, in debug code. I guess there is more value in --enable-debug than --disable-debug, so this patch just changes everything to debug.
Attachment #515716 - Flags: review?(catlee)
How soon do you want to land this ? catlee is out until Friday.
(Reporter)

Comment 2

8 years ago
(In reply to comment #1)
> How soon do you want to land this ? catlee is out until Friday.

I'm in no rush, Friday is fine, next week is fine.

But feel free to take it if you want :)
(Assignee)

Comment 3

8 years ago
Comment on attachment 515716 [details] [diff] [review]
Change --disable-debug to --enable-debug

Can this be landed at any time?
Attachment #515716 - Flags: review?(catlee) → review+
Might be a touch less confusing if it also came with a change in the build name to include "debug" so tbpl won't be showing builds as opt while they're failing in debug-only code.
(Reporter)

Comment 5

8 years ago
(In reply to comment #3)
> Can this be landed at any time?

Yes. It will go red though, so it might be worth giving a heads up to philor or the pusher. The fix for the fallout is at bug 637437.

(In reply to comment #4)
> Might be a touch less confusing if it also came with a change in the build name
> to include "debug" so tbpl won't be showing builds as opt while they're failing
> in debug-only code.

I agree. I looked through the code from bug 609413, but I can't see where the debug/opt name decision is made.
(Assignee)

Comment 6

8 years ago
(In reply to comment #5)
> (In reply to comment #3)
> > Can this be landed at any time?
> 
> Yes. It will go red though, so it might be worth giving a heads up to philor or
> the pusher. The fix for the fallout is at bug 637437.
> 
> (In reply to comment #4)
> > Might be a touch less confusing if it also came with a change in the build name
> > to include "debug" so tbpl won't be showing builds as opt while they're failing
> > in debug-only code.
> 
> I agree. I looked through the code from bug 609413, but I can't see where the
> debug/opt name decision is made.

I think if we change the platform names in config.py, the builds/tinderbox columns will be named e.g. tracemonkey_win32-debug_nomethodjit.

Might need a TBPL patch as well?
Shouldn't need any tbpl patching, http://hg.mozilla.org/users/mstange_themasta.com/tinderboxpushlog/file/1ea98f52fac3/js/Data.js#l459 does debug-or-not and then the bit below digs out the name.
(Reporter)

Comment 8

8 years ago
Created attachment 516339 [details] [diff] [review]
REname platforms

I think this is what you mean?
Attachment #516339 - Flags: review?(catlee)
(Assignee)

Comment 9

8 years ago
Comment on attachment 516339 [details] [diff] [review]
REname platforms

missing patch?
(Assignee)

Updated

8 years ago
Assignee: nobody → catlee
Priority: -- → P3
(Reporter)

Comment 10

8 years ago
Created attachment 516343 [details] [diff] [review]
Rename platforms (wthi actual patch)

My bad. Attached properly.
Attachment #516339 - Attachment is obsolete: true
Attachment #516339 - Flags: review?(catlee)
Attachment #516343 - Flags: review?(catlee)
(Assignee)

Comment 11

8 years ago
Created attachment 516657 [details] [diff] [review]
Use base platform names for slave lookups

If we use '-debug' platforms, the SLAVES[platform] lookup fails in generateSpiderMonkeyObjects. This patch strips off the '-debug' for the purposes of getting the list of slaves for a platform.
Attachment #516657 - Flags: review?(bhearsum)
(Assignee)

Comment 12

8 years ago
Comment on attachment 516343 [details] [diff] [review]
Rename platforms (wthi actual patch)

Looks good, thanks!
Attachment #516343 - Flags: review?(catlee) → review+
Attachment #516657 - Flags: review?(bhearsum) → review+
(Assignee)

Comment 13

8 years ago
Comment on attachment 516657 [details] [diff] [review]
Use base platform names for slave lookups

http://hg.mozilla.org/build/buildbotcustom/rev/9dc090ae6021
Attachment #516657 - Flags: checked-in+
(Assignee)

Comment 14

8 years ago
Comment on attachment 516343 [details] [diff] [review]
Rename platforms (wthi actual patch)

http://hg.mozilla.org/build/buildbot-configs/rev/be6c75caa6f8

Should be deployed tomorrow morning
Attachment #516343 - Flags: checked-in+
(Assignee)

Comment 15

8 years ago
Comment on attachment 515716 [details] [diff] [review]
Change --disable-debug to --enable-debug

http://hg.mozilla.org/build/tools/rev/624f28867749
Attachment #515716 - Flags: checked-in+
(Assignee)

Comment 16

8 years ago
Should be all done here.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.