Closed Bug 1543982 Opened 6 years ago Closed 6 years ago

./mach build for android rebuilds too much

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(firefox68 fixed)

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: emilio, Assigned: nalexander)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

STR:

With the following mozconfig:

mk_add_options AUTOCLOBBER=1
ac_add_options --enable-application=mobile/android
ac_add_options --with-android-ndk="/home/emilio/.mozbuild/android-ndk-r17b"
ac_add_options --enable-application=mobile/android
CC="/home/emilio/.mozbuild/clang/bin/clang"
CXX="/home/emilio/.mozbuild/clang/bin/clang++"

Run:

$ ./mach build
$ ./mach android install-geckoview_example

Tweak a single cpp file, then:

$ ./mach build

Expected:

  • ./mach build rebuilds only my C++ file, links libxul, and I can install my updated geckoview example app again.

Actual:

  • `./mach build rebuilds large parts of the js engine, gfx, dom/media, mfbt, security, xpcom, ...

If we're supposed to use gradle unconditionally (haven't tried that yet, still building...) after the first ./mach build, then it should bail out rather than reconfigure and rebuild the world if there's a pre-existing build. Or something...

So what you don't mention in comment 0 but kind of mention in comment 1 is that running mach build the second time reruns configure. Which would show two possible different problems:

  • that configure reruns
  • that rerunning configure changes something that triggers large parts of C++ to be rebuilt

For the latter, can you set REBUILD_CHECK=1 in the environment and check what it says it's rebuilding for?

BTW, is this the kind of report that should be linked to mach-busted?

(In reply to Mike Hommey [:glandium] from comment #2)

So what you don't mention in comment 0 but kind of mention in comment 1 is that running mach build the second time reruns configure. Which would show two possible different problems:

  • that configure reruns

Even ./mach android install-geckoview_example triggers reconfigure, though it doesn't trigger a full rebuild.

./mach build; ./mach build also triggers reconfigure. It says:

/home/emilio/src/moz/gecko-3/obj-arm-unknown-linux-androideabi/config.status is out of date with respect to /home/emilio/src/moz/gecko-3/obj-arm-unknown-linux-androideabi/.mozconfig.json
[ configure snip ]
 0:45.55 Rebuilding host_nsinstall.o because ../config/autoconf.mk changed
 0:45.67 Rebuilding host_pathsub.o because ../config/autoconf.mk changed
 0:46.31 Rebuilding nsinstall_real because host_nsinstall.o, host_pathsub.o, ../config/autoconf.mk changed
 0:46.89 Rebuilding .deps/android_apks.stub because FORCE was removed

I'll attach a whole build log with REBUILD_CHECK=1

BTW, is this the kind of report that should be linked to mach-busted?

Not sure, it kind of seemed to fit, but if you think it doesn't please remove it :)

machBuildGeneratedAndroidCodeAndResources seems to reconfigure stuff when ran from mach build, but not when ran separately as ./mach build mobile/android/base/generated_android_code_and_resources.

$  diff config.status.old  obj-arm-unknown-linux-androideabi/config.status                                                                                                                 
779c779
<     'PATH': '/home/emilio/.mozbuild/moz-git-tools:/home/emilio/.mozbuild/git-cinnabar:/home/emilio/.npm-packages/bin:/home/emilio/.cargo/bin:/home/emilio/.local/bin:/home/emilio/bin:/usr/share/Modules/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/home/emilio/src/moz/arcanist/bin:/home/emilio/.tools:/home/emilio/src/ig/depot_tools:/home/emilio/src/ig/icecc-chromium:/home/emilio/.cargo/bin:/home/emilio/.mozbuild/clang/bin:/home/emilio/.mozbuild/cbindgen:/home/emilio/.mozbuild/nasm:/home/emilio/.cargo/bin',
---
>     'PATH': '/home/emilio/.mozbuild/moz-git-tools:/home/emilio/.mozbuild/git-cinnabar:/home/emilio/.npm-packages/bin:/home/emilio/.cargo/bin:/home/emilio/.local/bin:/home/emilio/bin:/usr/share/Modules/bin:/usr/local/bin:/usr/local/sbin:/usr/bin:/usr/sbin:/home/emilio/src/moz/arcanist/bin:/home/emilio/.tools:/home/emilio/src/ig/depot_tools:/home/emilio/src/ig/icecc-chromium:/home/emilio/.cargo/bin:/home/emilio/.mozbuild/clang/bin:/home/emilio/.mozbuild/cbindgen:/home/emilio/.mozbuild/nasm:/home/emilio/.cargo/bin:/home/emilio/.mozbuild/clang/bin:/home/emilio/.mozbuild/cbindgen:/home/emilio/.mozbuild/nasm:/home/emilio/.cargo/bin',

So when mach calls into mach, we append redundant PATH entries which seem to cause reconfigure?

Though there's also:

$ diff .mozconfig.json.old obj-arm-unknown-linux-androideabi/.mozconfig.json                                                                                                               
12c12
<           "/usr/libexec/icecc/bin/clang", 
---
>           " /home/emilio/.mozbuild/clang/bin/clang -std=gnu99 --target=arm-linux-androideabi", 
16c16
<           "/usr/libexec/icecc/bin/clang++", 
---
>           " /home/emilio/.mozbuild/clang/bin/clang++ --target=arm-linux-androideabi", 
40c40
<           "([0]=\"/home/emilio/src/moz/gecko-3/python/mozbuild/mozbuild/action/dump_env.py\" [1]=\"/usr/bin/python2.7\" [2]=\"/home/emilio/src/moz/gecko-3/.mozconfig\" [3]=\"/home/emilio/src/moz/gecko-3\")"
---
>           "([0]=\"/home/emilio/src/moz/gecko-3/python/mozbuild/mozbuild/action/dump_env.py\" [1]=\"/home/emilio/src/moz/gecko-3/obj-arm-unknown-linux-androideabi/_virtualenvs/init/bin/python\" [2]=\"/home/emilio/src/moz/gecko-3/.mozconfig\" [3]=\"/home/emilio/src/moz/gecko-3\")"

So when I call mach from the top level directory, my regular export CC="/usr/libexec/icecc/bin/clang" which I have in my profile file is noted in .mozconfig.json, but when called from mach the env var that mach adds comes into play.

I think we should skip the mach build tasks when called from mach. Is there an easy way to detect we're being called from mach?

I think I know how to fix it.

Assignee: nobody → emilio

(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)

I think we should skip the mach build tasks when called from mach. Is there an easy way to detect we're being called from mach?

No, you can't do this. There's a lot going on here, and additional changes in the works (eventually) -- see https://bugzilla.mozilla.org/show_bug.cgi?id=1509539 and related work. Please r? me directly for any changes here; there are many, many things at play.

(In reply to Nick Alexander :nalexander [he/him] from comment #9)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)
No, you can't do this. There's a lot going on here, and additional changes in the works (eventually) -- see https://bugzilla.mozilla.org/show_bug.cgi?id=1509539 and related work. Please r? me directly for any changes here; there are many, many things at play.

I wonder why not though? My patch only avoids calling ./mach build from ./mach build (effectively). Calling ./mach build something/more/specific from ./mach build doesn't make much sense to me...

But of course you know way more about the setup here, so any better solution would definitely be appreciated!

I also realized that this doesn't fix it completely. It does fix the "./mach build continuously reconfigures" situation, but ./mach android install-geckoview_example ends up reconfiguring (and thus I end up having to rebuild the world again next time).

This seems to have gotten worse the last few weeks and is now taking up hours of my time waiting for it to constantly rebuild. Consecutive ./mach builds without even touching a file in between triggers a 15 minute rebuild on my machine.

Is there a fix in the works?

(In reply to Jamie Nicol [:jnicol] from comment #12)

This seems to have gotten worse the last few weeks and is now taking up hours of my time waiting for it to constantly rebuild. Consecutive ./mach builds without even touching a file in between triggers a 15 minute rebuild on my machine.

Sorry, I see now that I lost Emilio's patch in my queue!

Is there a fix in the works?

I'll look at this today. Sorry for the big delay!

So when I call mach from the top level directory, my regular export CC="/usr/libexec/icecc/bin/clang" which I have in my profile file is noted in .mozconfig.json, but when called from mach the env var that mach adds comes into play.

I can repro this locally, and what Emilio says is exactly correct. The CC from the toplevel env is changed inside mach build, which means the recursive mach build ... sees a different CC (and other things), triggering reconfigure.

For example, I see:

toplevel: ('CC', '/Users/nalexander/.mozbuild/clang2/bin/clang')

:machBuildGeneratedAndroidCodeAndResources> ('CC', ' /Users/nalexander/.mozbuild/clang/bin/clang -std=gnu99 --target=arm-linux-androideabi')

That means that recursive mach build invocations will never work, which makes me sad, but here we are. I'm still investigating the right way to address this.

The inline comment explains what is happening here. The issue is that
client.mk is setting MOZ_OBJDIR (and autoconf.mk is setting CC/CXX and
others) as part of mach build, which means that recursively invoking
mach build sees a different environment, and that triggers
reconfigure.

In some situations we can avoid this by recognizing that the
environment has changed and setting it back to what it was at the time
of mach build before client.mk adjusts it.

This commit avoids moz.build tasks when we're already within mach build.

This is belt-and-braces: from within mach build, we want the main
moz.build dependency graph to arrange for the Gradle invocations to be
in the right state. It's only in other situations, like mach android ... or invocation from Android Studio, that we want Gradle to arrange
to be in the right state vis. moz.build.

Depends on D30425

emilio: testing of these patches would be appreciated. The second is basically what you had. The first is very, very subtle. I believe that both are necessary. For testing, just go about your regular business; if it works, that's good enough to land.

jnicol: you too, if you can.

For future readers: to test this, you can use an artifact build and MOZ_OBJDIR. Set MOZ_OBJDIR in your mozconfig, and then use a relative path in the environment like:

env MOZ_OBJDIR=../relative/path ./mach build

configure should kick in and you'll have an absolute path in any recursive mach build invocations, due to client.mk setting variables here.

You can see this happening even without recursive mach build by changing the relative path. It'll be ignored, but still will cause a reconfigure 'cuz the .mozconfig.json file changes to say that it's being ignored!

env MOZ_OBJDIR=../relative/path/garbage ./mach build

Anyway, if these work in the wild I think they're the way to go. They definitely handle artifact and compiled builds for me, with mach build (the second patch) and mach android build-geckoview_example (the first patch). Very tricky!

Flags: needinfo?(jnicol)
Flags: needinfo?(emilio)

The patches seem to work perfectly for me, but I'll give them a more thorough testing tomorrow. Thanks Nick you're a lifesaver!

Flags: needinfo?(jnicol)

(In reply to Jamie Nicol [:jnicol] from comment #18)

The patches seem to work perfectly for me, but I'll give them a more thorough testing tomorrow. Thanks Nick you're a lifesaver!

Great! Can you confirm that you have one of MOZ_OBJDIR/CC/CXX/LD/LDFLAGS or similar set in your environment all the time? A dump of env might help.

I did test it locally (with CC and CXX at least, though with a slightly different environment to the one of comment 0, since I'm not in the office), and this seems to work for me. Thanks so much Nick! :)

Flags: needinfo?(emilio)

(In reply to Nick Alexander :nalexander [he/him] from comment #19)

(In reply to Jamie Nicol [:jnicol] from comment #18)
Can you confirm that you have one of MOZ_OBJDIR/CC/CXX/LD/LDFLAGS or similar set in your environment all the time? A dump of env might help.

I have CC and CXX set to mozbuild's clang in my .mozconfig, like in emilio's setup in comment 0. I also have --with-ccache and CCACHE_PREFIX=icecc if that's relevant. None of the other environment variables are set in my shell by default or in my mozconfig.

Assignee: emilio → nalexander
Status: NEW → ASSIGNED
Pushed by nalexander@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/55b7de7850be Part 1: Avoid re-configuring from within Gradle. r=emilio https://hg.mozilla.org/integration/autoland/rev/17c76d081a9a Part 2: Don't invoke `mach build ...` recursively. r=emilio
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Attachment #9057885 - Attachment is obsolete: false
Attachment #9057885 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: