Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(566)

Issue 973493003: Fixed version of https://codereview.chromium.org/748793002 (Closed)

Created:
5 years, 9 months ago by bradn
Modified:
5 years, 9 months ago
CC:
gyp-developer_googlegroups.com
Base URL:
https://chromium.googlesource.com/external/gyp.git@master
Target Ref:
refs/heads/master
Project:
gyp
Visibility:
Public.

Description

Fixed version of https://codereview.chromium.org/748793002 The original CL falsely added the inherited project path to the hashables of each child target even if the path wasn't unique. Not only was this undesired behavior and essentially a bug, but it also created an issue for very large projects like bling. The time it took to generate the project files went up from ~2min to over 20min. See https://code.google.com/p/chromium/issues/detail?id=447801 for more info. This CL fixes this behavior. Relanding in git repo. Original review: https://codereview.chromium.org/851103007 BUG=447801 R=mark@chromium.org, jsartisohn@google.com Committed: https://chromium.googlesource.com/external/gyp/+/002ebe4420a3b1c5731f604426393e845a4ca2f3

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+158 lines, -0 lines) Patch
M pylib/gyp/xcodeproj_file.py View 1 chunk +45 lines, -0 lines 0 comments Download
A test/mac/gyptest-identical-name.py View 1 chunk +45 lines, -0 lines 0 comments Download
A test/mac/identical-name/proxy/proxy.cc View 1 chunk +2 lines, -0 lines 0 comments Download
A test/mac/identical-name/proxy/proxy.gyp View 1 chunk +9 lines, -0 lines 0 comments Download
A test/mac/identical-name/proxy/testlib/testlib.cc View 1 chunk +2 lines, -0 lines 0 comments Download
A test/mac/identical-name/proxy/testlib/testlib.gyp View 1 chunk +8 lines, -0 lines 0 comments Download
A test/mac/identical-name/test.gyp View 1 chunk +11 lines, -0 lines 0 comments Download
A test/mac/identical-name/test.gypi View 1 chunk +7 lines, -0 lines 0 comments Download
A test/mac/identical-name/test-should-fail.gyp View 1 chunk +10 lines, -0 lines 0 comments Download
A test/mac/identical-name/testlib/main.cc View 1 chunk +3 lines, -0 lines 0 comments Download
A test/mac/identical-name/testlib/testlib.gyp View 1 chunk +14 lines, -0 lines 0 comments Download
A test/mac/identical-name/testlib/void.cc View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
bradn
5 years, 9 months ago (2015-03-02 19:43:08 UTC) #1
Mark Mentovai
LGTM
5 years, 9 months ago (2015-03-02 19:56:29 UTC) #2
bradn
Committed patchset #1 (id:1) manually as 002ebe4420a3b1c5731f604426393e845a4ca2f3.
5 years, 9 months ago (2015-03-02 19:57:06 UTC) #3
natalie
On 2015/03/02 19:57:06, bradn wrote: > Committed patchset #1 (id:1) manually as > 002ebe4420a3b1c5731f604426393e845a4ca2f3. This ...
5 years, 9 months ago (2015-03-02 23:21:30 UTC) #4
Mark Mentovai
Johannes, can you fix this up?
5 years, 9 months ago (2015-03-02 23:45:40 UTC) #5
jsartisohn
5 years, 9 months ago (2015-03-03 11:24:53 UTC) #6
Message was sent while issue was closed.
On 2015/03/02 23:45:40, Mark Mentovai wrote:
> Johannes, can you fix this up?

Sorry, don't know how this happened, I saw you fixed it already. Thanks.
Is there a way to run something like presubmits for a cl. It's like coding into
the dark sometimes, especially because of all the multiplatform stuff.
Again, I'm super sorry for all the trouble. This CL lay dormant on my machine
for over a month. :|

Powered by Google App Engine
This is Rietveld 408576698