Closed Bug 844654 Opened 11 years ago Closed 11 years ago

Move MODULE to moz.build

Categories

(Firefox Build System :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla22

People

(Reporter: gps, Assigned: mshal)

References

(Blocks 1 open bug)

Details

Attachments

(4 files)

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...
No longer blocks: 818246
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
Blocks: 850380
joey or mshal: Would either of you care to tackle this?
Over to mshal.
Assignee: nobody → mshal
Attachment #726816 - Flags: review?(gps)
Generated mostly automatically from mozbuild-migrate, except for a few conditional cases and the rules.mk change.
Attachment #726819 - Flags: review?(gps)
Attachment #726816 - Flags: review?(gps) → review+
Is there a reason you decided to migrate it instead of just removing it per the other bug?
(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 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+
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
I was thinking about self-reviewing this. But, I'm kinda hoping khuey can give a quick rubber stamp.
Attachment #726976 - Flags: review?(khuey)
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+
Attachment #727036 - Flags: review?(gps)
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+
Depends on: 853242
Depends on: 853245
Depends on: 853253
Blocks: 869425
Pushed by frgrahl@gmx.net:
https://hg.mozilla.org/comm-central/rev/420e272f6ff2
Part 2: Move MODULE to moz.build; rs=gps
https://hg.mozilla.org/comm-central/rev/6f17c39578f4
Part 3: Remove now empty Makefile.in files; rs=khuey
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.