Closed Bug 903321 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox Build System :: General, defect)

24 Branch
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla26

People

(Reporter: glandium, Assigned: glandium)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 3 obsolete files)

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.
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
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.
Attachment #788045 - Flags: review?(gps)
This gets me from 1099 files rebuilt to 858.
With typo fixed.
Attachment #788085 - Flags: review?(gps)
Attachment #788045 - Attachment is obsolete: true
Attachment #788045 - Flags: review?(gps)
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.
Attachment #788099 - Flags: review?(gps)
Attachment #788085 - Attachment is obsolete: true
Attachment #788085 - Flags: review?(gps)
Previous iteration forgot to create the file when it doesn't already exist, which was caught by unit tests.
Attachment #788160 - Flags: review?(gps)
Attachment #788099 - Attachment is obsolete: true
Attachment #788099 - Flags: review?(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?
(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 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.
Attachment #788160 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/714f45588ae8
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Blocks: 905489
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.