Closed Bug 1137000 Opened 9 years ago Closed 9 years ago

Move sdk building to moz-automation

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: glandium, Assigned: mshal)

References

Details

Attachments

(2 files)

Most build steps for automation are not in moz-automation.mk, but building the sdk is not. This bug is about changing that.
Comment on attachment 8569529 [details] [diff] [review]
Add support for SDK building to moz-automation.mk

Looks good!
Attachment #8569529 - Flags: review?(mshal) → review+
Reassigning to mshal for the buildbot/mozconfig part.
Assignee: mh+mozilla → mshal
Depends on: 1137406
I tried to turn MOZ_AUTOMATION_SDK on as a default just to see what the impact would be. Here's an overview:

linux builds:  ~1m30s runtime, 116MB file
osx builds:     ~3-4m runtime, 159MB file
windows builds: ~3-4m runtime, 67MB file
linux asan:     ~4-5m runtime, 400-642MB file

I'm guessing we really don't want it turned on for asan (the large runtime & filesize comes from a ~8x larger libxul.so), so we probably want a more targetted approach.

Right now the xulrunner builds are only on Linux opt, Linux x64 opt, OS X 10.8 opt, and Windows XP opt as far as I can tell. We can probably mimic that closely enough by just adding MOZ_AUTOMATION_SDK=1 to browser/config/mozconfigs/*/nightly. catlee, does that sound reasonable?
Flags: needinfo?(catlee)
glandium, is the goal of this bug ultimately to fix bug 672509 (and by extension, bug 1137382)? If so, I'm guessing the mozconfig part is just enabling MOZ_AUTOMATION_SDK where we need it, and the buildbot part involves nuking xulrunner builds, right? If there is some other intent behind this bug, let me know.
Flags: needinfo?(mh+mozilla)
Ultimately, bug 672509, but currently, just to get make sdk off buildbot.
Flags: needinfo?(mh+mozilla)
Note nightly mozconfigs are used for all builds.
Yeah, I think having this on for asan isn't worthwhile.

Here are a few options I'm thinking about, they're not mutually exclusive:
- Build & upload sdk as part of nightly builds. This is marginally better than the current situation because people do pay attention to nightly builds (vs xulrunner builds), and can reproduce on try. Downside is that finding the bustage could take a while.

- Build sdk but don't upload on all opt builds. Upside is we find the bustage immediately. Downside is extra run-time on osx/windows in particular.

- Build sdk but don't upload on periodic builds (e.g. PGO). This misses OSX coverage, but the advantage is we're not spending 3-4 extra minutes per push doing this.

Thoughts?

Is there a need to upload the SDK per push?
Flags: needinfo?(catlee)
(In reply to Mike Hommey [:glandium] from comment #9)
> Note nightly mozconfigs are used for all builds.

They aren't used in debug builds though, as far as I can see. If we want to do "real" nightly instead of opt-only, we can put it inside an IS_NIGHTLY check, similar to upload_symbols/update_packaging.
(In reply to Chris AtLee [:catlee] from comment #10)
> - Build & upload sdk as part of nightly builds. This is marginally better
> than the current situation because people do pay attention to nightly builds
> (vs xulrunner builds), and can reproduce on try. Downside is that finding
> the bustage could take a while.

IMO this is a good next step. I think finding the bustage after a day is better than what we have now (whenever someone happens to look at xulrunner builds?)

By reproduce on try, that means changing the mozconfig to set MOZ_AUTOMATION_SDK? That's not too bad, but of course I would prefer bug 1132503 :)

> - Build sdk but don't upload on all opt builds. Upside is we find the
> bustage immediately. Downside is extra run-time on osx/windows in particular.
> 
> - Build sdk but don't upload on periodic builds (e.g. PGO). This misses OSX
> coverage, but the advantage is we're not spending 3-4 extra minutes per push
> doing this.
> 
> Thoughts?
> 
> Is there a need to upload the SDK per push?

The downside to building but not uploading is that we'd need some custom logic to conditionally add the sdk file to UPLOAD_FILES, so probably another variable to set in the mozconfig (MOZ_AUTOMATION_SDK_UPLOAD?). It's not hard to do, but it just adds to the configuration mess.

I don't have the historical context to know if it's worth adding 3-4min for OSX+Windows builds to catch SDK breakage immediately - I'm leaning towards "not worth it". Let me know if there are arguments to the contrary!
Note that ATM I don't really care about enabling SDK where it's not *currently* built. At this point, I'd like this bug to only be about moving things from buildbot to in-tree. Enabling SDK on more builds can be a followup (although, if there was one place I'd be interested in SDK builds, it's nightlies (and not because or the possibility to find breakage))
See Also: → 1142231
I was looking into just turning on mozharness+mach for xulrunner builds, but unfortunately the way they're defined in buildbot is a bit odd. Instead of a separate platform that points to the xulrunner config like you might expect, there is a src_xulrunner_mozconfig parameter tacked on to existing platforms. Eg, linux has:

            'src_mozconfig': 'browser/config/mozconfigs/linux32/nightly',
            'src_xulrunner_mozconfig': 'xulrunner/config/mozconfigs/linux32/xulrunner',

So there isn't a good place to just add another 'mozharness_desktop_build' parameter and start building with mach for xulrunner builds. And without using mozharness+mach to build, we don't get the MOZ_AUTOMATION_* flags. I think it would involve quite a bit more buildbot work than I was expecting to get this running as the platforms are currently defined. Are there any objections to jumping straight to just enabling SDK builds on nightlies?
Attached patch sdk-nightlySplinter Review
This seems to work for nightlies when I tested in staging. Note I had to add an sdkUrl so the sdk.tar.bz2 doesn't get picked up as the packageUrl. Also, I removed this strange line from packager.mk:

@rm -rf $(DIST)/$(PKG_PATH)$(PKG_BASENAME).tar $(DIST)/$(PKG_PATH)$(PKG_BASENAME).dmg $@ $(EXCLUDE_LIST)

As far as I can tell, the .tar file doesn't exist anywhere, $@ refers to "stage-package", which we don't actually create, and EXCLUDE_LIST isn't defined anywhere. That means only the .dmg file actually does anything, and it results in 'make sdk' deleting the package created by 'make package', but only on OSX. Since the other platforms seem to get along fine without removing their package files in stage-package, I think the whole line can go away.
Attachment #8579043 - Flags: review?(mh+mozilla)
Attachment #8579043 - Flags: review?(mh+mozilla) → review+
How much is it going to add on the ftp?
When I tested on staging, the sdk added ~70MB-160MB depending on platform.
Status: NEW → RESOLVED
Closed: 9 years ago
Keywords: leave-open
Resolution: --- → FIXED
Blocks: 1147577
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.