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.

Sign up for GitHub

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

Jump to bottom

Exclude test resource and framework paths from aggregate targets #7000

Merged
merged 1 commit into from Sep 12, 2017
Merged

Exclude test resource and framework paths from aggregate targets #7000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -43,6 +43,10 @@ To install release candidates run `[sudo] gem install cocoapods --pre`
[Justin Martin](https://github.com/justinseanmartin)
[#7013](https://github.com/CocoaPods/CocoaPods/pull/7013)

* Exclude test resource and framework paths from aggregate targets
[Dimitris Koutsogiorgas](https://github.com/dnkoutso)
[#7000](https://github.com/CocoaPods/CocoaPods/pull/7000)

* Wrap platform warning message with quotes
[Dimitris Koutsogiorgas](https://github.com/dnkoutso)
[#6968](https://github.com/CocoaPods/CocoaPods/pull/6968)
Expand Down
4 changes: 2 additions & 2 deletions lib/cocoapods/target/aggregate_target.rb
View file
Open in desktop
Expand Up @@ -194,7 +194,7 @@ def framework_paths_by_config
relevant_pod_targets = pod_targets.select do |pod_target|
pod_target.include_in_build_config?(target_definition, config)
end
framework_paths_by_config[config] = relevant_pod_targets.flat_map(&:framework_paths)
framework_paths_by_config[config] = relevant_pod_targets.flat_map { |pt| pt.framework_paths(false) }
end
framework_paths_by_config
end
Expand All @@ -210,7 +210,7 @@ def resource_paths_by_config
user_build_configurations.keys.each_with_object({}) do |config, resources_by_config|
resources_by_config[config] = relevant_pod_targets.flat_map do |pod_target|
next [] unless pod_target.include_in_build_config?(target_definition, config)
(pod_target.resource_paths + [bridge_support_file].compact).uniq
(pod_target.resource_paths(false) + [bridge_support_file].compact).uniq
end
end
end
Expand Down
90 changes: 56 additions & 34 deletions lib/cocoapods/target/pod_target.rb
View file
Open in desktop
Expand Up @@ -197,59 +197,81 @@ def supported_test_types
specs.select(&:test_specification?).map(&:test_type).uniq
end

# Returns the framework paths associated with this target. By default all paths include the framework paths
# that are part of test specifications.
#
# @param [Boolean] include_test_spec_paths
# Whether to include framework paths from test specifications or not.
#
# @return [Array<Hash{Symbol => [String]}>] The vendored and non vendored framework paths
# this target depends upon.
#
def framework_paths
@framework_paths ||= begin
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed the @framework_paths ||= begin this is why the diff looks weird.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Easier to review with https://github.com/CocoaPods/CocoaPods/pull/7000/files?w=1

Copy link
Member

Choose a reason for hiding this comment

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

What's the purpose of removing the caching? This is kinda expensive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So now we are parameterized whether we return paths including the ones from tests or not.

I can actually cache it where the key includes the boolean if you'd like. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cache is now added back and it depends on the parameter.

frameworks = []
file_accessors.flat_map(&:vendored_dynamic_artifacts).map do |framework_path|
relative_path_to_sandbox = framework_path.relative_path_from(sandbox.root)
framework = { :name => framework_path.basename.to_s,
:input_path => "${PODS_ROOT}/#{relative_path_to_sandbox}",
:output_path => "${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/#{framework_path.basename}" }
# Until this can be configured, assume the dSYM file uses the file name as the framework.
# See https://github.com/CocoaPods/CocoaPods/issues/1698
dsym_name = "#{framework_path.basename}.dSYM"
dsym_path = Pathname.new("#{framework_path.dirname}/#{dsym_name}")
if dsym_path.exist?
framework[:dsym_name] = dsym_name
framework[:dsym_input_path] = "${PODS_ROOT}/#{relative_path_to_sandbox}.dSYM"
framework[:dsym_output_path] = "${DWARF_DSYM_FOLDER_PATH}/#{dsym_name}"
def framework_paths(include_test_spec_paths = true)
@framework_paths ||= Hash.new do |h, key|
h[key] = begin
accessors = file_accessors
accessors = accessors.reject { |a| a.spec.test_specification? } unless include_test_spec_paths
frameworks = []
accessors.flat_map(&:vendored_dynamic_artifacts).map do |framework_path|
relative_path_to_sandbox = framework_path.relative_path_from(sandbox.root)
framework = { :name => framework_path.basename.to_s,
:input_path => "${PODS_ROOT}/#{relative_path_to_sandbox}",
:output_path => "${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/#{framework_path.basename}" }
# Until this can be configured, assume the dSYM file uses the file name as the framework.
# See https://github.com/CocoaPods/CocoaPods/issues/1698
dsym_name = "#{framework_path.basename}.dSYM"
dsym_path = Pathname.new("#{framework_path.dirname}/#{dsym_name}")
if dsym_path.exist?
framework[:dsym_name] = dsym_name
framework[:dsym_input_path] = "${PODS_ROOT}/#{relative_path_to_sandbox}.dSYM"
framework[:dsym_output_path] = "${DWARF_DSYM_FOLDER_PATH}/#{dsym_name}"
end
frameworks << framework
end
frameworks << framework
end
if should_build? && requires_frameworks? && !static_framework?
frameworks << { :name => product_name,
:input_path => build_product_path('${BUILT_PRODUCTS_DIR}'),
:output_path => "${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/#{product_name}" }
if should_build? && requires_frameworks? && !static_framework?
frameworks << { :name => product_name,
:input_path => build_product_path('${BUILT_PRODUCTS_DIR}'),
:output_path => "${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/#{product_name}" }
end
frameworks
end
frameworks
end
@framework_paths[include_test_spec_paths]
end

# Returns the resource paths associated with this target. By default all paths include the resource paths
# that are part of test specifications.
#
# @param [Boolean] include_test_spec_paths
# Whether to include resource paths from test specifications or not.
#
# @return [Array<String>] The resource and resource bundle paths this target depends upon.
#
def resource_paths
@resource_paths ||= begin
resource_paths = file_accessors.flat_map do |accessor|
accessor.resources.flat_map { |res| "${PODS_ROOT}/#{res.relative_path_from(sandbox.project.path.dirname)}" }
end
resource_bundles = file_accessors.flat_map do |accessor|
prefix = Generator::XCConfig::XCConfigHelper::CONFIGURATION_BUILD_DIR_VARIABLE
prefix = configuration_build_dir unless accessor.spec.test_specification?
accessor.resource_bundles.keys.map { |name| "#{prefix}/#{name.shellescape}.bundle" }
def resource_paths(include_test_spec_paths = true)
@resource_paths ||= Hash.new do |h, key|
h[key] = begin
accessors = file_accessors
accessors = accessors.reject { |a| a.spec.test_specification? } unless include_test_spec_paths
resource_paths = accessors.flat_map do |accessor|
accessor.resources.flat_map { |res| "${PODS_ROOT}/#{res.relative_path_from(sandbox.project.path.dirname)}" }
end
resource_bundles = accessors.flat_map do |accessor|
prefix = Generator::XCConfig::XCConfigHelper::CONFIGURATION_BUILD_DIR_VARIABLE
prefix = configuration_build_dir unless accessor.spec.test_specification?
accessor.resource_bundles.keys.map { |name| "#{prefix}/#{name.shellescape}.bundle" }
end
resource_paths + resource_bundles
end
resource_paths + resource_bundles
end
@resource_paths[include_test_spec_paths]
end

# Returns the corresponding native target to use based on the provided specification.
# This is used to figure out whether to add a source file into the library native target or any of the
# test native targets.
#
# @param [Specification] spec
# The specifcation to base from in order to find the native target.
# The specification to base from in order to find the native target.
#
# @return [PBXNativeTarget] the native target to use or `nil` if none is found.
#
Expand Down
63 changes: 63 additions & 0 deletions spec/unit/target/pod_target_spec.rb
View file
Open in desktop
Expand Up @@ -375,6 +375,69 @@ module Pod
@test_pod_target.stubs(:file_accessors).returns([fa])
@test_pod_target.resource_paths.should == ['$PODS_CONFIGURATION_BUILD_DIR/TestResourceBundle.bundle']
end

it 'includes framework paths from test specifications' do
fa = Sandbox::FileAccessor.new(nil, @test_pod_target)
fa.stubs(:vendored_dynamic_artifacts).returns([config.sandbox.root + Pathname.new('Vendored/Vendored.framework')])
fa.stubs(:spec).returns(stub(:test_specification? => false))
test_fa = Sandbox::FileAccessor.new(nil, @test_pod_target)
test_fa.stubs(:vendored_dynamic_artifacts).returns([config.sandbox.root + Pathname.new('Vendored/TestVendored.framework')])
test_fa.stubs(:spec).returns(stub(:test_specification? => true))
@test_pod_target.stubs(:file_accessors).returns([fa, test_fa])
@test_pod_target.stubs(:should_build?).returns(true)
@test_pod_target.framework_paths.should == [
{ :name => 'Vendored.framework',
:input_path => '${PODS_ROOT}/Vendored/Vendored.framework',
:output_path => '${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/Vendored.framework' },
{ :name => 'TestVendored.framework',
:input_path => '${PODS_ROOT}/Vendored/TestVendored.framework',
:output_path => '${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/TestVendored.framework' },
]
end

it 'excludes framework paths from test specifications when not requested' do
fa = Sandbox::FileAccessor.new(nil, @test_pod_target)
fa.stubs(:vendored_dynamic_artifacts).returns([config.sandbox.root + Pathname.new('Vendored/Vendored.framework')])
fa.stubs(:spec).returns(stub(:test_specification? => false))
test_fa = Sandbox::FileAccessor.new(nil, @test_pod_target)
test_fa.stubs(:vendored_dynamic_artifacts).returns([config.sandbox.root + Pathname.new('Vendored/TestVendored.framework')])
test_fa.stubs(:spec).returns(stub(:test_specification? => true))
@test_pod_target.stubs(:file_accessors).returns([fa, test_fa])
@test_pod_target.stubs(:should_build?).returns(true)
@test_pod_target.framework_paths(false).should == [
{ :name => 'Vendored.framework',
:input_path => '${PODS_ROOT}/Vendored/Vendored.framework',
:output_path => '${TARGET_BUILD_DIR}/${FRAMEWORKS_FOLDER_PATH}/Vendored.framework' },
]
end

it 'includes resource paths from test specifications' do
config.sandbox.stubs(:project => stub(:path => Pathname.new('ProjectPath')))
fa = Sandbox::FileAccessor.new(nil, @test_pod_target)
fa.stubs(:resource_bundles).returns({})
fa.stubs(:resources).returns([Pathname.new('Model.xcdatamodeld')])
fa.stubs(:spec).returns(stub(:test_specification? => false))
test_fa = Sandbox::FileAccessor.new(nil, @test_pod_target)
test_fa.stubs(:resource_bundles).returns({})
test_fa.stubs(:resources).returns([Pathname.new('TestModel.xcdatamodeld')])
test_fa.stubs(:spec).returns(stub(:test_specification? => true))
@test_pod_target.stubs(:file_accessors).returns([fa, test_fa])
@test_pod_target.resource_paths.should == ['${PODS_ROOT}/Model.xcdatamodeld', '${PODS_ROOT}/TestModel.xcdatamodeld']
end

it 'excludes resource paths from test specifications when not requested' do
config.sandbox.stubs(:project => stub(:path => Pathname.new('ProjectPath')))
fa = Sandbox::FileAccessor.new(nil, @test_pod_target)
fa.stubs(:resource_bundles).returns({})
fa.stubs(:resources).returns([Pathname.new('Model.xcdatamodeld')])
fa.stubs(:spec).returns(stub(:test_specification? => false))
test_fa = Sandbox::FileAccessor.new(nil, @test_pod_target)
test_fa.stubs(:resource_bundles).returns({})
test_fa.stubs(:resources).returns([Pathname.new('TestModel.xcdatamodeld')])
test_fa.stubs(:spec).returns(stub(:test_specification? => true))
@test_pod_target.stubs(:file_accessors).returns([fa, test_fa])
@test_pod_target.resource_paths(false).should == ['${PODS_ROOT}/Model.xcdatamodeld']
end
end
end
end
Expand Down