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

Status: RESOLVED DUPLICATE of bug 778236

(Firefox Build System :: General, defect, P1)

Description

[Randell Jesup [:jesup] (needinfo me)]

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.

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

https://tbpl.mozilla.org/php/getParsedLog.php?id=17428030&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=17429168&tree=Mozilla-Inbound

Comment 2

[Gregory Szorc [:gps]]

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.

Comment 3

[Gregory Szorc [:gps]]

Attachment: Add .gypi files to GYP project dependencies

This seems to do it!

Comment 4

[Gregory Szorc [:gps]]

Attachment: signaling common.mk after patch

For reference.

Comment 5

[(not currently active) Ted Mielczarek]

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.

Comment 6

[Gregory Szorc [:gps]]

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...

Comment 7

[Gregory Szorc [:gps]]

Attachment: 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.

Comment 8

[(not currently active) Ted Mielczarek]

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.

Comment 9

[(not currently active) Ted Mielczarek]

Sorry for the nearly-month-long review delay, that's unacceptable.

Comment 10

[Gregory Szorc [:gps]]

glandium is tackling this as part of integrating GYP into config.status. That should land within the next day or two (cross your fingers).