Fix errors when using mutually recursive build settings #727
Conversation
|
Looks like that test still fails 😬 |
|
I know, I'm working on it 😃 |
revolter
changed the title
revolter
changed the title
|
@segiddins, It's ready for review. |
| @@ -6,6 +6,8 @@ module Object | |||
| # {PBXProject} or a {PBXNativeTarget}. | |||
| # | |||
| class XCBuildConfiguration < AbstractObject | |||
| MUTUAL_RECURSION_SENTINEL = 'xcodeproj.mutual_recursion_sentinel'.freeze | |||
There was a problem hiding this comment.
Might actually be more performant to make this = Object.new.freeze. Also, I think it can be a private_constant?
There was a problem hiding this comment.
- It doesn't work with
Object.new.freeze(actually::Object.new), becauseStringspecific methods are called on it, and so it crashes. - TIL about
private_constant.
| # | ||
| # @return [String] The value of the build setting | ||
| # | ||
| def resolve_build_setting(key, root_target = nil) | ||
| def resolve_build_setting(key, root_target = nil, previous_key = nil) |
There was a problem hiding this comment.
is it always sufficient to have a single previous key, or do we need a list of all of them?
There was a problem hiding this comment.
Do you mean for cases like this:
Xcodeproj/spec/project/object/build_configuration_spec.rb
Lines 75 to 76 in 12cca46
?
Case in which both mutually_recursive and mutually_recursive_nested are resolved to nil:
Xcodeproj/spec/project/object/build_configuration_spec.rb
Lines 122 to 123 in 12cca46
There was a problem hiding this comment.
Or if you have another specific case in mind, I could make a test for it and see if it works.
| @@ -168,9 +181,19 @@ def resolve_variable_substitution(key, value, root_target) | |||
| when key | |||
| # to prevent infinite recursion | |||
| nil | |||
| when previous_key | |||
| # to prevent infinite recursion; we don't return nil as for the self recursion because it | |||
There was a problem hiding this comment.
missing some words at the end?
There was a problem hiding this comment.
Dang, and I forgot what exactly I wanted to say. I pushed an update.
|
This was not rebased and therefore the CHANGELOG entry was in the wrong spot. Will fix for release of 1.15.0 |
|
Shipped with 1.15.0! |
|
Sorry, and thanks for releasing! |
|
Could you edit the GitHub release notes too? |
|
Done. |
No description provided.