Last Comment Bug 844654 - Move MODULE to moz.build
: Move MODULE to moz.build
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla22
Assigned To: Michael Shal [:mshal]
:
: Gregory Szorc [:gps]
Mentors:
Depends on: 784841 853242 853245 853253
Blocks: nomakefiles 850380 869425
  Show dependency treegraph
 
Reported: 2013-02-24 13:16 PST by Gregory Szorc [:gps]
Modified: 2013-05-14 16:49 PDT (History)
6 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part 1: Support MODULE in moz.build (2.87 KB, patch)
2013-03-19 11:46 PDT, Michael Shal [:mshal]
gps: review+
Details | Diff | Splinter Review
Part 2: Move MODULE to moz.build (806.85 KB, patch)
2013-03-19 11:47 PDT, Michael Shal [:mshal]
gps: review+
Details | Diff | Splinter Review
Part 3: Remove now empty Makefile.in, v1 (54.17 KB, patch)
2013-03-19 17:03 PDT, Gregory Szorc [:gps]
khuey: review+
Details | Diff | Splinter Review
Part 4: Move MODULE to moz.build (c-c) (108.81 KB, patch)
2013-03-19 20:21 PDT, Michael Shal [:mshal]
gps: review+
Details | Diff | Splinter Review

Description Gregory Szorc [:gps] 2013-02-24 13:16:49 PST
We have a number of variables in Makefile.in related to defining components/libraries:

* MODULE
* MODULE_NAME
* XPIDL_MODULE (typically MODULE)
* GRE_MODULE (unused outside Makefile.in AFAICT)
* EXPORT_LIBRARY
* FORCE_SHARED_LIB
* LIBRARY_NAME
* LIBXUL_LIBRARY

We should first decide if we want to keep things as separate variables or go the function route. We could even do something like:

# Define a new component/library.
lib = library('dom_file', static=True, libxul=True)

# Attach stuff to it!
lib.cpp_sources += ['AsyncHelper.cpp', 'File.cpp']
lib.idls += ['foo.idl', 'bar.idl']
lib.defines += ['FOO', 'BAR']

If we went this light OO route, that would certainly open up the door for easier consolidation of build system definitions into fewer files.

We /may/ want to figure this out before we attempt bug 818246 because IDLs require MODULE or XPIDL_MODULE. I suppose we could always port IDLs straight then figure out the larger picture later...
Comment 1 Gregory Szorc [:gps] 2013-03-12 13:31:00 PDT
We'll tackle the larger problem of module/library/component definitions later. For now, let's just move MODULE to moz.build files.
Comment 2 Gregory Szorc [:gps] 2013-03-13 08:34:04 PDT
joey or mshal: Would either of you care to tackle this?
Comment 3 Gregory Szorc [:gps] 2013-03-14 11:04:13 PDT
Over to mshal.
Comment 4 Michael Shal [:mshal] 2013-03-19 11:46:40 PDT
Created attachment 726816 [details] [diff] [review]
Part 1: Support MODULE in moz.build
Comment 5 Michael Shal [:mshal] 2013-03-19 11:47:57 PDT
Created attachment 726819 [details] [diff] [review]
Part 2: Move MODULE to moz.build

Generated mostly automatically from mozbuild-migrate, except for a few conditional cases and the rules.mk change.
Comment 6 Ted Mielczarek [:ted.mielczarek] 2013-03-19 11:58:42 PDT
Is there a reason you decided to migrate it instead of just removing it per the other bug?
Comment 7 Michael Shal [:mshal] 2013-03-19 12:31:45 PDT
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #6)
> Is there a reason you decided to migrate it instead of just removing it per
> the other bug?

That seemed to be the consensus on IRC, I believe because of concerns that MODULE might be used differently in c-c (and so removing it might break things there), and we weren't entirely sure on the version_win.pl / NetBSD usage. I can attempt a patch there if we just want to go straight for the removal.
Comment 8 Gregory Szorc [:gps] 2013-03-19 13:47:35 PDT
Comment on attachment 726819 [details] [diff] [review]
Part 2: Move MODULE to moz.build

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

rs=gps.

This will break comm-central. Want to submit a patch for them real quick?

::: dom/bluetooth/moz.build
@@ +13,5 @@
>  # See the License for the specific language governing permissions and
>  # limitations under the License.
>  
>  if CONFIG['MOZ_B2G_BT']:
> +    MODULE = 'dom'

Since MODULE doesn't do anything, this probably doesn't need to be inside the conditional. But meh.

::: toolkit/components/filepicker/moz.build
@@ +4,5 @@
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  if CONFIG['MOZ_XUL'] and \
>      CONFIG['MOZ_WIDGET_TOOLKIT'] not in ('android', 'qt', 'os2', 'cocoa', 'windows'):
> +        MODULE = 'filepicker'

Ditto. meh.

::: tools/profiler/moz.build
@@ +3,5 @@
>  # License, v. 2.0. If a copy of the MPL was not distributed with this
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  if CONFIG['MOZ_ENABLE_PROFILER_SPS']:
> +    MODULE = 'profiler'

And ditto meh.
Comment 9 Gregory Szorc [:gps] 2013-03-19 15:33:56 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/76538d572881
https://hg.mozilla.org/integration/mozilla-inbound/rev/97e443115162

Let's try to get the comm-central patches up before the comm-central people descend on us.
Comment 10 Gregory Szorc [:gps] 2013-03-19 17:03:53 PDT
Created attachment 726976 [details] [diff] [review]
Part 3: Remove now empty Makefile.in, v1

I was thinking about self-reviewing this. But, I'm kinda hoping khuey can give a quick rubber stamp.
Comment 11 Joe Drew (not getting mail) 2013-03-19 18:31:13 PDT
This, unfortunately, needed a clobber. https://hg.mozilla.org/integration/mozilla-inbound/rev/fdf8bb76c9ac
Comment 12 Gregory Szorc [:gps] 2013-03-19 18:38:35 PDT
Technically this didn't require a full clobber: it required a config.status run. However, since CLOBBER is the only mechanism available to us, we're left with using a chainsaw to conduct surgery.
Comment 13 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2013-03-19 18:39:43 PDT
Comment on attachment 726976 [details] [diff] [review]
Part 3: Remove now empty Makefile.in, v1

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

I'm just rubberstamping this ...
Comment 15 Michael Shal [:mshal] 2013-03-19 20:21:45 PDT
Created attachment 727036 [details] [diff] [review]
Part 4: Move MODULE to moz.build (c-c)
Comment 16 Gregory Szorc [:gps] 2013-03-19 20:25:58 PDT
Comment on attachment 727036 [details] [diff] [review]
Part 4: Move MODULE to moz.build (c-c)

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

rs=gps

I effectively scanned this. Most of it looks to be done automatically. Fine by me!

Note You need to log in before you can comment on or make changes to this bug.