Run |mach artifact| automatically when --disable-compile-environment is set

RESOLVED FIXED in Firefox 46

Status

defect
RESOLVED FIXED
4 years ago
Last year

People

(Reporter: nalexander, Assigned: nalexander)

Tracking

unspecified
mozilla46
Dependency tree / graph

Firefox Tracking Flags

(firefox46 fixed)

Details

Attachments

(5 attachments)

We want to make artifact based builds transparent when we can.  This ticket tracks extracting enough build environment information from the build configuration to run |mach artifact| at an appropriate time.

Right now, mobile/android needs |mach artifact| before |mach package| only.  (|mach build| does not interact with the binary artifacts.)  glandium's |mach build faster| work wants artifacts to be in place before the build begins, since there is code that copies artifacts into place at build time.  Desktop developers likely won't use |mach package| at all, but they will use |mach run|.
It's important that artifacts be updated as part of incremental builds, because developers navigate between local commits in their source trees which may not trigger a configure (or even a build-backend).

glandium: can you suggest a good place to hook this updating?  Maybe into the same mechanism that checks if build-backend (or configure) is needed?  Maybe into the .xpt refreshing mechanism?
Flags: needinfo?(mh+mozilla)
> Run |mach artifact| automatically when --disable-compile-environment is set

Not a good idea. There are different use cases for --disable-compile-environment that _don't_ require artifacts.

(In reply to Nick Alexander :nalexander from comment #1)
> glandium: can you suggest a good place to hook this updating?  Maybe into
> the same mechanism that checks if build-backend (or configure) is needed? 
> Maybe into the .xpt refreshing mechanism?

That's a difficult question. I'd rather avoid anything that requires different build backends to have their own version of the hooking code. I think the right place would be in the mach build command handler, but aiui, mach artifact needs to run after configure, and mach build doesn't handle configure separately itself. So I guess at this point the right place would be client.mk, with the expected outcome of being moved along other things from client.mk into mach build itself (or configure/build-backend, depending on how this all ends up being hooked up together)
Flags: needinfo?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #2)
> > Run |mach artifact| automatically when --disable-compile-environment is set
> 
> Not a good idea. There are different use cases for
> --disable-compile-environment that _don't_ require artifacts.

I agree.  I'll add a --enable-artifact-downloading flag to opt-in to this.  I wish we had a mach-specific rc file for this kind of configuration switch, but it's not so bad since that flag will imply --d-c-e, which isn't something you can turn on and off without needing to clobber.

> (In reply to Nick Alexander :nalexander from comment #1)
> > glandium: can you suggest a good place to hook this updating?  Maybe into
> > the same mechanism that checks if build-backend (or configure) is needed? 
> > Maybe into the .xpt refreshing mechanism?
> 
> That's a difficult question. I'd rather avoid anything that requires
> different build backends to have their own version of the hooking code. I
> think the right place would be in the mach build command handler, but aiui,
> mach artifact needs to run after configure, and mach build doesn't handle
> configure separately itself. So I guess at this point the right place would
> be client.mk, with the expected outcome of being moved along other things
> from client.mk into mach build itself (or configure/build-backend, depending
> on how this all ends up being hooked up together)

You're correct that |mach artifact| wants a configured tree, for both build flags and the Python virtualenv.

I've dug into this and it is a difficult problem.  I found I could wedge a target into Makefile.in that would execute 1-3 times for incremental builds, but I didn't find such a place in client.mk.

The more I think about it, the less I think we should put any artifact stuff in Make at all.  |mach artifact| just doesn't have nice dependencies -- it depends on the source revision, and the state of the upstream builders.

I see that bare client.mk is executed from |mach build| at https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/mach_commands.py#379.  How hard would it be to make the configure step explicit?  |make -f client.mk configure| forces a reconfigure, so that's bad; could we check for the presence of $OBJDIR/Makefile (like |mach build what| does) and explicitly run configure then, and then insert the artifact step, and then run |make -f client.mk|?  That should work save in unusual circumstances.
Flags: needinfo?(mh+mozilla)
(In reply to Nick Alexander :nalexander from comment #3)
> I see that bare client.mk is executed from |mach build| at
> https://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/
> mach_commands.py#379.  How hard would it be to make the configure step
> explicit?  |make -f client.mk configure| forces a reconfigure, so that's
> bad; could we check for the presence of $OBJDIR/Makefile (like |mach build
> what| does) and explicitly run configure then, and then insert the artifact
> step, and then run |make -f client.mk|?  That should work save in unusual
> circumstances.

I'm sure that will lead to many corner cases where configure runs twice or something. That's why I suggested to put this into client.mk somehow, until the day we actually get rid of client.mk. It seems to me to be the least worse option.
Flags: needinfo?(mh+mozilla)
> It seems to me to be the least worse option.

At the moment, that is.
glandium: I have a WIP patch that runs |mach artifact install| early in the build.  That extracts .so libraries into dist/bin.  In #build, we talked about the dist/bin install manifest deleting those .so libraries immediately after they are installed, and you suggested modifying the emitter to capture the .so files that would be produced.

Sadly, !COMPILE_ENVIRONMENT skips nss/ and other parts of the Makefile.in-based build system and not all the relevant SharedLibraries are produced.

The set of .so files is encoded in the package-manifest.in, but reading that at the beginning of the build seems totally wrong.

The simplest thing to do is, for these artifact based builds, not purge *.so (and some other stuff) in dist/bin.  Or we could just not purge for dist/bin altogether.

To purge just *.so is non-trivial, since right now, optional_exists doesn't accept a pattern.  Patterns in the manifest code evaluate patterns at manifest creation time, which doesn't make sense in this case (we need the pattern to be evaluated at manifest install time, not at manifest creation time).  In a different ticket I once added a filter to the manifest deleter to allow keeping file names that match a predicate, which would work here.

Is there some terrible repercussion to not purging dist/bin for artifact based builds?
Flags: needinfo?(mh+mozilla)
(In reply to Nick Alexander :nalexander from comment #6)
> Is there some terrible repercussion to not purging dist/bin for artifact
> based builds?

About as bad as FasterMake builds not doing so. And in both cases, the right thing to do is actually to have install manifest processing keep track of the files it installed, and remove those from the list that it doesn't have to install anymore. That's something on my todo list, but I don't know when I'll get to it. I think the simplest thing to do is to just copy the install manifest after processing, and pass the (old) copy to subsequent calls to the processor.
Flags: needinfo?(mh+mozilla)
(with the full copy of the install manifest, we can also correctly skip preprocessing if dependencies haven't changed *and* defines haven't changed ; although if we kept track of the defines actually used, that would allow even more fine-grained skipping, but I don't think it's worth the extra effort)
Bug 1216817 - Part 1: Add install_callback to artifacts. r?glandium
Attachment #8681468 - Flags: review?(mh+mozilla)
Bug 1216817 - Part 2: Narrow distdir to bindir in artifacts. r?glandium
Attachment #8681469 - Flags: review?(mh+mozilla)
Bug 1216817 - Part 3: Add files from artifacts to $OBJDIR/_build_manifests/install/dist_bin. r?glandium

This allows to run |mach artifact install| immediately after |mach
configure| and not have a subsequent |mach build| delete the files
that |mach artifact install| copied into dist/bin.
Attachment #8681470 - Flags: review?(mh+mozilla)
Bug 1216817 - Part 4: Add MOZ_ARTIFACT_BUILDS; run |mach artifact install| automatically. r?glandium

It turns out to be much easier to hook |mach artifact install| into
config.status and |mach build| than to hook into client.mk.
Attachment #8681471 - Flags: review?(mh+mozilla)
See Also: → 1227248
Comment on attachment 8681468 [details]
MozReview Request: Bug 1216817 - Part 1: Add install_callback to artifacts. r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23833/diff/1-2/
Comment on attachment 8681469 [details]
MozReview Request: Bug 1216817 - Part 2: Narrow distdir to bindir in artifacts. r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23835/diff/1-2/
Comment on attachment 8681470 [details]
MozReview Request: Bug 1216817 - Part 3: Add files from artifacts to $OBJDIR/_build_manifests/install/dist_bin. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23837/diff/1-2/
Attachment #8681471 - Attachment description: MozReview Request: Bug 1216817 - Part 4: Add MOZ_ARTIFACT_BUILDS; run |mach artifact install| automatically. r?glandium → MozReview Request: Bug 1216817 - Part 4: Add --enable-artifact-builds and MOZ_ARTIFACT_BUILDS. r?glandium
Comment on attachment 8681471 [details]
MozReview Request: Bug 1216817 - Part 4: Add --enable-artifact-builds and MOZ_ARTIFACT_BUILDS. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23839/diff/1-2/
Bug 1216817 - Part 5: Run |mach artifact install| automatically when asked. r?glandium

It turns out to be much easier to hook |mach artifact install| into
config.status and |mach build| than to hook into client.mk.
Attachment #8691739 - Flags: review?(mh+mozilla)
glandium: here's a rebased version that puts the somewhat controversial part at the end.  I would like you to either respond to this or delegate it very quickly; it's been more than three weeks.

gps: do you have bandwidth to review this?

Since it's behind a flag, it would only impact folks who opt-in, and that will be only Android consumers for some time.
Assignee: nobody → nalexander
Status: NEW → ASSIGNED
Flags: needinfo?(gps)
mhoye: this is relevant to your interests.
Flags: needinfo?(mhoye)
Attachment #8681468 - Flags: review+
Comment on attachment 8681468 [details]
MozReview Request: Bug 1216817 - Part 1: Add install_callback to artifacts. r=gps

https://reviewboard.mozilla.org/r/23833/#review23625

This patch by itself is fine.

An alternate would be to refactor this whole thing to be a generator of some sort. But that feels like a lot of work. Callbacks feel easier given the code you have.
Comment on attachment 8681469 [details]
MozReview Request: Bug 1216817 - Part 2: Narrow distdir to bindir in artifacts. r=gps

https://reviewboard.mozilla.org/r/23835/#review23627

Nothing controversial here. Just factoring out the os.path.join(X, 'bin', Y).
Attachment #8681469 - Flags: review+
https://reviewboard.mozilla.org/r/23837/#review23629

This patch is a bit contentious because it means that files from the artifact will linger in the objdir. I could imagine getting into trouble where the artifact files are installed, the source directory is updated to a version that removed a file, but the file still lingers in the objdir.

I'm going to give glandium a chance to weigh in here.

::: mobile/android/mach_commands.py:251
(Diff revision 2)
> +        help='Do not add files from artifacts to _build_manifests/install/dist_bin manifest.')

That's too low level. How about "force the build system to delete and replace files from the artifact."
Comment on attachment 8681471 [details]
MozReview Request: Bug 1216817 - Part 4: Add --enable-artifact-builds and MOZ_ARTIFACT_BUILDS. r=glandium

https://reviewboard.mozilla.org/r/23839/#review24277

::: configure.in:153
(Diff revision 2)
> +MOZ_ARG_ENABLE_BOOL(artifact-builds,

Why plural?
Attachment #8681471 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8691739 [details]
MozReview Request: Bug 1216817 - Part 5: Run |mach artifact install| automatically when asked. r?glandium

https://reviewboard.mozilla.org/r/26167/#review23559

::: python/mozbuild/mozbuild/config_status.py:213
(Diff revision 1)
> +        # Execute |mach artifact install| from the top source directory.
> +        os.chdir(topsrcdir) 
> +        os.execlp('sh', 'sh', '-c', ' '.join([os.path.join(topsrcdir, 'mach'), 'artifact', 'install']))

It would feel better if you could somehow invoke _run_mach_artifact_install here instead of doing manual work with os.execlp.

::: python/mozbuild/mozbuild/mach_commands.py:412
(Diff revision 1)
> +                    self.log(logging.INFO, 'artifact',

Seems like something unnecessary to print out.
Attachment #8691739 - Flags: review?(mh+mozilla)
Attachment #8681470 - Flags: review?(mh+mozilla)
Comment on attachment 8681470 [details]
MozReview Request: Bug 1216817 - Part 3: Add files from artifacts to $OBJDIR/_build_manifests/install/dist_bin. r=glandium

https://reviewboard.mozilla.org/r/23837/#review24279

I'm still not sure about this part. I have something baking that should make this unnecessary, which aiui would make part 1 and 2 unnecessary as well. Let's see how far I can take the baking tomorrow.
Attachment #8681468 - Flags: review?(mh+mozilla)
Attachment #8681469 - Flags: review?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #25)
> Comment on attachment 8681470 [details]
> MozReview Request: Bug 1216817 - Part 3: Add files from artifacts to
> $OBJDIR/_build_manifests/install/dist_bin. r?glandium
> 
> https://reviewboard.mozilla.org/r/23837/#review24279
> 
> I'm still not sure about this part. I have something baking that should make
> this unnecessary, which aiui would make part 1 and 2 unnecessary as well.
> Let's see how far I can take the baking tomorrow.

Filed as bug 1230060 with patches, which don't handle the RecursiveMake backend, but that could be done.
(In reply to Mike Hommey [:glandium] from comment #25)
> Comment on attachment 8681470 [details]
> MozReview Request: Bug 1216817 - Part 3: Add files from artifacts to
> $OBJDIR/_build_manifests/install/dist_bin. r?glandium
> 
> https://reviewboard.mozilla.org/r/23837/#review24279
> 
> I'm still not sure about this part. I have something baking that should make
> this unnecessary, which aiui would make part 1 and 2 unnecessary as well.
> Let's see how far I can take the baking tomorrow.

glandium: you and I didn't get a chance to talk about this.  Can you explain what has happened that changes what these patches use, or what you want to see happen, in order to get |mach artifact install| running automatically as part of |mach configure| and each |mach build| invocation?
Flags: needinfo?(mh+mozilla)
Comment on attachment 8681470 [details]
MozReview Request: Bug 1216817 - Part 3: Add files from artifacts to $OBJDIR/_build_manifests/install/dist_bin. r=glandium

https://reviewboard.mozilla.org/r/23837/#review25705

Turns out bug 1230060 can't be extended to the recursive make backend without leaving dangling files on incremental builds when things are removed from jar.mn. This has been waiting for long enough already, let's go for it.
Attachment #8681470 - Flags: review+
https://reviewboard.mozilla.org/r/23837/#review25707

::: mobile/android/mach_commands.py:250
(Diff revision 2)
> -    def artifact_install(self, source=None, tree=None, job=None, verbose=False):
> +    @CommandArgument('--no-update-manifest', dest='update_manifest', action='store_false',

That said, I'm not sure why provide an override here...
And sorry again for the various delays on this bug :(
Flags: needinfo?(mh+mozilla)
(In reply to Mike Hommey [:glandium] from comment #29)
> https://reviewboard.mozilla.org/r/23837/#review25707
> 
> ::: mobile/android/mach_commands.py:250
> (Diff revision 2)
> > -    def artifact_install(self, source=None, tree=None, job=None, verbose=False):
> > +    @CommandArgument('--no-update-manifest', dest='update_manifest', action='store_false',
> 
> That said, I'm not sure why provide an override here...

I've removed this option.
(In reply to Mike Hommey [:glandium] from comment #23)
> Comment on attachment 8681471 [details]
> MozReview Request: Bug 1216817 - Part 4: Add --enable-artifact-builds and
> MOZ_ARTIFACT_BUILDS. r?glandium
> 
> https://reviewboard.mozilla.org/r/23839/#review24277
> 
> ::: configure.in:153
> (Diff revision 2)
> > +MOZ_ARG_ENABLE_BOOL(artifact-builds,
> 
> Why plural?

No significant reason.  To me, |mach artifact| isn't really a "build flag" but a "mode switch" that doesn't apply to just a single build but instead to a a whole sequence of builds (and source directory updates).
(In reply to Mike Hommey [:glandium] from comment #24)
> Comment on attachment 8691739 [details]
> MozReview Request: Bug 1216817 - Part 5: Run |mach artifact install|
> automatically when asked. r?glandium
> 
> https://reviewboard.mozilla.org/r/26167/#review23559
> 
> ::: python/mozbuild/mozbuild/config_status.py:213
> (Diff revision 1)
> > +        # Execute |mach artifact install| from the top source directory.
> > +        os.chdir(topsrcdir) 
> > +        os.execlp('sh', 'sh', '-c', ' '.join([os.path.join(topsrcdir, 'mach'), 'artifact', 'install']))
> 
> It would feel better if you could somehow invoke _run_mach_artifact_install
> here instead of doing manual work with os.execlp.

I agree but I don't know how to arrange it, since the mach environment is very different from the config.status environment.  I cribbed this from similar existing code: https://dxr.mozilla.org/mozilla-central/rev/388bdc46ba51ee31da8b8abe977e0ca38d117434/python/mozbuild/mozbuild/config_status.py#168.

> ::: python/mozbuild/mozbuild/mach_commands.py:412
> (Diff revision 1)
> > +                    self.log(logging.INFO, 'artifact',
> 
> Seems like something unnecessary to print out.

I've dropped this down to DEBUG.  I'm looking ahead to when we try to debug issues with this in the wild; it might help spot issues where unconfigured or freshly clobbered trees don't get artifacts correctly.
Comment on attachment 8681468 [details]
MozReview Request: Bug 1216817 - Part 1: Add install_callback to artifacts. r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23833/diff/2-3/
Attachment #8681468 - Attachment description: MozReview Request: Bug 1216817 - Part 1: Add install_callback to artifacts. r?glandium → MozReview Request: Bug 1216817 - Part 1: Add install_callback to artifacts. r=gps
Comment on attachment 8681469 [details]
MozReview Request: Bug 1216817 - Part 2: Narrow distdir to bindir in artifacts. r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23835/diff/2-3/
Attachment #8681469 - Attachment description: MozReview Request: Bug 1216817 - Part 2: Narrow distdir to bindir in artifacts. r?glandium → MozReview Request: Bug 1216817 - Part 2: Narrow distdir to bindir in artifacts. r=gps
Comment on attachment 8681470 [details]
MozReview Request: Bug 1216817 - Part 3: Add files from artifacts to $OBJDIR/_build_manifests/install/dist_bin. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23837/diff/2-3/
Attachment #8681470 - Attachment description: MozReview Request: Bug 1216817 - Part 3: Add files from artifacts to $OBJDIR/_build_manifests/install/dist_bin. r?glandium → MozReview Request: Bug 1216817 - Part 3: Add files from artifacts to $OBJDIR/_build_manifests/install/dist_bin. r=glandium
Comment on attachment 8681471 [details]
MozReview Request: Bug 1216817 - Part 4: Add --enable-artifact-builds and MOZ_ARTIFACT_BUILDS. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23839/diff/2-3/
Attachment #8681471 - Attachment description: MozReview Request: Bug 1216817 - Part 4: Add --enable-artifact-builds and MOZ_ARTIFACT_BUILDS. r?glandium → MozReview Request: Bug 1216817 - Part 4: Add --enable-artifact-builds and MOZ_ARTIFACT_BUILDS. r=glandium
Comment on attachment 8691739 [details]
MozReview Request: Bug 1216817 - Part 5: Run |mach artifact install| automatically when asked. r?glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26167/diff/1-2/
Attachment #8691739 - Flags: review?(mh+mozilla)
Comment on attachment 8691739 [details]
MozReview Request: Bug 1216817 - Part 5: Run |mach artifact install| automatically when asked. r?glandium

https://reviewboard.mozilla.org/r/26167/#review25929

::: python/mozbuild/mozbuild/config_status.py:216
(Diff revision 2)
> +        os.execlp('sh', 'sh', '-c', ' '.join([os.path.join(topsrcdir, 'mach'), 'artifact', 'install']))

I'm still not comfortable with this, but if we're going to go with an execlp, we might as well avoid using sh, and use python directly, through sys.executable.
Attachment #8691739 - Flags: review?(mh+mozilla)
(In reply to Mike Hommey [:glandium] (VAC: 31 Dec - 11 Jan) from comment #39)
> Comment on attachment 8691739 [details]
> MozReview Request: Bug 1216817 - Part 5: Run |mach artifact install|
> automatically when asked. r?glandium
> 
> https://reviewboard.mozilla.org/r/26167/#review25929
> 
> ::: python/mozbuild/mozbuild/config_status.py:216
> (Diff revision 2)
> > +        os.execlp('sh', 'sh', '-c', ' '.join([os.path.join(topsrcdir, 'mach'), 'artifact', 'install']))
> 
> I'm still not comfortable with this, but if we're going to go with an
> execlp, we might as well avoid using sh, and use python directly, through
> sys.executable.

I don't really care about execlp -- would you prefer just subprocess.check_call (followed by return)?  I was just following suit.  So:
```
subprocess.check_call([sys.executable, os.path.join(topsrcdir, 'mach'), 'artifact', 'install'])
return 0
```

I'd really like to get some form of this in the tree before you PTO in a few days, so tell me whatever you think is appropriate and we'll make it happen.
Flags: needinfo?(mh+mozilla)
os.execlp or subprocess.check_call, it doesn't matter wrt comment 24. If you use the latter, then you should make it return the return value of the check_call.
Flags: needinfo?(mh+mozilla)
(In reply to Mike Hommey [:glandium] (VAC: 31 Dec - 11 Jan) from comment #41)
> os.execlp or subprocess.check_call, it doesn't matter wrt comment 24. If you
> use the latter, then you should make it return the return value of the
> check_call.

I just tried to use sys.executable and check_call; both fail with a Python import error (below).  I wonder if the existing execlp is a work-around for this.  In any case, this shell invocation doesn't really seem worth fighting for.  If you feel strongly, can you work out the updated invocation?

0:45.89 Traceback (most recent call last):
 0:45.89   File "/Users/nalexander/Mozilla/gecko/mach", line 148, in <module>
 0:45.89     main(sys.argv[1:])
 0:45.89   File "/Users/nalexander/Mozilla/gecko/mach", line 76, in main
 0:45.90     mach = get_mach()
 0:45.90   File "/Users/nalexander/Mozilla/gecko/mach", line 67, in get_mach
 0:45.90     mach = check_and_get_mach(dir_path)
 0:45.90   File "/Users/nalexander/Mozilla/gecko/mach", line 42, in check_and_get_mach
 0:45.90     return load_mach(dir_path, mach_path)
 0:45.90   File "/Users/nalexander/Mozilla/gecko/mach", line 30, in load_mach
 0:45.90     return mach_bootstrap.bootstrap(dir_path)
 0:45.90   File "/Users/nalexander/Mozilla/gecko/build/mach_bootstrap.py", line 312, in bootstrap
 0:45.90     mach.load_commands_from_file(os.path.join(mozilla_dir, path))
 0:45.90   File "/Users/nalexander/Mozilla/gecko/python/mach/mach/main.py", line 258, in load_commands_from_file
 0:45.90     imp.load_source(module_name, path)
 0:45.90   File "/Users/nalexander/Mozilla/gecko/testing/xpcshell/mach_commands.py", line 28, in <module>
 0:45.90     from xpcshellcommandline import parser_desktop, parser_remote, parser_b2g
 0:45.90   File "/Users/nalexander/Mozilla/gecko/build/mach_bootstrap.py", line 338, in __call__
 0:45.90     module = self._original_import(name, globals, locals, fromlist, level)
 0:45.90   File "/Users/nalexander/Mozilla/gecko/build/mach_bootstrap.py", line 338, in __call__
 0:45.90     module = self._original_import(name, globals, locals, fromlist, level)
 0:45.90 ImportError: No module named xpcshellcommandline
Flags: needinfo?(mh+mozilla)
That fails because mach apparently fails to run with the virtualenv python, which I think is an unintended bug. In fact, considering the bootstrap code, I'm pretty sure mach was intended to also run with the virtualenv python when it was written, but nothing enforces that it's actually true. So there's discrepancy between the list of SEARCH_PATHS in mach_bootstrap.py, and things available in the virtualenv as per virtualenv_packages.txt. Adding "xpcshell.pth:testing/xpcshell" to that file fixes it.
Flags: needinfo?(mh+mozilla)
Comment on attachment 8681468 [details]
MozReview Request: Bug 1216817 - Part 1: Add install_callback to artifacts. r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23833/diff/3-4/
Comment on attachment 8681469 [details]
MozReview Request: Bug 1216817 - Part 2: Narrow distdir to bindir in artifacts. r=gps

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23835/diff/3-4/
Comment on attachment 8681470 [details]
MozReview Request: Bug 1216817 - Part 3: Add files from artifacts to $OBJDIR/_build_manifests/install/dist_bin. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23837/diff/3-4/
Comment on attachment 8681471 [details]
MozReview Request: Bug 1216817 - Part 4: Add --enable-artifact-builds and MOZ_ARTIFACT_BUILDS. r=glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/23839/diff/3-4/
Attachment #8691739 - Flags: review?(mh+mozilla)
Comment on attachment 8691739 [details]
MozReview Request: Bug 1216817 - Part 5: Run |mach artifact install| automatically when asked. r?glandium

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/26167/diff/2-3/
Attachment #8691739 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8691739 [details]
MozReview Request: Bug 1216817 - Part 5: Run |mach artifact install| automatically when asked. r?glandium

https://reviewboard.mozilla.org/r/26167/#review25991

::: python/mozbuild/mozbuild/config_status.py:216
(Diff revision 3)
> +        os.chdir(topsrcdir) 

trailing whitespace.
https://hg.mozilla.org/integration/fx-team/rev/2f95996c84cfce8687880a05f80943b2ce8a9a21
Bug 1216817 - Part 1: Add install_callback to artifacts. r=gps

https://hg.mozilla.org/integration/fx-team/rev/c37ff20166fe7bdcb108bc193db32d6b88e8ca3a
Bug 1216817 - Part 2: Narrow distdir to bindir in artifacts. r=gps

https://hg.mozilla.org/integration/fx-team/rev/b602d16fa13bde9932e71d14c72132dd1b39489d
Bug 1216817 - Part 3: Add files from artifacts to $OBJDIR/_build_manifests/install/dist_bin. r=glandium

https://hg.mozilla.org/integration/fx-team/rev/90b7e77b7f67f1ae0c9394d9e6db7c869a39011b
Bug 1216817 - Part 4: Add --enable-artifact-builds and MOZ_ARTIFACT_BUILDS. r=glandium

https://hg.mozilla.org/integration/fx-team/rev/e9e84784daf92455a07a563864284ed6047eb50b
Bug 1216817 - Part 5: Run |mach artifact install| automatically when asked. r=glandium
https://hg.mozilla.org/integration/fx-team/rev/ebaa931f095af094dbccca6c760cf923e2211257
Bug 1216817 - Follow-up: Fix "KeyError: uMOZ_ARTIFACT_BUILDS" in config.status. r=bustage
This has busted B2GDroid builds like this:

ValueError: Don't know how to compute android:versionCode for CPU arch armeabi-v7a and min SDK 11
ValueError: Don't know how to compute android:versionCode for CPU arch armeabi-v7a and min SDK 11
AndroidManifest.xml:2: error: Error: String types not allowed (at 'versionCode' with value '').
gmake[5]: *** [.aapt.deps] Error 1
gmake[5]: *** Deleting file `.aapt.deps'
gmake[5]: *** Waiting for unfinished jobs....
gmake[4]: *** [mobile/android/base/libs] Error 2
gmake[3]: *** [libs] Error 2
gmake[2]: *** [default] Error 2
gmake[1]: *** [realbuild] Error 2
gmake: *** [build] Error 2
Return code: 2
'mach build' did not run successfully. Please check log for errors.
Running post_fatal callback... 

https://treeherder.mozilla.org/logviewer.html#?job_id=6379062&repo=fx-team

Investigating with glandium's help.
Flags: needinfo?(nalexander)
Looks like rnewman was at fault and not this bug. Thanks to glandium for help in figuring it out :)
Flags: needinfo?(nalexander)
I don't think my needinfo is still needed.
Flags: needinfo?(gps)
Flags: needinfo?(mhoye)
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.