903321 - Running configure with no configure.in change overwrites some files that shouldn't be overwritten

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Mike Hommey [:glandium]]

Namely, at least $objdir/Makefile and $objdir/js/src/Makefile. There's also media/webrtc and media/mtransport Makefiles, but that comes from gyp, so that would be a separate bug.

This makes us rebuild all of js, and a lot of tests that link against js_static.

Comment 1

[Mike Hommey [:glandium]]

js/src/Makefile is due to a combination of bug 812265 and bug 874543.
Makefile is due to http://hg.mozilla.org/mozilla-central/file/e33c2011643e/client.mk#l334

Comment 2

[Mike Hommey [:glandium]]

Attachment: Don't update backend.RecursiveMakeBackend.built when no backend file changed

This is the lowest hanging fruit to work around the issue. I'm pretty sure there's a way to get rid of the Makefile dependency on backend.RecursiveMakeBackend.built, but as it's tricky, i'd rather have something that works fine for 99% of the builds than something that misbehaves in many cases.

As for $objdir/Makefile, I'd rather not change it either, for the same reasons. There's no file that depends on it anyways, so it's less a problem than $objdir/js/src/Makefile.

As I'm not familiar with the code and how it's supposed to be eventually with more backends, here's my rationale:
- I don't see why not track the number of generated files for any backend.
- I can see why a backend wouldn't care about giving stats about the number of files (like, if there's only one)

As an unrelated note, I don't see why consume_finished runs after backend.$backend.built is touched.

Comment 3

[Mike Hommey [:glandium]]

This gets me from 1099 files rebuilt to 858.

Comment 4

[Mike Hommey [:glandium]]

Attachment: Don't update backend.RecursiveMakeBackend.built when no backend file changed

With typo fixed.

Comment 5

[Mike Hommey [:glandium]]

Attachment: Don't update backend.RecursiveMakeBackend.built when no backend file changed

touch backend.RecursiveMake.build when running config.status from make, so that we avoid config.status running when doing make -C subdir when the python scripts have changed (which happen with this patch)

I don't think this makes things worse for make -C other-subdir.

Comment 6

[Mike Hommey [:glandium]]

Attachment: Don't update backend.RecursiveMakeBackend.built when no backend file changed

Previous iteration forgot to create the file when it doesn't already exist, which was caught by unit tests.

Comment 7

[Gregory Szorc [:gps]]

Comment on attachment 788160 [details] [diff] [review]
Don't update backend.RecursiveMakeBackend.built when no backend file changed

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

How does moving the touch of RecursiveMakeBackend.built from Python into rules.mk make things better?

Comment 8

[Mike Hommey [:glandium]]

(In reply to Gregory Szorc [:gps] from comment #7)
> Comment on attachment 788160 [details] [diff] [review]
> Don't update backend.RecursiveMakeBackend.built when no backend file changed
> 
> Review of attachment 788160 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> How does moving the touch of RecursiveMakeBackend.built from Python into
> rules.mk make things better?

Because configure always runs python, not make.

Comment 9

[Gregory Szorc [:gps]]

Comment on attachment 788160 [details] [diff] [review]
Don't update backend.RecursiveMakeBackend.built when no backend file changed

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

We talked it over in IRC. Root Makefiles have dependency on RecursiveMakeBackend.built which invalidate all local targets since Makefile depends on RecursiveMakeBackend.built. This is a big deal for js.

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/714f45588ae8