Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(9)

Issue 6476070: Fixing ID collision if two gyp files share the same name. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years ago by noyau
Modified:
3 years ago
Base URL:
http://gyp.googlecode.com/svn/trunk/pylib/gyp/
Visibility:
Public.

Description

Fixing ID collision if two gyp files share the same name. If a project has one target that has a dependency on two gyp files of the same name at different paths the generation of an xcode project fails as the generated ID collides. The ID for those objects was created by using the name of the gyp file and the parent name. The ID now also includes the name of all the children. Pushed in r1481.

Patch Set 1 #

Patch Set 2 : Alternative Implementation. #

Total comments: 1

Patch Set 3 : Implemented as suggested by Mark. #

Patch Set 4 : Removing unnecessary copy(). #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -5 lines) Patch
M xcodeproj_file.py View 1 2 3 4 chunks +39 lines, -5 lines 1 comment Download

Messages

Total messages: 14
noyau
3 years ago (2012-08-27 15:43:25 UTC) #1
Mark Mentovai
I think it’s a mistake to do this in all cases, because it will cause ...
3 years ago (2012-08-27 15:51:00 UTC) #2
noyau
On 2012/08/27 15:51:00, Mark Mentovai wrote: > I think it’s a mistake to do this ...
3 years ago (2012-08-27 16:44:25 UTC) #3
Mark Mentovai
https://codereview.appspot.com/6476070/diff/3002/xcodeproj_file.py File xcodeproj_file.py (right): https://codereview.appspot.com/6476070/diff/3002/xcodeproj_file.py#newcode442 xcodeproj_file.py:442: child.ComputeIDs(recursive, overwrite, hash.copy(), index) Now this will cause two ...
3 years ago (2012-08-27 17:29:39 UTC) #4
noyau
On 2012/08/27 17:29:39, Mark Mentovai wrote: > https://codereview.appspot.com/6476070/diff/3002/xcodeproj_file.py > File xcodeproj_file.py (right): > > https://codereview.appspot.com/6476070/diff/3002/xcodeproj_file.py#newcode442 ...
3 years ago (2012-08-27 18:19:39 UTC) #5
Mark Mentovai
A duplicate ID is normally an indication of some sort of logic problem in the ...
3 years ago (2012-08-27 18:25:18 UTC) #6
stuartmorgan
On 2012/08/27 18:25:18, Mark Mentovai wrote: > You should scope your fix very narrowly to ...
3 years ago (2012-08-27 19:55:48 UTC) #7
Mark Mentovai
My main concern is to make sure that the hash that children “steal” from their ...
3 years ago (2012-08-27 20:08:59 UTC) #8
noyau
PTAL.
3 years ago (2012-08-28 09:23:06 UTC) #9
Mark Mentovai
This seems good, but now I’m confused, because it seems like at some point we ...
3 years ago (2012-08-28 12:56:22 UTC) #10
noyau
On 2012/08/28 12:56:22, Mark Mentovai wrote: > This seems good, but now I’m confused, because ...
3 years ago (2012-08-28 13:21:27 UTC) #11
Mark Mentovai
I see. LGTM.
3 years ago (2012-08-28 13:24:55 UTC) #12
thakis
Why is there no test?
3 years ago (2012-08-29 02:36:39 UTC) #13
noyau
3 years ago (2012-08-30 12:56:14 UTC) #14
On 2012/08/29 02:36:39, thakis wrote:
> Why is there no test?

Because I didn't think about it as I was testing with the issue that caused me
grief in the first place. I'll work on one.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld fd993c8