Produce and smoketest artifact builds in automation

RESOLVED FIXED

Status

defect
RESOLVED FIXED
4 years ago
7 months ago

People

(Reporter: nalexander, Assigned: chmanchester)

Tracking

(Blocks 2 bugs)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

This ticket tracks trying to prevent regressions with the artifact build toolchain itself by producing artifact builds and running some minimal smoketest against them in automation.  It's really not about prevent regressions in the produced artifact builds themselves, per se -- that's a harder problem, better suited to Bug 1237688.

Artifact builds are unusual because they're best modeled in TaskCluster as build jobs that depend on earlier build jobs.  (And then have subsequent test jobs depending on them.) I hope TC models this smoothly, but I am only aware of build jobs with no dependencies and test jobs depending only on build jobs.  To be clear, we'd like jobs like:

B => Artifact B => Smoketest

where "X => Y" denotes that outputs from X are fed into Y, and therefore that Y depends on X.

In theory, arranging for the "Artifact B" jobs shouldn't be too hard, since |mach artifact install| supports using artifacts from a local file and a remote URL.  (Recent changes with test binaries might require us to improve this support.)

We could remove the smoketest entirely by verifying that the package the "Artifact B" job produces is binary identical to the package produced by the underlying "B" job, although this may run into all the regular issues: timestamps, build machine paths, etc.
Assignee

Updated

3 years ago
Depends on: 1247852
Assignee

Updated

3 years ago
Depends on: 1248011
I have a work in progress for linux I will post shortly. This still fails when we try to find a recent revision due to mozext not being installed on the builders, but I think that requirement is going away soon anyway.
is the purpose of this bug to prove that we can use artifact builds in automation instead of regular/clobber builds?  Are there concerns that we are producing invalid builds?
(In reply to Joel Maher (:jmaher) from comment #5)
> is the purpose of this bug to prove that we can use artifact builds in
> automation instead of regular/clobber builds?  Are there concerns that we
> are producing invalid builds?

The purpose is trying to make sure that changes to the build system or our in-tree moz.build files / code (insofar as one would argue there's a difference) don't break artifact builds made by developers. Shortly after the builds were introduced they were broken on some platforms by the changes to support testing, and so like most other things that we (or at least some of us) rely upon for our daily work, it should turn the tree red/orange if it is broken. :-)
Assignee

Updated

3 years ago
Attachment #8720599 - Attachment description: MozReview Request: Bug 1240149 - Build system changes necessary to run linux artifact builds in automation. → MozReview Request: Bug 1240149 - Build system changes necessary to run linux artifact builds in automation. r=glandium
Attachment #8720599 - Flags: review?(mh+mozilla)
Comment on attachment 8720599 [details]
MozReview Request: Bug 1240149 - Build system changes necessary to run linux artifact builds in automation. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35369/diff/1-2/
Assignee

Updated

3 years ago
Attachment #8720600 - Attachment description: MozReview Request: Bug 1240149 - Add a mozharness config for linux artifact builds in automation. → MozReview Request: Bug 1240149 - Install Python packages necessary to artifact builds from mozilla pypi when running in automation. r=nalexander
Attachment #8720600 - Flags: review?(nalexander)
Comment on attachment 8720600 [details]
MozReview Request: Bug 1240149 - Install Python packages necessary to artifact builds from mozilla pypi when running in automation. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35371/diff/1-2/
Comment on attachment 8720601 [details]
MozReview Request: Bug 1240149 - Add a mozharness config for linux artifact builds in automation. r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35373/diff/1-2/
Attachment #8720601 - Attachment description: MozReview Request: Bug 1240149 - Add taskcluster definitions for a linux artifact based build in automation. → MozReview Request: Bug 1240149 - Add a mozharness config for linux artifact builds in automation. r=jlund
Attachment #8720601 - Flags: review?(jlund)
Comment on attachment 8720600 [details]
MozReview Request: Bug 1240149 - Install Python packages necessary to artifact builds from mozilla pypi when running in automation. r=nalexander

https://reviewboard.mozilla.org/r/35371/#review34473

If it works for you, it works for me.

::: python/mozbuild/mozbuild/mach_commands.py:1503
(Diff revision 2)
> +        self.virtualenv_manager.install_pip_package(package)

Consider folding this into `virtualenv_manager`.  I expect others will want the same thing.  I suppose it's hard to know which packages need it, so it needs to be a flag or a separate method.
Attachment #8720600 - Flags: review?(nalexander) → review+
Comment on attachment 8720601 [details]
MozReview Request: Bug 1240149 - Add a mozharness config for linux artifact builds in automation. r=jlund

https://reviewboard.mozilla.org/r/35373/#review34475

::: testing/mozharness/configs/builds/releng_sub_linux_configs/64_artifact.py:66
(Diff revision 2)
> +        'gcc45_0moz3', 'gcc454_0moz1', 'gcc472_0moz1', 'gcc473_0moz1',

Shouldn't we not need these for artifact builds?  I'm aware there's often work needed to *really* not require a toolchain, but I thought that was done for Linux.
Attachment #8720601 - Flags: review+
Attachment #8725995 - Flags: review?(nalexander) → review+
Comment on attachment 8725995 [details]
MozReview Request: Bug 1240149 - Add taskcluster definitions for a linux artifact based build in automation. r=nalexander

https://reviewboard.mozilla.org/r/37749/#review34479

lgtm.
https://reviewboard.mozilla.org/r/35369/#review34483

::: toolkit/mozapps/installer/packager.py:370
(Diff revision 2)
> -        if not mozinfo.isMac:
> +        if not mozinfo.isMac and not buildconfig.substs.get('MOZ_ARTIFACT_BUILDS'):

`MOZ_ARTIFACT_BUILDS` seems wrong here.  Shouldn't it be something to do with the compile environment?
https://reviewboard.mozilla.org/r/35373/#review34475

> Shouldn't we not need these for artifact builds?  I'm aware there's often work needed to *really* not require a toolchain, but I thought that was done for Linux.

This config is pretty much cargo-culted from others in this directory, but I did find that configure has some check compile tests that run even without COMPILE_ENVIRONMENT, so we need a compiler to build (the first one that gets hit is checking for joystick.h, but I'm sure there are others).
Comment on attachment 8720601 [details]
MozReview Request: Bug 1240149 - Add a mozharness config for linux artifact builds in automation. r=jlund

https://reviewboard.mozilla.org/r/35373/#review34515

mh config seems sane. if it works for you, works for me :)
Attachment #8720601 - Flags: review?(jlund) → review+
Comment on attachment 8720599 [details]
MozReview Request: Bug 1240149 - Build system changes necessary to run linux artifact builds in automation. r=glandium

https://reviewboard.mozilla.org/r/35369/#review34637

::: toolkit/locales/Makefile.in:31
(Diff revision 2)
> +ifneq (,$(wildcard $(FINAL_TARGET)))

That seems weird. This /should/ always be true. What's going wrong?
Attachment #8720599 - Flags: review?(mh+mozilla)
https://reviewboard.mozilla.org/r/35369/#review34637

> That seems weird. This /should/ always be true. What's going wrong?

This rule runs during |make package|, and the value of FINAL_TARGET is dist/xpi-stage at that time, which doesn't exist for artifact builds. Obviously this is just a cosmetic fix, why we're running this rule during make package I don't know.
https://reviewboard.mozilla.org/r/35371/#review34473

> Consider folding this into `virtualenv_manager`.  I expect others will want the same thing.  I suppose it's hard to know which packages need it, so it needs to be a flag or a separate method.

I'm reluctant to make this seem standard. For reasons discussed yesterday we're in a grey area here.
https://reviewboard.mozilla.org/r/35371/#review34473

> I'm reluctant to make this seem standard. For reasons discussed yesterday we're in a grey area here.

Agreed.
needinfo re: comment 19, I'm not sure how to proceed here.
Flags: needinfo?(mh+mozilla)
(In reply to Chris Manchester (:chmanchester) from comment #19)
> https://reviewboard.mozilla.org/r/35369/#review34637
> 
> > That seems weird. This /should/ always be true. What's going wrong?
> 
> This rule runs during |make package|, and the value of FINAL_TARGET is
> dist/xpi-stage at that time, which doesn't exist for artifact builds.
> Obviously this is just a cosmetic fix, why we're running this rule during
> make package I don't know.

Is it limited to android builds or does it happen on other builds?
Flags: needinfo?(mh+mozilla)
Flags: needinfo?(cmanchester)
(In reply to Mike Hommey [:glandium] from comment #23)
> (In reply to Chris Manchester (:chmanchester) from comment #19)
> > https://reviewboard.mozilla.org/r/35369/#review34637
> > 
> > > That seems weird. This /should/ always be true. What's going wrong?
> > 
> > This rule runs during |make package|, and the value of FINAL_TARGET is
> > dist/xpi-stage at that time, which doesn't exist for artifact builds.
> > Obviously this is just a cosmetic fix, why we're running this rule during
> > make package I don't know.
> 
> Is it limited to android builds or does it happen on other builds?

I've only tried this on a Linux desktop build.

I looked in to this a little more, and this is failing when `make package` invokes `make langpack` in browser/locales, which invokes `make libs` in toolkit/locales/Makefile.in. The failing libs rule is expecting JAR_MANIFEST to be set by the recursive make backend, which triggers creating the FINAL_TARGET directory and jar manifest processing in rules.mk. JAR_MANIFEST isn't set in backend.mk when we're using the hybrid faster make backend, because the FasterMake backend handles the corresponding JARManifest, and the RecursiveMake backend never sees it.

As far as I can tell, things like this are not intended to work when using FasterMake for now, and we can smoketest an artifact build without it, so I'll just post a patch that guards this rule in a slightly more intelligent way.
Flags: needinfo?(cmanchester)
Comment on attachment 8720599 [details]
MozReview Request: Bug 1240149 - Build system changes necessary to run linux artifact builds in automation. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35369/diff/2-3/
Attachment #8720599 - Flags: review?(mh+mozilla)
Comment on attachment 8720600 [details]
MozReview Request: Bug 1240149 - Install Python packages necessary to artifact builds from mozilla pypi when running in automation. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35371/diff/2-3/
Comment on attachment 8720601 [details]
MozReview Request: Bug 1240149 - Add a mozharness config for linux artifact builds in automation. r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/35373/diff/2-3/
Comment on attachment 8725995 [details]
MozReview Request: Bug 1240149 - Add taskcluster definitions for a linux artifact based build in automation. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/37749/diff/1-2/
Comment on attachment 8720599 [details]
MozReview Request: Bug 1240149 - Build system changes necessary to run linux artifact builds in automation. r=glandium

https://reviewboard.mozilla.org/r/35369/#review35529

::: browser/installer/Makefile.in:143
(Diff revision 3)
> +# Builds using the hybrid FasterMake/RecursiveMake backend will
> +# fail to produce a langpack. This is the default for artifact
> +# based builds.

Can you file a bug so that we fix that?

::: browser/installer/Makefile.in:147
(Diff revision 3)
> +ifndef MOZ_ARTIFACT_BUILDS

Considering it's a backend thing more than a MOZ_ARTIFACT_BUILDS thing, it's better to check ifeq (,$(filter FasterMake+RecursiveMake,$(BUILD_BACKENDS)))
Attachment #8720599 - Flags: review?(mh+mozilla) → review+
Assignee

Updated

3 years ago
Blocks: 1255096
https://reviewboard.mozilla.org/r/35369/#review35529

> Can you file a bug so that we fix that?

Filed bug 1255096.
I'm going to land what I have here, but I still need to get these scheduled by default and add a smoketest step.
Assignee: nobody → cmanchester
Keywords: leave-open
(In reply to Chris Manchester (:chmanchester) from comment #31)
> I'm going to land what I have here, but I still need to get these scheduled
> by default and add a smoketest step.

Correction, the previous patch did enable them by default. I've asked they be moved to Tier 2 for now, if they're particularly unstable I'll back out the last patch.
Depends on: 1256004

Updated

Last year
Product: Core → Firefox Build System
The leave-open keyword is there and there is no activity for 6 months.
:chmanchester, maybe it's time to close this bug?
Flags: needinfo?(cmanchester)
This is fixed.
Status: NEW → RESOLVED
Closed: 7 months ago
Flags: needinfo?(cmanchester)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.