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

Do not include settings from file accessors of test specs into aggregate xcconfigs #7019

Merged

Conversation

dnkoutso
Copy link
Contributor

@dnkoutso dnkoutso commented Sep 8, 2017

No description provided.

@dnkoutso dnkoutso force-pushed the no_test_frameworks_on_aggregates branch 3 times, most recently from fdf22bf to 3dc419e Compare September 8, 2017 18:50
@dnkoutso dnkoutso force-pushed the no_test_frameworks_on_aggregates branch from 3dc419e to 81bee36 Compare September 8, 2017 19:00
@dnkoutso dnkoutso added this to the 1.4.0 milestone Sep 9, 2017
@@ -76,13 +76,15 @@ def self.default_ld_flags(target, include_objc_flag = false)
#
def self.add_settings_for_file_accessors_of_target(aggregate_target, pod_target, xcconfig)
pod_target.file_accessors.each do |file_accessor|
XCConfigHelper.add_spec_build_settings_to_xcconfig(file_accessor.spec_consumer, xcconfig)
XCConfigHelper.add_static_dependency_build_settings(aggregate_target, pod_target, xcconfig, file_accessor)
if aggregate_target.nil? || !file_accessor.spec.test_specification?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the check thats important here and in the other methods...

@@ -241,6 +241,93 @@ module XCConfig

#---------------------------------------------------------------------#

describe 'concerning settings for file accessors' do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol @paulb777 I think we wrote almost the same tests (#6988 think the change we both did is different however :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change basically ensures test spec paths dont end up in aggregates

Copy link
Member

Choose a reason for hiding this comment

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

:) Yeah, if I understand correctly, this change is preventing test spec paths from ending up in aggregates, while #6988 was about making sure that paths were properly propagated aggregates including static frameworks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeap exactly.

@endocrimes endocrimes merged commit fd1191d into CocoaPods:master Sep 11, 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.

3 participants