Closed Bug 1011352 Opened 6 years ago Closed 6 years ago

Add a MOZ_AUTOMATION environment to all builds

Categories

(Release Engineering :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: glandium, Assigned: glandium)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Attached patch Mike Hommey <mh+mozilla@glandium.org> (obsolete) β€” β€” Splinter Review
The rationale for adding a variable instead of renaming as per bug 946691 is that many things depend on TINDERBOX_OUTPUT being set, and they can't be switched to a new variable without that variable already existing. So TINDERBOX_OUTPUT would need to be phased out, riding the train.

Note that factoring TINDERBOX_OUTPUT has the side effect of setting it where it was supposed to be but actually wasn't. MOZ_CRASHREPORTER_NO_REPORT can't hurt to be set on the few things where it's not set currently, as far as I can tell. I'm sure there are other environment variables that can be factored (MOZ_OBJDIR?), but I'll leave that for another day.
Attachment #8423554 - Flags: review?(bhearsum)
Assignee: nobody → mh+mozilla
Status: NEW → ASSIGNED
Comment on attachment 8423554 [details] [diff] [review]
Mike Hommey <mh+mozilla@glandium.org>

Review of attachment 8423554 [details] [diff] [review]:
-----------------------------------------------------------------

I really don't like the addition of more loops here - the ones we have make these configs difficult to read already, adding more of them (especially a new class of them) makes things worse. This is an area where I'd rather err on the side of repetition and verbosity. Please add these to the existing platforms instead.

Also, do the test configs need these set as well?

::: calendar/config.py
@@ +85,1 @@
>        'dom-inspector':          'domi',

This isn't one of our configs - you probably shouldn't touch it.

::: seamonkey/config.py
@@ +116,1 @@
>      'enabled_products': ['seamonkey'],

Same with this one.
Attachment #8423554 - Flags: review?(bhearsum) → review-
Comment on attachment 8423554 [details] [diff] [review]
Mike Hommey <mh+mozilla@glandium.org>

The very fact that TINDERBOX_OUTPUT is not set on all platforms when it was meant to is proof that the repetition model doesn't work. I'm seriously sick of this whole thing. And it makes any global changes a PITA to implement. Like, for example, if we ever need to trigger SCCACHE_DISABLE globally for some reason, it's going to be very painful.

Please reconsider.
Attachment #8423554 - Flags: review- → review?(bhearsum)
Comment on attachment 8423554 [details] [diff] [review]
Mike Hommey <mh+mozilla@glandium.org>

r=meh - configs are fucked up already anyways
Attachment #8423554 - Flags: review?(bhearsum) → review+
Comment on attachment 8423554 [details] [diff] [review]
Mike Hommey <mh+mozilla@glandium.org>

Review of attachment 8423554 [details] [diff] [review]:
-----------------------------------------------------------------

Adding Callek for seamonkey and Fallen for calendar.
Attachment #8423554 - Flags: review?(philipp)
Attachment #8423554 - Flags: review?(bugspam.Callek)
(In reply to Ben Hearsum [:bhearsum] from comment #2)
> Also, do the test configs need these set as well?

I don't know. Do you think we need a MOZ_AUTOMATION there?
Comment on attachment 8423554 [details] [diff] [review]
Mike Hommey <mh+mozilla@glandium.org>

You don't need to change the calendar/ configs, we should probably be removing them in a different bug anyway. Calendar now gets built fully on the Thunderbird configs. I'd suggest just omitting the calendar changes. Thanks a bunch for keeping us in mind though!
Attachment #8423554 - Flags: review?(philipp)
I've filed bug 1011679 for removing the calendar bits.
Comment on attachment 8423554 [details] [diff] [review]
Mike Hommey <mh+mozilla@glandium.org>

Review of attachment 8423554 [details] [diff] [review]:
-----------------------------------------------------------------

SeaMonkey is stuck on seamonkey-production atm, so I'd say thank you for thinking/caring about us, but best to just omit it from your patch right now and land away.
Attachment #8423554 - Flags: review?(bugspam.Callek)
Same patch, without calendar and seamonkey.
Attachment #8423554 - Attachment is obsolete: true
Comment on attachment 8425882 [details] [diff] [review]
Add a MOZ_AUTOMATION environment to all builds

Carrying over r+
Attachment #8425882 - Flags: review+
Something here went live today
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.