1296798 - upgrade GYP from upstream repo

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Myk Melez [:myk] [@mykmelez]]

In order to support projects that use newer features of GYP, we should upgrade it from its upstream repo.

Comment 1

[Myk Melez [:myk] [@mykmelez]]

Attachment: upgrade gyp from upstream

Review commit: https://reviewboard.mozilla.org/r/73076/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/73076/

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Attachment: specify root_targets

Review commit: https://reviewboard.mozilla.org/r/73078/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/73078/

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Attachment: provide PYTHON value to run sub-commands with same Python

Review commit: https://reviewboard.mozilla.org/r/73080/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/73080/

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Attachment: Bug 1296798 - set depth to chrome_src for refs in common.gypi

Review commit: https://reviewboard.mozilla.org/r/73082/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/73082/

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Ted: the first patch upgrades GYP to the current tip of the master branch of https://chromium.googlesource.com/external/gyp.  The next three fix various configuration errors that I encountered after upgrading GYP.

The last patch is the only one that isn't obvious.  After upgrading, reading various GYP files were failing on invocations of ./dir_exists.py.  It turned out that read_from_gyp set |depth| to mozpath.dirname(path), which LoadTargetBuildFile (in GYP's input.py) converted to ".".

So paths in common.gypi that start with <(DEPTH) were being interpreted resolved to the directory containing the GYP file (f.e. media/webrtc/signaling/ in the case of media/webrtc/signaling/signaling.gyp), whereas common.gypi appears to expect them to be interpreted relative to its directory (media/webrtc/trunk/build/).

The last patch addresses this by setting depth to chrome_src unconditionally, which fixes all cases that are using gyp_chromium.  It conflicts with your work in bug 1295937, but that's easy to fix by predicating on !no_chromium.

Incidentally, this change will also allow us to get rid of ./media/libyuv/build/mac/find_sdk.py, which appears to have been introduced to fix an earlier incarnation of this problem that affected media/libyuv/.  (I'm unsure why other directories use gyp.mozbuild weren't affected before this GYP upgrade.  By my reading of the GYP source, both old and new, they should've been as well.)

Comment 6

[Myk Melez [:myk] [@mykmelez]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=40c7e6c941b4

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 8783130 [details]
upgrade gyp from upstream

https://reviewboard.mozilla.org/r/73076/#review71478

rs=me. I think I actually need this for my NSS work anyway, so thanks!

Comment 8

[(not currently active) Ted Mielczarek]

Comment on attachment 8783131 [details]
specify root_targets

https://reviewboard.mozilla.org/r/73078/#review71480

Comment 9

[(not currently active) Ted Mielczarek]

Comment on attachment 8783132 [details]
provide PYTHON value to run sub-commands with same Python

https://reviewboard.mozilla.org/r/73080/#review71484

Oh, good, I had to work around this in my NSS patch:
https://hg.mozilla.org/try/file/0ed45617b06d/security/moz.build#l23

Comment 10

[(not currently active) Ted Mielczarek]

Comment on attachment 8783133 [details]
Bug 1296798 - set depth to chrome_src for refs in common.gypi

https://reviewboard.mozilla.org/r/73082/#review71486

Yeah, I'll just rebase over this with my patches and make it conditional. I really hate how chromium piled a bunch of hacks on top of a build tool that they wrote internally.

Comment 11

[Myk Melez [:myk] [@mykmelez]]

(In reply to Ted Mielczarek [:ted.mielczarek] from comment #9)
> Comment on attachment 8783132 [details]
> provide PYTHON value to run sub-commands with same Python
> 
> https://reviewboard.mozilla.org/r/73080/#review71484
> 
> Oh, good, I had to work around this in my NSS patch:
> https://hg.mozilla.org/try/file/0ed45617b06d/security/moz.build#l23

This actually just reintroduces the fix from bug 814333, which was lost in the initial patch to upgrade GYP from upstream.  I'm not sure if there are other such downstream changes that we should re-apply.  If so, presumably there aren't any that influence current builds, since the tryserver run seems to have been successful (modulo all the intermittents).

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/738ae6308649
upgrade gyp from upstream; r=ted
https://hg.mozilla.org/integration/mozilla-inbound/rev/667b39ddd0da
specify root_targets; r=ted
https://hg.mozilla.org/integration/mozilla-inbound/rev/3554eb856762
provide PYTHON value to run sub-commands with same Python; r=ted
https://hg.mozilla.org/integration/mozilla-inbound/rev/5de51bab9236
set depth to chrome_src for refs in common.gypi; r=ted

Comment 13

[Myk Melez [:myk] [@mykmelez]]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #11)
> This actually just reintroduces the fix from bug 814333, which was lost in
> the initial patch to upgrade GYP from upstream.  I'm not sure if there are
> other such downstream changes that we should re-apply.  If so, presumably
> there aren't any that influence current builds, since the tryserver run
> seems to have been successful (modulo all the intermittents).

Note also that Mozilla tried to upstream the fix a few years back, but it didn't end up landing:

https://bugs.chromium.org/p/gyp/issues/detail?id=322
https://codereview.chromium.org/12374093

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/738ae6308649
https://hg.mozilla.org/mozilla-central/rev/667b39ddd0da
https://hg.mozilla.org/mozilla-central/rev/3554eb856762
https://hg.mozilla.org/mozilla-central/rev/5de51bab9236