Closed Bug 880246 Opened 12 years ago Closed 12 years ago

Move EXTRA_PP_COMPONENTS to moz.build

Categories

(Firefox Build System :: General, defect)

Product:
Firefox Build System
Component:
General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla25

People

(Reporter: joey, Assigned: jarmstrong)

References

(Blocks 1 open bug)

Details

Attachments

(5 files, 5 obsolete files)

move EXTRA_PP_COMPONENTS to moz.build (logic).
5.66 KB, patch
Details | Diff | Splinter Review
move EXTRA_PP_COMPONENTS to mozbuild (file batch #1)
24.17 KB, patch
Details | Diff | Splinter Review
move EXTRA_PP_COMPONENTS to mozbuild (batch #2).
14.54 KB, patch
Details | Diff | Splinter Review
Cleanup for mozbuild EXTRA_PP_COMPONENTS conversion.
5.66 KB, patch
Details | Diff | Splinter Review
move EXTRA_PP_COMPONENTS to mozbuild (file batch #3).
836 bytes, patch
mshal
: review+
Details | Diff | Splinter Review
No description provided.
No longer depends on: 870370, 880245
Blocks: 880254
Blocks: 881446
No longer blocks: 881446
Assignee: nobody → joey
Comment on attachment 760980 [details] [diff] [review] move EXTRA_PP_COMPONENTS to moz.build (logic). Add EXTRA_PP_COMPONENTS as a passthrough variable.
Attachment #760980 - Flags: review?(gps)
Comment on attachment 760980 [details] [diff] [review] move EXTRA_PP_COMPONENTS to moz.build (logic). Review of attachment 760980 [details] [diff] [review]: ----------------------------------------------------------------- ::: python/mozbuild/mozbuild/frontend/sandbox_symbols.py @@ +106,5 @@ > > + 'EXTRA_PP_COMPONENTS': (StrictOrderingOnAppendList, list, [], > + """Javascript XPCOM files. > + > + This variable contains a list of files to preprocess. Please note that these are installed in the /components directory of the distribution.
Attachment #760980 - Flags: review?(gps) → review+
Attachment #760980 - Attachment is obsolete: true
Comment on attachment 761404 [details] [diff] [review] move EXTRA_PP_COMPONENTS to moz.build (logic). Added a doc comment about the destination directory, r=gps carried forward.
No longer blocks: 880254
Comment on attachment 761404 [details] [diff] [review] move EXTRA_PP_COMPONENTS to moz.build (logic). Inbound push: committed changeset 134818:76bbed327768 https://hg.mozilla.org/integration/mozilla-inbound/rev/76bbed327768
Attachment #761701 - Attachment is obsolete: true
Comment on attachment 762258 [details] [diff] [review] move EXTRA_PP_COMPONENTS to mozbuild (file batch #1) First batch of converted files. Try results for b2g and fedora pending. All others passed earlier.
Attachment #762258 - Flags: review?(mshal)
Comment on attachment 762258 [details] [diff] [review] move EXTRA_PP_COMPONENTS to mozbuild (file batch #1) Try job: https://tbpl.mozilla.org/?tree=Try&rev=d11471046362
Comment on attachment 762258 [details] [diff] [review] move EXTRA_PP_COMPONENTS to mozbuild (file batch #1) >diff --git a/toolkit/components/filepicker/moz.build b/toolkit/components/filepicker/moz.build >--- a/toolkit/components/filepicker/moz.build >+++ b/toolkit/components/filepicker/moz.build >@@ -11,8 +11,13 @@ if CONFIG['MOZ_XUL'] and \ > 'nsIFileView.idl', > ] > CPP_SOURCES += [ > 'nsFileView.cpp', > ] > EXTRA_COMPONENTS += [ > 'nsFilePicker.js', > ] >+ >+if CONFIG['MOZ_XUL'] and CONFIG['MOZ_WIDGET_TOOLKIT'] not in ('android', 'cocoa', 'qt', 'os2', 'windows'): >+ EXTRA_PP_COMPONENTS += [ >+ 'nsFilePicker.manifest', >+ ] Can you just re-use the existing conditional block here? Or do we have a bug on file to fix those up after the automated conversion?
Attachment #762258 - Flags: review?(mshal) → review+
Please re-use existing conditional blocks as part of manual porting. There was likely only 1 block in the source Makefile.in before we started moz.build conversion. So if we're lazy and produce multiple conditional blocks, it may be a net regression in terms of readability.
(In reply to Gregory Szorc [:gps] from comment #13) > Please re-use existing conditional blocks as part of manual porting. There > was likely only 1 block in the source Makefile.in before we started > moz.build conversion. So if we're lazy and produce multiple conditional > blocks, it may be a net regression in terms of readability. Umm why would joining two conditionals into a single block when we are able to be a problem ? This was a nit condition from some of the earlier conversions that would have required a followup bug to cleanup.
(In reply to Joey Armstrong [:joey] from comment #14) > (In reply to Gregory Szorc [:gps] from comment #13) > > Please re-use existing conditional blocks as part of manual porting. There > > was likely only 1 block in the source Makefile.in before we started > > moz.build conversion. So if we're lazy and produce multiple conditional > > blocks, it may be a net regression in terms of readability. > > Umm why would joining two conditionals into a single block when we are able > to be a problem ? This was a nit condition from some of the earlier > conversions that would have required a followup bug to cleanup. Oh NM I missed the other conditional [ thought this comment was from mike ]
Attachment #762258 - Attachment is obsolete: true
Comment on attachment 762775 [details] [diff] [review] move EXTRA_PP_COMPONENTS to mozbuild (file batch #1) toolkit/components/filepicker/moz.build was an automated addition not manual but can move the conditional into the other block for the inbound push.
Comment on attachment 762775 [details] [diff] [review] move EXTRA_PP_COMPONENTS to mozbuild (file batch #1) Inbound push: committed changeset 135102:a6d60a556ed4 https://hg.mozilla.org/integration/mozilla-inbound/rev/a6d60a556ed4
Assignee: joey → jarmstrong
Comment on attachment 764346 [details] [diff] [review] move EXTRA_PP_COMPONENTS to mozbuild (batch #2). Conversion patch has baked long enough. Remove DISABLED_ tokens and add EXPORT_PP_COMPONENTS to the bad variable list.
Attachment #764346 - Flags: review?(mshal)
Comment on attachment 764346 [details] [diff] [review] move EXTRA_PP_COMPONENTS to mozbuild (batch #2). Review of attachment 764346 [details] [diff] [review]: ----------------------------------------------------------------- Looks good!
Attachment #764346 - Flags: review?(mshal) → review+
Blocks: 884819
Attachment #764346 - Attachment is obsolete: true
Attachment #765529 - Attachment is obsolete: true
Fixed embedded bug comment, should be cleanup not file batch #2. Last patch reviewed deleted DISABLED_EXTRA_PP_COMPONENTS. Refresh patch and upload with corrected comment. r=mshal carried forward.
Comment on attachment 765538 [details] [diff] [review] Cleanup for mozbuild EXTRA_PP_COMPONENTS conversion. Inbound push: committed changeset 135858:69f37f4cdd2e https://hg.mozilla.org/integration/mozilla-inbound/rev/69f37f4cdd2e
Whiteboard: [leave open]
Comment on attachment 768283 [details] [diff] [review] move EXTRA_PP_COMPONENTS to mozbuild (file batch #3). Fix one last straggler from the cleanup bug.
Attachment #768283 - Flags: review?(mshal)
Attachment #768283 - Flags: review?(mshal) → review+
https://hg.mozilla.org/mozilla-central/rev/ebac633ae469
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: