-
Notifications
You must be signed in to change notification settings - Fork 458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix regression resulting in projects existing product refs being altered #429
Fix regression resulting in projects existing product refs being altered #429
Conversation
Buddybuild uses xcodeproj against thousands of xcode projects, and there are a few we’ve noticed issues with as a result of this commit: 36ad554 Fixes issue #409 The problem is that xcodeproj was actually significantly altering the structure of a project if it already has a product_ref_group defined and it wasn’t part of the “Products” main group. This would resultantly lead to fatal failures when we attempt to build the project.
Thanks! Could you please add a test and a CHANGELOG entry? |
Ah, nice one - I've seen a few error reports around this CocoaPods/CocoaPods#5847 I think is one still open - thanks! |
Will do guys. :) |
I've pushed the tests. |
|
||
# Projects can have product_ref_groups that are not listed in the main_groups["Products"] | ||
if root_object.product_ref_group.nil? | ||
root_object.product_ref_group = root_object.main_group['Products'] || root_object.main_group.new_group('Products') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe root_object.product_ref_group ||= root_object.main_group['Products'] || root_object.main_group.new_group('Products')
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That works as well. I don't really have ties to either way.
I'll push a change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. :)
-Rashin
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with CHANGELOG.md update
@@ -8,8 +8,9 @@ | |||
|
|||
##### Bug Fixes | |||
|
|||
* None. | |||
|
|||
* Fix regression resulting in projecs existing product ref groups being altered |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add two spaces to the end of the changelog entry (i.e * Fix regression resulting in projecs existing product ref groups being altered
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Thanks! |
Buddybuild uses xcodeproj against thousands of xcode projects, and there are a few we’ve noticed issues with as a result of this commit:
36ad554 Fixes issue #409
The problem is that xcodeproj was actually significantly altering the structure of a project if it already has a product_ref_group defined and it wasn’t a “Products” main group.
This would resultantly lead to fatal failures when we attempt to build the project.