840568 - mach creates additional objdir as obj-@CONFIG_GUESS@

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Connor [:mconnor]]

Current mozconfig:

mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-@CONFIG_GUESS@
mk_add_options MOZ_MAKE_FLAGS="-j4 -s"
ac_add_options --enable-application=browser
export MOZ_TELEMETRY_REPORTING=1

I end up with srcdir/obj-@CONFIG_GUESS@ and srcdir/obj-i686-pc-mingw32 when I run ./mach build.  This makes Bug 840567 matter to me, because ./mach clobber stops after the first one.

Comment 1

[Mike Hommey [:glandium]]

Try replacing @CONFIG_GUESS@ with $($topsrcdir/build/autoconf/config.guess) in your mozconfig.

Comment 2

[Mike Hommey [:glandium]]

That being said, considering @CONFIG_GUESS@ is a documented and widespread construct for MOZ_OBJDIR, I'd say we should add support for it in MozconfigLoader.read_mozconfig.

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Attachment: Hack support for @CONFIG_GUESS@

It seemed a little better to hack this support into mozconfig.py, but this is the first time I've touched or seen this code, so I might be missing something.  However, it does prevent that obj-@CONFIG_GUESS@ directory being created and makes "mach clobber" work as expected for me.

Note that I didn't wrap the code in a check for Windows, as I figured that if other platforms don't have the problem then it means the variable has already been resolved, so the check for @CONFIG_GUESS@ will fail.  But see above, I really don't know this code...

Comment 4

[Gregory Szorc [:gps]]

Comment on attachment 719826 [details] [diff] [review]
Hack support for @CONFIG_GUESS@

Review of attachment 719826 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozbuild/mozbuild/mozconfig.py
@@ +254,5 @@
> +                        cg = subprocess.check_output(args, cwd=self.topsrcdir, env=env)
> +                    except subprocess.CalledProcessError as e:
> +                        raise MozconfigLoadException(path, MOZCONFIG_BAD_EXIT_CODE,
> +                                                     e.output.splitlines())
> +                    value = value.replace("@CONFIG_GUESS@", cg.strip())

We already have code in base.py to obtain the config.guess value. While I plan to refactor base.py to make it suck less, I think refactoring the config.guess invocation into a module level function and then consuming it in both locations is better than duplicating the logic here.

Comment 5

[Mike Hommey [:glandium]]

Comment on attachment 719826 [details] [diff] [review]
Hack support for @CONFIG_GUESS@

Review of attachment 719826 [details] [diff] [review]:
-----------------------------------------------------------------

::: python/mozbuild/mozbuild/mozconfig.py
@@ +254,5 @@
> +                        cg = subprocess.check_output(args, cwd=self.topsrcdir, env=env)
> +                    except subprocess.CalledProcessError as e:
> +                        raise MozconfigLoadException(path, MOZCONFIG_BAD_EXIT_CODE,
> +                                                     e.output.splitlines())
> +                    value = value.replace("@CONFIG_GUESS@", cg.strip())

Besides what gps said, you should probably handle @CONFIG_GUESS@ like @TOPSRCDIR@ is handled.

Comment 6

[Mark Hammond [:markh] [:mhammond]]

Attachment: Do the magic directly in base.py

(In reply to Gregory Szorc [:gps] from comment #4)
> We already have code in base.py to obtain the config.guess value. While I
> plan to refactor base.py to make it suck less, I think refactoring the
> config.guess invocation into a module level function and then consuming it
> in both locations is better than duplicating the logic here.

It looks like a simple hack in base.py is all that is needed, as that is what loads the config.  Does this look better?

Comment 7

[Mark Hammond [:markh] [:mhammond]]

Attachment: all config_guess in one spot

Apologies for the spam, but I think this version is much nicer.  It will mean that if someone arranges for a blank MOZ_OBJDIR in their mozconfig it will be treated as 'obj-@CONFIG_GUESS@', but I'm calling that a feature :)  Obviously changing that would be trivial if requested.

Comment 8

[Gregory Szorc [:gps]]

Comment on attachment 720260 [details] [diff] [review]
all config_guess in one spot

Review of attachment 720260 [details] [diff] [review]:
-----------------------------------------------------------------

I love elegant patches like these.

Comment 9

[Mark Hammond [:markh] [:mhammond]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/75fb66838232

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/75fb66838232