Closed Bug 870401 Opened 9 years ago Closed 8 years ago

Move EXPORTS[_NAMESPACES] to moz.build files: mfbt/exported_headers.mk

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla28

People

(Reporter: joey, Assigned: bokeefe)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

One more file to convert:

mfbt/exported_headers.mk
EXPORTS_NAMESPACES += mozilla
EXPORTS_mozilla += \
  Assertions.h \
  [ ... ]
No longer blocks: 862608, 864192
This is blocking me from nuking EXPORTS_NAMESPACES from rules.mk in bug 890097. I'll take this one real quick.
Assignee: nobody → gps
Blocks: 890097
Status: NEW → ASSIGNED
Hmm. The file in question is included in the JS build system to facilitate standalone JS builds. Unfortunately, the moz.build reader has smarts preventing include() from working on files outside of the top source directory. Since it needs to do a |include('../../mfbt/<path>')|, it's failing. Hmmm.
Blocks: 892644
(In reply to Gregory Szorc [:gps] from comment #2)
> Hmm. The file in question is included in the JS build system to facilitate
> standalone JS builds. Unfortunately, the moz.build reader has smarts
> preventing include() from working on files outside of the top source
> directory. Since it needs to do a |include('../../mfbt/<path>')|, it's
> failing. Hmmm.

Maybe some --with-external-source-dir= magic would help?
No longer blocks: 890097
Duplicate of this bug: 900530
Blocks: 900530
Returning to the pool.
Assignee: gps → nobody
Status: ASSIGNED → NEW
Attached patch fix-mfbt-exports.patch (obsolete) — Splinter Review
I figured I could take a stab at this.

js/src/configure.in just sets EXTERNAL_SOURCE_DIR to a hardcoded path, so you don't need to remember to specify it in standalone spidermonkey builds. It was hardcoded in js/src/Makefile.in anyway, so it's not any worse off, really.

I figured I'd do the sources as well, while I was touching things, since it was basically the same change.

When I moved the js/src/Makefile.in bits to moz.build, I had to make some minor changes:
- The headers are under JS_STANDALONE now, because otherwise mozbuild was unhappy that files were getting exported twice.
- I switched the sources from MOZ_GLUE_PROGRAM_LDFLAGS to JS_STANDALONE, since the former was empty in the standalone and subset-of-firefox builds, and the comment suggests that it only applies in the standalone case.


Try was happy: https://tbpl.mozilla.org/?tree=Try&rev=ed6e5121ac2f
Assignee: nobody → bokeefe
Status: NEW → ASSIGNED
Attachment #824087 - Flags: review?(mshal)
Comment on attachment 824087 [details] [diff] [review]
fix-mfbt-exports.patch

This looks good! Only suggestion I have is it may make sense to combine sources.mozbuild and exported_headers.mozbuild into a single file - I don't think it buys us anything to keep them separate. Up to you though!
Attachment #824087 - Flags: review?(mshal) → review+
(In reply to Michael Shal [:mshal] from comment #7)
> Comment on attachment 824087 [details] [diff] [review]
> fix-mfbt-exports.patch
> 
> This looks good! Only suggestion I have is it may make sense to combine
> sources.mozbuild and exported_headers.mozbuild into a single file - I don't
> think it buys us anything to keep them separate. Up to you though!

I was thinking the same thing. I moved the IMPL_MFBT define in there as well. I didn't run this through try, but I did check that it still builds locally.
Attachment #824087 - Attachment is obsolete: true
Attachment #825021 - Flags: review?(mshal)
Blocks: 933145
Comment on attachment 825021 [details] [diff] [review]
Fix up MFBT exports/sources

Looks good!
Attachment #825021 - Flags: review?(mshal) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/344508092626
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [qa-]
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.