Skip to content
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

Correctly set runtime search paths for OSX unit test bundles #6435

Merged
merged 1 commit into from Feb 10, 2017

Conversation

dnkoutso
Copy link
Contributor

No description provided.

@@ -217,8 +217,9 @@ def generate_ld_runpath_search_paths
ld_runpath_search_paths = ['$(inherited)']
if target.platform.symbolic_name == :osx
ld_runpath_search_paths << "'@executable_path/../Frameworks'"
symbol_type = target.user_targets.map(&:symbol_type).uniq.first
Copy link
Contributor Author

@dnkoutso dnkoutso Jan 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that an aggregate_target integrates more than one user_target? If so, what is the scenario I can use to test this out and what should the expected behavior be?

@dnkoutso
Copy link
Contributor Author

Will need to bump integration specs.

@dnkoutso
Copy link
Contributor Author

/cc @dantoml @benasher44

@benasher44
Copy link
Member

My only thought is maybe abstract targets can have multiple user targets? I haven't verified that though.

@benasher44
Copy link
Member

I think in that case, you'd just skip this for abstract targets, for which I think there is a flag/method you can call to ask that.

@dnkoutso
Copy link
Contributor Author

dnkoutso commented Feb 4, 2017

Will update.

@dnkoutso
Copy link
Contributor Author

dnkoutso commented Feb 4, 2017

@benasher44 I simulated this scenario but target.user_targets never returns abstract targets:

abstract_target 'Abstract1' do
  abstract_target 'Abstract2' do
    target 'SampleApp Mac Tests' do
      platform :osx, '10.10'
      pod 'OCMock', '~> 3.1.2'
    end
  end 
end

@benasher44
Copy link
Member

I believe target itself would be abstract. The user_targets should always be PBXNativeTarget types.

@dnkoutso
Copy link
Contributor Author

dnkoutso commented Feb 4, 2017

Ah, looks like still that the generate_ld_runpath_search_paths is only invoked for non-abstract targets.

Probably something higher up the call chain does not generate xcconfigs for abstract targets, which makes sense I think.

if I print out the target.name I see Pods-Abstract1-Abstract2-SampleApp Mac Tests

@endocrimes
Copy link
Member

Yep that should all be handled in analyzer/variants. This change LGTM.

@dnkoutso
Copy link
Contributor Author

dnkoutso commented Feb 5, 2017

Btw creating a new target in xcode for a macOS unit test bundle automatically creates this rpath:

screen shot 2017-02-05 at 8 55 04 am

@benasher44
Copy link
Member

This change looks good to me. Just waiting on merge conflict resolution and green CI

@dnkoutso
Copy link
Contributor Author

dnkoutso commented Feb 8, 2017

@benasher44 the corresponding change for integration specs is here CocoaPods/cocoapods-integration-specs#95

@endocrimes
Copy link
Member

@dnkoutso looks like there is one failure left but I'm not sure why. Maybe needs a rebase?

@dnkoutso
Copy link
Contributor Author

dnkoutso commented Feb 8, 2017

I just did lets see :)

@dnkoutso
Copy link
Contributor Author

dnkoutso commented Feb 9, 2017

Can I merge? :)

@endocrimes
Copy link
Member

Whoop, hadn't seen the ci pass, let ship it

@dnkoutso dnkoutso merged commit 8287d68 into CocoaPods:master Feb 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants