Move MODULE to moz.build

RESOLVED FIXED in mozilla22

Status

()

Core
Build Config
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: gps, Assigned: mshal)

Tracking

(Blocks: 1 bug)

unspecified
mozilla22
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments)

(Reporter)

Description

5 years ago
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...
(Reporter)

Updated

5 years ago
No longer blocks: 818246
Blocks: 847009
(Reporter)

Comment 1

5 years ago
We'll tackle the larger problem of module/library/component definitions later. For now, let's just move MODULE to moz.build files.
Summary: Define libraries/components in moz.build → Move MODULE to moz.build
(Reporter)

Updated

5 years ago
Blocks: 850380
(Reporter)

Comment 2

5 years ago
joey or mshal: Would either of you care to tackle this?
(Reporter)

Comment 3

5 years ago
Over to mshal.
Assignee: nobody → mshal
(Assignee)

Comment 4

5 years ago
Created attachment 726816 [details] [diff] [review]
Part 1: Support MODULE in moz.build
Attachment #726816 - Flags: review?(gps)
(Assignee)

Comment 5

5 years ago
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.
Attachment #726819 - Flags: review?(gps)
(Reporter)

Updated

5 years ago
Attachment #726816 - Flags: review?(gps) → review+
Is there a reason you decided to migrate it instead of just removing it per the other bug?
(Assignee)

Comment 7

5 years ago
(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.
(Reporter)

Comment 8

5 years ago
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.
Attachment #726819 - Flags: review?(gps) → review+
(Reporter)

Comment 9

5 years ago
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.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla22
(Reporter)

Comment 10

5 years ago
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.
Attachment #726976 - Flags: review?(khuey)
This, unfortunately, needed a clobber. https://hg.mozilla.org/integration/mozilla-inbound/rev/fdf8bb76c9ac
(Reporter)

Comment 12

5 years ago
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 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 ...
Attachment #726976 - Flags: review?(khuey) → review+
(Reporter)

Comment 14

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/5b4e1e56815e
(Assignee)

Comment 15

5 years ago
Created attachment 727036 [details] [diff] [review]
Part 4: Move MODULE to moz.build (c-c)
Attachment #727036 - Flags: review?(gps)
(Reporter)

Comment 16

5 years ago
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!
Attachment #727036 - Flags: review?(gps) → review+
https://hg.mozilla.org/mozilla-central/rev/76538d572881
https://hg.mozilla.org/mozilla-central/rev/97e443115162
https://hg.mozilla.org/mozilla-central/rev/fdf8bb76c9ac
https://hg.mozilla.org/mozilla-central/rev/5b4e1e56815e
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 853242

Updated

5 years ago
Depends on: 853245

Updated

5 years ago
Depends on: 853253
Blocks: 869425
You need to log in before you can comment on or make changes to this bug.