Closed Bug 1220000 Opened 4 years ago Closed 4 years ago

GENERATED_FILES dep files should be added to EXTRA_EXPORT_MDDEPEND_DIRS

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox45 fixed, b2g-v2.5 fixed)

RESOLVED FIXED
mozilla45
Tracking Status
firefox45 --- fixed
b2g-v2.5 --- fixed

People

(Reporter: chmanchester, Unassigned)

Details

Attachments

(1 file)

They aren't included right now. I have:

$ touch browser/themes/preprocess-tab-svgs.py
 ~/m-c 
$ REBUILD_CHECK=1 ./mach build browser/themes/
 0:00.10 /usr/bin/make -C /home/chris/m-c/obj-dbg -j8 -s backend.RecursiveMakeBackend
 0:00.22 /usr/bin/make -C browser/themes -j8 -s
 0:00.25 Rebuilding tab-selected-end.svg because /home/chris/m-c/browser/themes/preprocess-tab-svgs.py changed
 0:00.26 Rebuilding tab-selected-start.svg because /home/chris/m-c/browser/themes/preprocess-tab-svgs.py changed
 0:00.64 /usr/bin/make -C browser/app -j8 -s
 0:00.98 Your build was successful!

but

$ touch browser/themes/linux/linuxShared.inc 
 ~/m-c 
$ REBUILD_CHECK=1 ./mach build browser/themes/
 0:00.09 /usr/bin/make -C /home/chris/m-c/obj-dbg -j8 -s backend.RecursiveMakeBackend
 0:00.22 /usr/bin/make -C browser/themes -j8 -s
 0:00.59 /usr/bin/make -C browser/app -j8 -s
 0:00.98 Your build was successful!

Or maybe we should get rid of EXTRA_EXPORT_MDDEPEND_DIRS and do this a simpler way, because I don't see it used anywhere.
Bug 1220000 - Use EXTRA_EXPORT_MDDEPEND_DIRS so GENERATED_FILES dep files are included properly. r=glandium
Attachment #8680979 - Flags: review?(mh+mozilla)
Comment on attachment 8680979 [details]
MozReview Request: Bug 1220000 - Unconditionally include EXTRA_MDDEPEND_FILES so callers get what they expect. r=glandium

Bug 1220000 - Use EXTRA_EXPORT_MDDEPEND_DIRS so GENERATED_FILES dep files are included properly. r=glandium
Comment on attachment 8680979 [details]
MozReview Request: Bug 1220000 - Unconditionally include EXTRA_MDDEPEND_FILES so callers get what they expect. r=glandium

https://reviewboard.mozilla.org/r/23713/#review21429

Looking around, it turns out:
- Nothing uses EXTRA_EXPORT_MDDEPEND_FILES anymore (it was added and used for dom bindings, until bug 928195)
- EXTRA_MDDEPEND_FILES is only used by GeneratedFiles and manually used by accessible/xpcom/Makefile.in
- The use in accessible/xpcom/Makefile.in is actually wrong, since EXTRA_MDDEPEND_FILES are currently *not* included for the export tier.
- The uses for GeneratedFiles are wrong for the same reason, and this is what you're trying to fix.

I'd say we should just kill EXTRA_EXPORT_MDDEPEND_FILES and include EXTRA_MDDEPEND_FILES unconditionally.
Attachment #8680979 - Flags: review?(mh+mozilla)
Comment on attachment 8680979 [details]
MozReview Request: Bug 1220000 - Unconditionally include EXTRA_MDDEPEND_FILES so callers get what they expect. r=glandium

Bug 1220000 - Unconditionally include EXTRA_MDDEPEND_FILES so callers get what they expect. r=glandium

GENERATED_FILES and accessible/xpcom/Makefile.in add to EXTRA_MDDEPEND_FILES, but for
targets that run during export. Export doesn't include EXTRA_MDDEPEND_FILES, so none
of them is ending up with correct dependencies. The EXTRA_EXPORT_MDDEPEND_FILES variable
could be used for this purpose, but given the circumstances this variable is removed,
and EXTRA_MDDEPEND_FILES is included unconditionally to this mechanism.
Attachment #8680979 - Attachment description: MozReview Request: Bug 1220000 - Use EXTRA_EXPORT_MDDEPEND_DIRS so GENERATED_FILES dep files are included properly. r=glandium → MozReview Request: Bug 1220000 - Unconditionally include EXTRA_MDDEPEND_FILES so callers get what they expect. r=glandium
Attachment #8680979 - Flags: review?(mh+mozilla)
Comment on attachment 8680979 [details]
MozReview Request: Bug 1220000 - Unconditionally include EXTRA_MDDEPEND_FILES so callers get what they expect. r=glandium

https://reviewboard.mozilla.org/r/23713/#review21455
Attachment #8680979 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/ca648290f7de
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.