mozmake.py (for .gyp) needs to include .gypi files referenced in the common.mk dependencies it outputs

RESOLVED DUPLICATE of bug 778236

Status

P1
normal
RESOLVED DUPLICATE of bug 778236
6 years ago
9 months ago

People

(Reporter: jesup, Assigned: gps)

Tracking

(Blocks: 1 bug)

Trunk
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [WebRTC], [blocking-webrtc-])

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
See also bug 800847

In mozmake.py, we make a common.mk that adds dependencies on all the .gyp files, but none of the .gypi files included from them.  For example, modifying voice_engine_core.gypi will not cause the Makefiles to be regenerated.
Assignee: nobody → ted
Priority: -- → P1
Whiteboard: [webrtc] → [WebRTC], [blocking-webrtc-]
(Assignee)

Comment 2

5 years ago
I kinda fixed this in bug 778236, but that may not land for a little while. This patch should be trivial, so let's get it in the tree ASAP.
Assignee: ted → gps
Status: NEW → ASSIGNED
(Assignee)

Comment 3

5 years ago
Created attachment 794779 [details] [diff] [review]
Add .gypi files to GYP project dependencies

This seems to do it!
Attachment #794779 - Flags: review?(ted)
(Assignee)

Comment 4

5 years ago
Created attachment 794780 [details]
signaling common.mk after patch

For reference.
Comment on attachment 794779 [details] [diff] [review]
Add .gypi files to GYP project dependencies

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

Can't believe it's that simple, sorry I never fixed that.
Attachment #794779 - Flags: review?(ted) → review+
(Assignee)

Comment 6

5 years ago
Comment on attachment 794779 [details] [diff] [review]
Add .gypi files to GYP project dependencies

This patch doesn't work for media/webrtc/trunk for some reason...
Attachment #794779 - Flags: review+
(Assignee)

Updated

5 years ago
Blocks: 928193
(Assignee)

Comment 7

5 years ago
Created attachment 822003 [details] [diff] [review]
Add .gypi files to GYP project dependencies

An unfortunate side-effect of this change is that the GYP-derived
Makefiles will get updated several times during the course of a
recursive traversal. This is because there is no global state tracking
whether a "backend" generation has been performed yet. Doing this
correctly is very difficult given how GYP is currently integrated with
the build system. If we ever integrated GYP processing into mozbuild
traversal, this should become much easier.
Attachment #822003 - Flags: review?(ted)
Comment on attachment 822003 [details] [diff] [review]
Add .gypi files to GYP project dependencies

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

::: media/webrtc/trunk/tools/gyp/pylib/gyp/generator/mozmake.py
@@ +9,5 @@
>  import os
> +
> +from StringIO import StringIO
> +
> +from mozbuild.makeutil import Makefile

Not that we're ever going to upstream this, but pulling mozbuild modules into gyp sure feels a little weird. I assume for a better integration we'd do the opposite and use gyp as a module instead.

@@ +181,5 @@
> +
> +    makerule = StringIO()
> +    m.dump(makerule)
> +
> +    f.write(COMMON_MK % {'make_rule': makerule.getvalue()})

This doesn't seem like a huge win, honestly, it's more code to get the same effect. None of this code is super clean anyway, so it doesn't strike me as valuable cleanup.

@@ +447,5 @@
>        build_files.add(topsrcdir_path(build_file_))
>  
> +      for included_file in data[build_file]['included_files']:
> +        included_file = gyp.common.UnrelativePath(included_file, build_file)
> +        build_files.add(swapslashes(included_file))

Lame, can't believe I missed this.
Attachment #822003 - Flags: review?(ted) → review+
Sorry for the nearly-month-long review delay, that's unacceptable.
(Assignee)

Updated

5 years ago
Blocks: 941904
(Assignee)

Comment 10

5 years ago
glandium is tackling this as part of integrating GYP into config.status. That should land within the next day or two (cross your fingers).
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 778236

Updated

9 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.