Closed Bug 1296798 Opened 4 years ago Closed 4 years ago

upgrade GYP from upstream repo

Categories

(Firefox Build System :: General, defect)

defect
Not set

Tracking

(firefox51 fixed)

RESOLVED FIXED
mozilla51
Tracking Status
firefox51 --- fixed

People

(Reporter: myk, Assigned: myk)

Details

Attachments

(4 files)

In order to support projects that use newer features of GYP, we should upgrade it from its upstream repo.
Attachment #8783130 - Flags: review?(ted)
Attachment #8783131 - Flags: review?(ted)
Attachment #8783132 - Flags: review?(ted)
Attachment #8783133 - Flags: review?(ted)
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 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!
Attachment #8783130 - Flags: review?(ted) → review+
Comment on attachment 8783131 [details]
specify root_targets

https://reviewboard.mozilla.org/r/73078/#review71480
Attachment #8783131 - Flags: review?(ted) → review+
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
Attachment #8783132 - Flags: review?(ted) → review+
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.
Attachment #8783133 - Flags: review?(ted) → review+
(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).
Pushed by myk@mozilla.com:
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
(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
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.