1240149 - Produce and smoketest artifact builds in automation

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Nick Alexander :nalexander [he/him]]

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.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

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.

Comment 2

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1240149 - Build system changes necessary to run linux artifact builds in automation. r=glandium

Review commit: https://reviewboard.mozilla.org/r/35369/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/35369/

Comment 3

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1240149 - Install Python packages necessary to artifact builds from mozilla pypi when running in automation. r=nalexander

Review commit: https://reviewboard.mozilla.org/r/35371/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/35371/

Comment 4

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1240149 - Add a mozharness config for linux artifact builds in automation. r=jlund

Review commit: https://reviewboard.mozilla.org/r/35373/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/35373/

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

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?

Comment 6

[:Gijs (he/him)]

(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. :-)

Comment 7

[Chris Manchester (limited bugmail, email directly)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=68254b25f389

Comment 8

[Chris Manchester (limited bugmail, email directly)]

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/

Comment 9

[Chris Manchester (limited bugmail, email directly)]

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 10

[Chris Manchester (limited bugmail, email directly)]

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/

Comment 11

[Chris Manchester (limited bugmail, email directly)]

Attachment: MozReview Request: Bug 1240149 - Add taskcluster definitions for a linux artifact based build in automation. r=nalexander

Review commit: https://reviewboard.mozilla.org/r/37749/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/37749/

Comment 12

[Nick Alexander :nalexander [he/him]]

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.

Comment 13

[Nick Alexander :nalexander [he/him]]

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.

Comment 14

[Nick Alexander :nalexander [he/him]]

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.

Comment 15

[Nick Alexander :nalexander [he/him]]

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?

Comment 16

[Chris Manchester (limited bugmail, email directly)]

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 17

[Jordan Lund (:jlund)]

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 :)

Comment 18

[Mike Hommey [:glandium]]

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?

Comment 19

[Chris Manchester (limited bugmail, email directly)]

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.

Comment 20

[Chris Manchester (limited bugmail, email directly)]

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.

Comment 21

[Nick Alexander :nalexander [he/him]]

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.

Comment 22

[Chris Manchester (limited bugmail, email directly)]

needinfo re: comment 19, I'm not sure how to proceed here.

Comment 23

[Mike Hommey [:glandium]]

(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?

Comment 24

[Chris Manchester (limited bugmail, email directly)]

(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.

Comment 25

[Chris Manchester (limited bugmail, email directly)]

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/

Comment 26

[Chris Manchester (limited bugmail, email directly)]

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 27

[Chris Manchester (limited bugmail, email directly)]

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 28

[Chris Manchester (limited bugmail, email directly)]

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 29

[Mike Hommey [:glandium]]

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)))

Comment 30

[Chris Manchester (limited bugmail, email directly)]

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

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

Filed bug 1255096.

Comment 31

[Chris Manchester (limited bugmail, email directly)]

I'm going to land what I have here, but I still need to get these scheduled by default and add a smoketest step.

Comment 32

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/079e85fe0e4d
https://hg.mozilla.org/integration/mozilla-inbound/rev/00de67af498e
https://hg.mozilla.org/integration/mozilla-inbound/rev/f271e67bc7f4
https://hg.mozilla.org/integration/mozilla-inbound/rev/65f081666446

Comment 33

[Chris Manchester (limited bugmail, email directly)]

(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.

Comment 34

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/079e85fe0e4d
https://hg.mozilla.org/mozilla-central/rev/00de67af498e
https://hg.mozilla.org/mozilla-central/rev/f271e67bc7f4
https://hg.mozilla.org/mozilla-central/rev/65f081666446

Comment 35

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/6ed6f050bbf3

Comment 36

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/6ed6f050bbf3

Comment 37

[BugBot [:suhaib / :marco/ :calixte]]

The leave-open keyword is there and there is no activity for 6 months.
:chmanchester, maybe it's time to close this bug?

Comment 38

[Chris Manchester (limited bugmail, email directly)]

This is fixed.