"ac_add_options --enable-geckodriver" breaks artifact builds

RESOLVED FIXED in Firefox 56

Status

RESOLVED FIXED
2 years ago
11 months ago

People

(Reporter: whimboo, Assigned: chmanchester)

Tracking

(Blocks: 1 bug)

54 Branch
mozilla56

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(2 attachments)

Using "ac_add_options --enable-geckodriver" in the mozconfig breaks artifact builds in a way that no-one can determine what's wrong:

 0:21.41 Traceback (most recent call last):
 0:21.41   File "/Volumes/data/code/gecko/configure.py", line 124, in <module>
 0:21.41     sys.exit(main(sys.argv))
 0:21.41   File "/Volumes/data/code/gecko/configure.py", line 34, in main
 0:21.41     return config_status(config)
 0:21.41   File "/Volumes/data/code/gecko/configure.py", line 119, in config_status
 0:21.41     return config_status(args=[], **encode(sanitized_config, encoding))
 0:21.41   File "/Volumes/data/code/gecko/python/mozbuild/mozbuild/config_status.py", line 150, in config_status
 0:21.41     the_backend.consume(definitions)
 0:21.41   File "/Volumes/data/code/gecko/python/mozbuild/mozbuild/backend/base.py", line 126, in consume
 0:21.41     for obj in objs:
 0:21.41   File "/Volumes/data/code/gecko/python/mozbuild/mozbuild/frontend/emitter.py", line 183, in emit
 0:21.41     objs = list(emitfn(out))
 0:21.41   File "/Volumes/data/code/gecko/python/mozbuild/mozbuild/frontend/emitter.py", line 1026, in emit_from_context
 0:21.41     for obj in self._handle_linkables(context, passthru, generated_files):
 0:21.41   File "/Volumes/data/code/gecko/python/mozbuild/mozbuild/frontend/emitter.py", line 563, in _handle_linkables
 0:21.41     self._binaries[program] = cls(context, program, cargo_file)
 0:21.41   File "/Volumes/data/code/gecko/python/mozbuild/mozbuild/frontend/data.py", line 449, in __init__
 0:21.41     cargo_dir = cargo_output_directory(context, self.TARGET_SUBST_VAR)
 0:21.41   File "/Volumes/data/code/gecko/python/mozbuild/mozbuild/frontend/data.py", line 430, in cargo_output_directory
 0:21.41     return mozpath.join(context.config.substs[target_var], rust_build_kind)
 0:21.41 KeyError: u'RUST_TARGET'
 0:21.72 *** Fix above errors and then restart with               "/Applications/Xcode.app/Contents/Developer/usr/bin/make -f client.mk build"
 0:21.72 make[2]: *** [configure] Error 1
 0:21.73 make[1]: *** [/Volumes/data/code/gecko/obj/debug/Makefile] Error 2
 0:21.73 make: *** [build] Error 2

If we do not support artifact builds with geckodriver enabled, we should at least make it clear with a clean failure message.
Flags: needinfo?(ato)
RustLibrary in data.py has an explicit check for COMPILE_ENVIRONMENT; it may be worth investigating if a similar check would make sense for BaseRustProgram.  Might have to check to see if anything uses the paths defined in BaseRustProgram.
(In reply to Nathan Froyd [:froydnj] from comment #1)
> RustLibrary in data.py has an explicit check for COMPILE_ENVIRONMENT; it may
> be worth investigating if a similar check would make sense for
> BaseRustProgram.  Might have to check to see if anything uses the paths
> defined in BaseRustProgram.

Making a similar check for BaseRustProgram certainly makes the KeyError:
u'RUST_TARGET' to away:

> diff --git a/python/mozbuild/mozbuild/frontend/data.py b/python/mozbuild/mozbuild/frontend/data.py
> index f9328a426ddc..1ea8f75327c6 100644
> --- a/python/mozbuild/mozbuild/frontend/data.py
> +++ b/python/mozbuild/mozbuild/frontend/data.py
> @@ -445,10 +445,12 @@ class BaseRustProgram(ContextDerived):
>      def __init__(self, context, name, cargo_file):
>          ContextDerived.__init__(self, context)
>          self.name = name
> -        self.cargo_file = cargo_file
> -        cargo_dir = cargo_output_directory(context, self.TARGET_SUBST_VAR)
> -        exe_file = '%s%s' % (name, context.config.substs.get(self.SUFFIX_VAR, ''))
> -        self.location = mozpath.join(cargo_dir, exe_file)
> +
> +        if context.config.substs.get('COMPILE_ENVIRONMENT'):
> +            self.cargo_file = cargo_file
> +            cargo_dir = cargo_output_directory(context, self.TARGET_SUBST_VAR)
> +            exe_file = '%s%s' % (name, context.config.substs.get(self.SUFFIX_VAR, ''))
> +            self.location = mozpath.join(cargo_dir, exe_file)
>  
>  
>  class RustProgram(BaseRustProgram):

However, I find it is confusing that no geckodriver binary or error is
emitted when --enable-geckodriver is switched on for artifact builds.  What
is the approach we have taken in similar cases in the past?

Should it maybe return an error, and if so where, when asked to build
geckodriver in a non-compilation environment?
Flags: needinfo?(ato)
(In reply to Andreas Tolfsen ‹:ato› from comment #2)
> However, I find it is confusing that no geckodriver binary or error is
> emitted when --enable-geckodriver is switched on for artifact builds.  What
> is the approach we have taken in similar cases in the past?

I don't think we have prior art here, because --enable-geckodriver is a little unusual in building a specific program.  But --enable-artifact builds are designed to require zero compilation, so not producing geckodriver is expected.

> Should it maybe return an error, and if so where, when asked to build
> geckodriver in a non-compilation environment?

If --enable-geckodriver also toggles prefs or other settings, I think that means --enable-geckodriver should be an error with artifact builds.  Unless we could make --enable-geckodriver artifact builds look for artifacts from a normal --enable-geckodriver build?  Chris would be the person to ask about that.
Maybe we could have something like `mach build geckodriver` for those users who want to work with artifact builds?
(In reply to Henrik Skupin (:whimboo) from comment #4)
> Maybe we could have something like `mach build geckodriver` for those users
> who want to work with artifact builds?

We already have `./mach build testing/geckodriver`, but I’m not sure it is
possible to override the check for no compile environment in that case.
That might be. For my artifact build config it runs too quick:

 0:00.40 /usr/bin/make -C /Volumes/data/code/gecko/obj/debug -j4 -s backend
 0:00.55 /usr/bin/make -j4 -s testing
 0:00.75 /usr/bin/make -j4 -s testing/geckodriver
 0:00.95 Your build was successful!

So I think we abort without any failure too :/
(Assignee)

Comment 7

2 years ago
(In reply to Nathan Froyd [:froydnj] from comment #3)
> (In reply to Andreas Tolfsen ‹:ato› from comment #2)
> > However, I find it is confusing that no geckodriver binary or error is
> > emitted when --enable-geckodriver is switched on for artifact builds.  What
> > is the approach we have taken in similar cases in the past?
> 
> I don't think we have prior art here, because --enable-geckodriver is a
> little unusual in building a specific program.  But --enable-artifact builds
> are designed to require zero compilation, so not producing geckodriver is
> expected.

We should either make the option itself conditional with "when='--enable-compile-environment'" in the option definition or add it as a input to the option so we can error out with a better error message.

> 
> > Should it maybe return an error, and if so where, when asked to build
> > geckodriver in a non-compilation environment?
> 
> If --enable-geckodriver also toggles prefs or other settings, I think that
> means --enable-geckodriver should be an error with artifact builds.  Unless
> we could make --enable-geckodriver artifact builds look for artifacts from a
> normal --enable-geckodriver build?  Chris would be the person to ask about
> that.

We should be able to treat this like we do other test support binaries, by putting them in the test package definitions here: http://searchfox.org/mozilla-central/rev/20963d7269b1b14d455f47bc0260d0653015bf84/python/mozbuild/mozbuild/action/test_archive.py#34 and adding "test_artifact_patterns" in python/mozbuild/mozbuild/artifacts.py where appropriate. The only caveat here will be that the local artifact build will need to be on a platform where we build geckodriver in automation.
(Assignee)

Updated

2 years ago
Assignee: nobody → cmanchester
Blocks: 1278699
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

2 years ago
mozreview-review
Comment on attachment 8879183 [details]
Bug 1370535 - Provide a useful error message when passing --enable-geckodriver in artifact bulds.

https://reviewboard.mozilla.org/r/150518/#review155194
Attachment #8879183 - Flags: review?(nfroyd) → review+

Comment 11

2 years ago
mozreview-review
Comment on attachment 8879182 [details]
Bug 1370535 - Install the geckodriver binary in artifact builds when available.

https://reviewboard.mozilla.org/r/150516/#review155214

::: python/mozbuild/mozbuild/artifacts.py:134
(Diff revision 1)
>          ('bin/BadCertServer', ('bin', 'bin')),
>          ('bin/GenerateOCSPResponse', ('bin', 'bin')),
>          ('bin/OCSPStaplingServer', ('bin', 'bin')),
>          ('bin/certutil', ('bin', 'bin')),
>          ('bin/fileid', ('bin', 'bin')),
> +        ('bin/geckodriver', ('bin', 'bin')),

By my reading, this just means that `geckodriver` will be copied if it exists, not that things will error if it does not exist, correct?
Attachment #8879182 - Flags: review?(nfroyd) → review+

Comment 12

2 years ago
Pushed by cmanchester@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c58982c971e6
Install the geckodriver binary in artifact builds when available. r=froydnj
https://hg.mozilla.org/integration/autoland/rev/d4d573d44528
Provide a useful error message when passing --enable-geckodriver in artifact bulds. r=froydnj

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c58982c971e6
https://hg.mozilla.org/mozilla-central/rev/d4d573d44528
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
So I run artifact builds, but i cannot find the binary of geckodriver anywhere under obj/dist/. At which place do we exactly download the binary?
Flags: needinfo?(cmanchester)
(Assignee)

Comment 15

2 years ago
It will be under $objdir/dist/bin
Flags: needinfo?(cmanchester)
(In reply to Chris Manchester (:chmanchester) from comment #15)
> It will be under $objdir/dist/bin

I cannot find it there. So what does 'downloaded when available' mean? Don't we build it by default for all builds now?
Flags: needinfo?(cmanchester)
(Assignee)

Comment 17

2 years ago
(In reply to Henrik Skupin (:whimboo) [partly available 07/10 -07/14] from comment #16)
> (In reply to Chris Manchester (:chmanchester) from comment #15)
> > It will be under $objdir/dist/bin
> 
> I cannot find it there. So what does 'downloaded when available' mean? Don't
> we build it by default for all builds now?

It means if the upstream automation build we're installing from has a geckodriver binary we'll install it, but we can't if it doesn't.
Flags: needinfo?(cmanchester)
(In reply to Chris Manchester (:chmanchester) from comment #17)
> It means if the upstream automation build we're installing from has a
> geckodriver binary we'll install it, but we can't if it doesn't.

Andreas, aren't we creating a geckodriver binary all the time now? I wonder why it cannot be found.
Flags: needinfo?(ato)
(In reply to Henrik Skupin (:whimboo) [away 07/21 - 07/31] from
comment #18)

> (In reply to Chris Manchester (:chmanchester) from comment #17)
>
> > It means if the upstream automation build we're installing from
> > has a geckodriver binary we'll install it, but we can't if it
> > doesn't.
>
> Andreas, aren't we creating a geckodriver binary all the time
> now? I wonder why it cannot be found.

We are on environments that don’t cross-compile.  If you’re on
macOS, I’m afraid that’s a cross-compilation environment.
Flags: needinfo?(ato)
(In reply to Andreas Tolfsen ‹:ato› from comment #19)
> We are on environments that don’t cross-compile.  If you’re on
> macOS, I’m afraid that’s a cross-compilation environment.

Oh, darn. I am on that platform, and totally forgot about this situation. That perfectly explains it. Thanks.
FWIW I can’t find the bug right now (and I’m a bit rushed) but
it’s very likely a silly bug with not setting up the Rust compile
environment correctly that prevents us from compiling geckodriver
for a macOS target on a Linux host system.  I random guess would be
that we’re not seeing all the necessary output variables.

Updated

11 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.