Last Comment Bug 762358 - .mozconfig changes don't cause configure to run
: .mozconfig changes don't cause configure to run
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla33
Assigned To: Mike Hommey [:glandium]
:
Mentors:
: 837221 879258 884026 (view as bug list)
Depends on: 837846 1030717 1031132 1031180 1031291 1033958
Blocks: 998104 Woodduck 715048 832112 1195331
  Show dependency treegraph
 
Reported: 2012-06-06 21:23 PDT by Randell Jesup [:jesup]
Modified: 2015-08-17 07:58 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
fixed


Attachments
Don't create a .mozconfig.mk file, and re-run configure when .mozconfig changed (4.39 KB, patch)
2012-06-07 03:13 PDT, Mike Hommey [:glandium]
ted: review+
Details | Diff | Review
Re-run configure when mozconfig changed in a significant way (37.03 KB, patch)
2014-06-26 04:14 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
Re-run configure when mozconfig changed in a significant way (46.71 KB, patch)
2014-06-27 03:41 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
Re-run configure when mozconfig changed in a significant way (48.52 KB, patch)
2014-06-27 04:33 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
Re-run configure when mozconfig changed in a significant way (49.17 KB, patch)
2014-06-27 07:10 PDT, Mike Hommey [:glandium]
no flags Details | Diff | Review
Re-run configure when mozconfig changed in a significant way (49.72 KB, patch)
2014-06-27 20:53 PDT, Mike Hommey [:glandium]
gps: feedback+
Details | Diff | Review
Re-run configure when mozconfig changed in a significant way (58.27 KB, patch)
2014-07-01 17:03 PDT, Mike Hommey [:glandium]
gps: review+
Details | Diff | Review
Re-run configure when mozconfig changed in a significant way (61.22 KB, patch)
2014-07-02 00:19 PDT, Mike Hommey [:glandium]
gps: review+
Details | Diff | Review

Description Randell Jesup [:jesup] 2012-06-06 21:23:32 PDT
I assume this is a regression - if you change/touch .mozconfig, configure doesn't get run.  Very confusing why turning off a build option didn't turn it off
Comment 1 Ed Morley [:emorley] 2012-06-07 02:06:28 PDT
This was done intentionally by bug 715048.

See the similar (but for comm-central) bug 731985 comment 12 for Ted's comments on this.
Comment 2 Nicholas Nethercote [:njn] 2012-06-07 02:33:42 PDT
It's a feature, not a bug :)  See bug 731985 comment 10.
Comment 3 Mike Hommey [:glandium] 2012-06-07 02:37:36 PDT
Note, we could actually entirely get rid of the problem by *not* creating a .mozconfig.mk, and "parse" .mozconfig instead. We could then re-establish the dependency that makes configure be run when .mozconfig changes.
Comment 4 Mike Hommey [:glandium] 2012-06-07 03:13:38 PDT
Created attachment 630908 [details] [diff] [review]
Don't create a .mozconfig.mk file, and re-run configure when .mozconfig changed

I was thinking about something along these lines in comment 3. I don't know how much impact this has on windows, but compared to the whole build, it's probably negligible.
<none>
Comment 5 Mike Hommey [:glandium] 2012-06-07 03:24:36 PDT
(In reply to Mike Hommey [:glandium] from comment #4)
> Created attachment 630908 [details] [diff] [review]
> Don't create a .mozconfig.mk file, and re-run configure when .mozconfig
> changed
> 
> I was thinking about something along these lines in comment 3. I don't know
> how much impact this has on windows, but compared to the whole build, it's
> probably negligible.

Actually, it doesn't have any impact since build/autoconf/mozconfig2client-mk was already run every time anyway.
Comment 6 Gregory Szorc [:gps] 2012-06-07 03:27:46 PDT
Comment on attachment 630908 [details] [diff] [review]
Don't create a .mozconfig.mk file, and re-run configure when .mozconfig changed

Should you remove the hgignore and gitignore entries as well? I'm not sure how this would impact people using multiple mozconfig files, however (I can't remember how all that works).
Comment 7 Mike Hommey [:glandium] 2012-06-07 03:36:06 PDT
(In reply to Gregory Szorc [:gps] from comment #6)
> Should you remove the hgignore and gitignore entries as well? 

It would be tempting to do that, but that would expose the file previously created, with the risk of people adding it somehow in the tree.
Comment 8 Ted Mielczarek [:ted.mielczarek] 2012-06-18 11:43:43 PDT
Comment on attachment 630908 [details] [diff] [review]
Don't create a .mozconfig.mk file, and re-run configure when .mozconfig changed

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

::: client.mk
@@ +99,5 @@
>  
> +
> +endef
> +
> +$(eval $(subst ||,$(CR),$(shell $(TOPSRCDIR)/$(MOZCONFIG_LOADER) $(TOPSRCDIR) 2> $(TOPSRCDIR)/.mozconfig.out | sed 's/$$/||/')))

As I mentioned on IRC, a comment here explaining the subst/sed stuff would be useful. This line is a bit hard to read without knowing what it does.
Comment 10 Ed Morley [:emorley] 2012-06-20 02:24:07 PDT
https://hg.mozilla.org/mozilla-central/rev/aa9532ebf77d
Comment 11 Timothy B. Terriberry (:derf) 2012-11-30 16:31:26 PST
So, with a recent (revision 114573) checkout of m-c, changing .mozconfig is still not causing configure to run.
Comment 12 Gregory Szorc [:gps] 2012-11-30 16:42:00 PST
I can confirm this, even without using mach :)

1) make -f client.mk
2) Wait to get past configure stage then CTRL+c
3) touch .mozconfig
4) make -f client.mk
5) configure not executed
Comment 13 Ed Morley [:emorley] 2012-11-30 16:50:28 PST
This was intentionally done in bug 715048 aiui, see bug 715048 comment 2.
Comment 14 Ed Morley [:emorley] 2012-11-30 16:51:19 PST
(In reply to Ed Morley [UTC+0; email:edmorley@moco] from comment #13)
> This was intentionally done in bug 715048 aiui, see bug 715048 comment 2.

Oh sorry read patch landing dates backwards, that bug preceeded this.
Comment 15 Timothy B. Terriberry (:derf) 2012-11-30 20:17:41 PST
Even better, changing the CC=... path in my .mozconfig and touching configure.in _did_ cause it to re-run configure... with the old CC value (which didn't work, as that binary no longer existed).
Comment 16 Mike Hommey [:glandium] 2012-12-01 00:29:17 PST
(In reply to Timothy B. Terriberry (:derf) from comment #15)
> Even better, changing the CC=... path in my .mozconfig and touching
> configure.in _did_ cause it to re-run configure... with the old CC value
> (which didn't work, as that binary no longer existed).

Yeah, that's because of config.cache. :-/
Comment 17 Mike Hommey [:glandium] 2012-12-05 00:39:52 PST
(In reply to Timothy B. Terriberry (:derf) from comment #15)
> Even better, changing the CC=... path in my .mozconfig and touching
> configure.in _did_ cause it to re-run configure... with the old CC value
> (which didn't work, as that binary no longer existed).

Note that if you're building on mac, this could be because of bug 818092
Comment 18 Timothy B. Terriberry (:derf) 2012-12-05 07:07:36 PST
(In reply to Mike Hommey [:glandium] from comment #17)
> Note that if you're building on mac, this could be because of bug 818092

This was on Linux.
Comment 19 :Ehsan Akhgari (busy, don't ask for review please) 2013-02-01 12:33:21 PST
*** Bug 837221 has been marked as a duplicate of this bug. ***
Comment 20 Ted Mielczarek [:ted.mielczarek] 2013-06-04 07:20:59 PDT
*** Bug 879258 has been marked as a duplicate of this bug. ***
Comment 21 Ted Mielczarek [:ted.mielczarek] 2013-06-17 16:12:14 PDT
*** Bug 884026 has been marked as a duplicate of this bug. ***
Comment 22 Mike Hommey [:glandium] 2014-06-26 04:14:19 PDT
Created attachment 8446456 [details] [diff] [review]
Re-run configure when mozconfig changed in a significant way

This adds a format option to mach environment and uses it in client.mk to
create a .mozconfig.json in the objdir, containing all the relevant data
from mozconfig. If the mozconfig doesn't change in a way that alters that
data, we still skip configure.

At the same time, use mach environment in place of mozconfig2configure and
mozconfig2client-mk, which makes us now have only one mozconfig reader.

Finally, in the mozconfig reader, keep track of environment variables (as
opposed to shell variables), so that changes such as a variable that was
exported that is not exported anymore is spotted. At the opposite, in order
for irrelevant environment variable changes not to incur in re-running
configure, only a set of environment variables are stored when they are
unmodified. Otherwise, changes such as using a different terminal window,
or even rebooting, would trigger reconfigures.

This doesn't address the config.cache problem, which is a separate problem.

As a side effect, this kind of implements bug 998104, at least partially.

Only tested locally on Linux.
Comment 23 Mike Hommey [:glandium] 2014-06-26 04:16:41 PDT
https://tbpl.mozilla.org/?tree=Try&rev=e04264b0dec8
Comment 24 Mike Hommey [:glandium] 2014-06-26 04:21:33 PDT
Comment on attachment 8446456 [details] [diff] [review]
Re-run configure when mozconfig changed in a significant way

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

::: client.mk
@@ +350,5 @@
>    $(NULL)
>  
> +CREATE_MOZCONFIG_JSON = $(shell $(MAKE) --no-print-directory -f $(TOPSRCDIR)/client.mk $(OBJDIR)/.mozconfig.json CREATE_MOZCONFIG_JSON=1)
> +$(OBJDIR)/.mozconfig.json: $(call mkdir_deps,$(OBJDIR)) $(if $(CREATE_MOZCONFIG_JSON),FORCE)
> +	$(PYTHON) $(TOPSRCDIR)/mach environment --format=json -o $@

Missing a @ here. Fixed locally while editing the bzexport message, I thought it would pick the updated patch, I was wrong.
Comment 25 Mike Hommey [:glandium] 2014-06-26 04:40:09 PDT
sigh... this is failing on try because of this:

mach and the build system store shared state in a common directory on the
filesystem. The following directory will be created:

  /Users/cltbld/.mozbuild

If you would like to use a different directory, hit CTRL+c and set the
MOZBUILD_STATE_PATH environment variable to the directory you would like to
use and re-run mach. For this change to take effect forever, you'll likely
want to export this environment variable from your shell's init scripts.

20 19 18 17 16 15 14 13 12 11 10 9 8 7 6 5 4 3 2 1 0 
Creating default state directory: /Users/cltbld/.mozbuild
Please re-run mach.
Comment 26 Mike Hommey [:glandium] 2014-06-26 05:43:43 PDT
New attempt with bug 1030717 applied:
  https://tbpl.mozilla.org/?tree=Try&rev=c7a212094887
Comment 27 Mike Hommey [:glandium] 2014-06-26 05:44:04 PDT
Comment on attachment 8446456 [details] [diff] [review]
Re-run configure when mozconfig changed in a significant way

There are still rough edges on mac.
Comment 28 Mike Hommey [:glandium] 2014-06-26 06:15:06 PDT
... and windows, where there's something fishy with .mozconfig.mk, and linux, where it fails because of:

Python 2.7 or above (but not Python 3) is required to run mach.
You are running Python 2.6.6
Comment 29 Mike Hommey [:glandium] 2014-06-26 18:19:46 PDT
And I'm going to have to fix bug 832112 at the same time.
Comment 30 Mike Hommey [:glandium] 2014-06-27 03:41:05 PDT
Created attachment 8447094 [details] [diff] [review]
Re-run configure when mozconfig changed in a significant way

This adds a format option to mach environment and uses it in client.mk to
create a .mozconfig.json in the objdir, containing all the relevant data
from mozconfig. If the mozconfig doesn't change in a way that alters that
data, we still skip configure.

At the same time, use mach environment in place of mozconfig2configure and
mozconfig2client-mk, which makes us now have only one mozconfig reader.

Also, in the mozconfig reader, keep track of environment variables (as
opposed to shell variables), so that changes such as a variable that was
exported not being exported anymore is spotted. At the opposite, in order
for irrelevant environment variable changes not to incur in re-running
configure, only a set of environment variables are stored when they are
unmodified. Otherwise, changes such as using a different terminal window,
or even rebooting, would trigger reconfigures.

Finally, make mach environment emit both MOZ_OBJDIR and OBJDIR for
client.mk, and cleanup some objdir-related things in client.mk..
At the same time, make the mozconfig reader take MOZ_OBJDIR from the
environment if it is defined there and not in the mozconfig.

https://tbpl.mozilla.org/?tree=Try&rev=907faff24da7

Note the OSX orange. I don't expect this to take a big interdiff, so I'd
rather have a review of this big chunk early, and will send fixups later.
Comment 31 Mike Hommey [:glandium] 2014-06-27 04:33:50 PDT
Created attachment 8447115 [details] [diff] [review]
Re-run configure when mozconfig changed in a significant way

Turns out it didn't take long to reproduce and find a fix (only a test change):
https://tbpl.mozilla.org/?tree=Try&rev=3f9fb448aeab
Comment 32 Mike Hommey [:glandium] 2014-06-27 07:10:53 PDT
Created attachment 8447170 [details] [diff] [review]
Re-run configure when mozconfig changed in a significant way

This one is good (again, a test-only change):
https://tbpl.mozilla.org/?tree=Try&rev=cc935da8325e

There's a remaining issue building comm-central, though, but don't let that stop you from reviewing :)
Comment 33 Mike Hommey [:glandium] 2014-06-27 20:53:53 PDT
Created attachment 8447573 [details] [diff] [review]
Re-run configure when mozconfig changed in a significant way

https://tbpl.mozilla.org/?tree=Try&rev=3fb4348fa9c4

This is the one. And it doesn't break c-c more than it currently is as far as my testing goes.
Comment 34 Gregory Szorc [:gps] 2014-07-01 14:00:47 PDT
Comment on attachment 8447573 [details] [diff] [review]
Re-run configure when mozconfig changed in a significant way

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

This is essentially r+. Just want to see some minor feedback addressed.

::: python/mozbuild/mozbuild/base.py
@@ +187,5 @@
> +            _config_topobjdir = config_topobjdir
> +            if current_project:
> +                _config_topobjdir = os.path.join(config_topobjdir, current_project)
> +            mozilla_dir = os.path.join(_config_topobjdir, 'mozilla')
> +            if not samepath(topobjdir, _config_topobjdir) \

Given how many bugs we've had with this code in the past, I'd *really* like to see test coverage of this.

::: python/mozbuild/mozbuild/mach_commands.py
@@ +917,5 @@
> +
> +        if output:
> +            from mozbuild.util import FileAvoidWrite
> +            with FileAvoidWrite(output) as out:
> +                return func(out, verbose)

I'm guessing we can't use shell redirection for file writing because we want to preserve mtimes if output hasn't changed?

@@ +1008,5 @@
> +                for key in self.mozconfig['env']['removed'].keys() + \
> +                        self.mozconfig['vars']['removed'].keys():
> +                    print("unset %s" % key, file=out)
> +
> +    def _environment_json(self, out, verbose):

I can't remember who, but someone was recently asking about machine readable output for this. I'm glad we have it!

::: python/mozbuild/mozbuild/mozconfig.py
@@ +259,5 @@
> +                if vars_before[key] != vars_after[key]:
> +                    changed['modified'][key] = (
> +                        vars_before[key], vars_after[key])
> +                elif key in self.ENVIRONMENT_VARIABLES:
> +                    changed['unmodified'][key] = vars_after[key]

It's not entirely obvious why the ENVIRONMENT_VARIABLES whitelist was added. Please add a comment here.

::: python/mozbuild/mozbuild/test/test_base.py
@@ +36,5 @@
>      def setUp(self):
>          self._old_cwd = os.getcwd()
>          self._old_env = dict(os.environ)
>          os.environ.pop('MOZCONFIG', None)
> +        os.environ.pop('MOZ_OBJDIR', None)

There are a bunch of unittest.skip properties in this file. Could you try enabling these tests after making this change? See also bug 853954.

http://dxr.mozilla.org/mozilla-central/source/python/mozbuild/mozbuild/test/test_base.py#74
Comment 35 Mike Hommey [:glandium] 2014-07-01 15:42:28 PDT
> ::: python/mozbuild/mozbuild/mach_commands.py
> @@ +917,5 @@
> > +
> > +        if output:
> > +            from mozbuild.util import FileAvoidWrite
> > +            with FileAvoidWrite(output) as out:
> > +                return func(out, verbose)
> 
> I'm guessing we can't use shell redirection for file writing because we want
> to preserve mtimes if output hasn't changed?

Yes. Will add a comment.
Comment 36 Mike Hommey [:glandium] 2014-07-01 15:44:05 PDT
(In reply to Gregory Szorc [:gps] from comment #34)
> I can't remember who, but someone was recently asking about machine readable
> output for this. I'm glad we have it!

I guess you are thinking of bug 998104.
Comment 37 Mike Hommey [:glandium] 2014-07-01 17:03:00 PDT
Created attachment 8449068 [details] [diff] [review]
Re-run configure when mozconfig changed in a significant way

This adds a format option to mach environment and uses it in client.mk to
create a .mozconfig.json in the objdir, containing all the relevant data
from mozconfig. If the mozconfig doesn't change in a way that alters that
data, we still skip configure.

At the same time, use mach environment in place of mozconfig2configure and
mozconfig2client-mk, which makes us now have only one mozconfig reader.

Also, in the mozconfig reader, keep track of environment variables (as
opposed to shell variables), so that changes such as a variable that was
exported not being exported anymore is spotted. At the opposite, in order
for irrelevant environment variable changes not to incur in re-running
configure, only a set of environment variables are stored when they are
unmodified. Otherwise, changes such as using a different terminal window,
or even rebooting, would trigger reconfigures.

Finally, make mach environment emit both MOZ_OBJDIR and OBJDIR for
client.mk, and cleanup some objdir-related things in client.mk..
At the same time, make the mozconfig reader take MOZ_OBJDIR from the
environment if it is defined there and not in the mozconfig.
Comment 38 Mike Hommey [:glandium] 2014-07-01 17:06:04 PDT
https://tbpl.mozilla.org/?tree=Try&rev=7dfd58d8ef8a
Comment 39 Gregory Szorc [:gps] 2014-07-01 17:06:51 PDT
Comment on attachment 8449068 [details] [diff] [review]
Re-run configure when mozconfig changed in a significant way

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

Intradiff looks sane.
Comment 40 Mike Hommey [:glandium] 2014-07-02 00:19:20 PDT
Created attachment 8449211 [details] [diff] [review]
Re-run configure when mozconfig changed in a significant way

Turns out I had to revert a part of the last interdiff because of universal builds on c-c, and modify that code slightly to accomodate.

After much fight with the tests you made me enable, I think this is it.

https://tbpl.mozilla.org/?tree=Try&rev=b5b3e73fd635
Comment 41 Mike Hommey [:glandium] 2014-07-02 00:21:04 PDT
So, for python/mozbuild/mozbuild/base.py, this interdiff:
https://bugzilla.mozilla.org/attachment.cgi?oldid=8447573&action=interdiff&newid=8449211&headers=1
is more readable than the one comparing the last two patches.
Comment 42 Gregory Szorc [:gps] 2014-07-02 14:54:55 PDT
Comment on attachment 8449211 [details] [diff] [review]
Re-run configure when mozconfig changed in a significant way

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

Intradiff looks sane.
Comment 44 Mike Hommey [:glandium] 2014-07-02 16:34:48 PDT
Fixup for linux32 orange:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f14b0641458
Comment 45 Mike Hommey [:glandium] 2014-07-02 17:41:27 PDT
And another fixup for windows orange caused by previous fixup.
https://hg.mozilla.org/integration/mozilla-inbound/rev/56b54954e7fb
Comment 47 :Ehsan Akhgari (busy, don't ask for review please) 2014-07-03 21:19:48 PDT
Thanks so much for fixing this bug!
Comment 48 neil@parkwaycc.co.uk 2014-07-06 16:44:16 PDT
The old mozconfig2configure script used to know both the srcdir and the objdir and didn't have to try very hard to find my mozconfig. It then allowed my mozconfig to detect the objdir so the latter could then automatically tweak the configure options appropriately.

However nobody bothers to tell mach environment where the comm-central srcdir is so the first time you want to build an objdir with this change in you have to either build with the srcdir as your working directory or explicitly specify the location of the mozconfig, which is at least possible on the command line (I wasn't able to find a way of overriding the location of the objdir from the command line).
Comment 49 Mike Hommey [:glandium] 2014-07-06 17:21:35 PDT
comm-central's client.mk is still using mozconfig2client-mk to get the objdir.

Note You need to log in before you can comment on or make changes to this bug.