Add aggregated search paths targets to vendored build settings #5514
Conversation
| @@ -150,7 +150,9 @@ def generate_settings_to_import_pod_targets | |||
| # user target. | |||
| # | |||
| def generate_vendored_build_settings | |||
| pod_targets.each do |pod_target| | |||
| targets = pod_targets + target.search_paths_aggregate_targets.map(&:pod_targets) | |||
|
@mrackwitz looks good to me, seem good to you too? |
|
I'll have a look at the failing tests in the meantime. |
|
I'm using - - '[CP] Check Pods Manifest.lock': []
+ - "[CP] Check Pods Manifest.lock": []Do you have any hint for me where this might be coming from? |
|
I was using |
|
Ok, rebasing |
|
I've updated the test expectations. I'm not sure how to handle the submodule reference - am I supposed to update that one? |
|
It would be nice to have a unit test for that. Beside that this looks good to me though! 👍 |
|
I assume the test should go into |
|
This might also fix #5173 |
|
@plu, this branch has conflicts |
@Coeur: I'm asking mainly for tests not because I'm afraid that these changes here could be wrong or have unexpected effects, but more because untested code tends to break in the future when you're unaware because expectations were not encoded in the specs. So this is completely unrelated from the release context. If that makes it in without tests, no one is ever going to write tests for it. |
|
I think I just got bit by #5512, hoping to see this make it into 1.1.0. Let me know if I can help test. |
|
Testing this branch and I get an error trying to install the KeychainAccess pod I don't see anywhere that active support |
|
I put together a test for this PR. Not sure what to do with it |
|
Any update getting this merged? |
|
@trevonmckay @plu this needs a rebase and some unit tests. |
|
I think in order to make the build pass this PR will need to be accepted first? CocoaPods/cocoapods-integration-specs#73 I'm not the original author of the PR, but I have a unit test written and can do the rebase if it will help move this along. I think I'd have to make a new PR then though? |
|
@dnkoutso, as a member of the project, you may have the ability to rebase during the merging process: |
|
@Coeur sorry I am not a member of the project :) |
|
Nope, this needs a manual rebase |
|
Rebased and added @chrisortman's Unit Test. |
|
I'm using 1.2.0 and this bug seems to still be present. Had to extract my test targets from the app target and append all the pods to them manually because |
|
I think this has regressed in 1.2.0 final - it was fixed in one of the release candidates as I tested it then and it was fine, but have just spent a frustrating morning going in circles with it! |
|
This broke via c45a4bb |
|
@plu can you explain? Based on #6065 static frameworks on static libraries should not be linked if the target is using Search paths should still be added though. There is one case where this is broken if the target explicitly asks for a pod to be integrated which at the moment will not properly add the I am working on a PR that fixes the case above. |
|
So the pull request that broke it was #6437 |
|
Here's a sample project that shows the issue: https://github.com/plu/SearchPath If you build the test target, it cannot find the Once I comment out these lines here it's working again (L106 & L108): #if aggregate_target.nil? || aggregate_target.target_definition.inheritance != 'search_paths'
XCConfigHelper.add_framework_build_settings(vendored_static_framework, xcconfig, pod_target.sandbox.root)
#endAfter doing this change and running diff --git a/Pods/Target Support Files/Pods-SearchPathTests/Pods-SearchPathTests.debug.xcconfig b/Pods/Target Support Files/Pods-SearchPathTests/Pods-SearchPa
index 5aa6975..4de062c 100644
--- a/Pods/Target Support Files/Pods-SearchPathTests/Pods-SearchPathTests.debug.xcconfig
+++ b/Pods/Target Support Files/Pods-SearchPathTests/Pods-SearchPathTests.debug.xcconfig
@@ -1,9 +1,9 @@
-FRAMEWORK_SEARCH_PATHS = $(inherited) "$PODS_CONFIGURATION_BUILD_DIR/1PasswordExtension"
+FRAMEWORK_SEARCH_PATHS = $(inherited) "$PODS_CONFIGURATION_BUILD_DIR/1PasswordExtension" "${PODS_ROOT}/Google-Mobile-Ads-SDK/Frameworks"
GCC_PREPROCESSOR_DEFINITIONS = $(inherited) COCOAPODS=1
HEADER_SEARCH_PATHS = $(inherited) "${PODS_ROOT}/Headers/Public" "${PODS_ROOT}/Headers/Public/Google-Mobile-Ads-SDK"
LD_RUNPATH_SEARCH_PATHS = $(inherited) '@executable_path/Frameworks' '@loader_path/Frameworks'
OTHER_CFLAGS = $(inherited) -iquote "$PODS_CONFIGURATION_BUILD_DIR/1PasswordExtension/OnePasswordExtension.framework/Headers" -isystem "${PODS_ROOT}/Headers/
-OTHER_LDFLAGS = $(inherited) -framework "AVFoundation" -framework "AudioToolbox" -framework "CoreBluetooth" -framework "CoreGraphics" -framework "CoreMedia"
+OTHER_LDFLAGS = $(inherited) -framework "AVFoundation" -framework "AudioToolbox" -framework "CoreBluetooth" -framework "CoreGraphics" -framework "CoreMedia"
PODS_BUILD_DIR = $BUILD_DIR
PODS_CONFIGURATION_BUILD_DIR = $PODS_BUILD_DIR/$(CONFIGURATION)$(EFFECTIVE_PLATFORM_NAME)
PODS_ROOT = ${SRCROOT}/Pods
diff --git a/Pods/Target Support Files/Pods-SearchPathTests/Pods-SearchPathTests.release.xcconfig b/Pods/Target Support Files/Pods-SearchPathTests/Pods-Search
index 5aa6975..4de062c 100644
--- a/Pods/Target Support Files/Pods-SearchPathTests/Pods-SearchPathTests.release.xcconfig
+++ b/Pods/Target Support Files/Pods-SearchPathTests/Pods-SearchPathTests.release.xcconfig
@@ -1,9 +1,9 @@
-FRAMEWORK_SEARCH_PATHS = $(inherited) "$PODS_CONFIGURATION_BUILD_DIR/1PasswordExtension"
+FRAMEWORK_SEARCH_PATHS = $(inherited) "$PODS_CONFIGURATION_BUILD_DIR/1PasswordExtension" "${PODS_ROOT}/Google-Mobile-Ads-SDK/Frameworks"
GCC_PREPROCESSOR_DEFINITIONS = $(inherited) COCOAPODS=1
HEADER_SEARCH_PATHS = $(inherited) "${PODS_ROOT}/Headers/Public" "${PODS_ROOT}/Headers/Public/Google-Mobile-Ads-SDK"
LD_RUNPATH_SEARCH_PATHS = $(inherited) '@executable_path/Frameworks' '@loader_path/Frameworks'
OTHER_CFLAGS = $(inherited) -iquote "$PODS_CONFIGURATION_BUILD_DIR/1PasswordExtension/OnePasswordExtension.framework/Headers" -isystem "${PODS_ROOT}/Headers/
-OTHER_LDFLAGS = $(inherited) -framework "AVFoundation" -framework "AudioToolbox" -framework "CoreBluetooth" -framework "CoreGraphics" -framework "CoreMedia"
+OTHER_LDFLAGS = $(inherited) -framework "AVFoundation" -framework "AudioToolbox" -framework "CoreBluetooth" -framework "CoreGraphics" -framework "CoreMedia"
PODS_BUILD_DIR = $BUILD_DIR
PODS_CONFIGURATION_BUILD_DIR = $PODS_BUILD_DIR/$(CONFIGURATION)$(EFFECTIVE_PLATFORM_NAME)
PODS_ROOT = ${SRCROOT}/Pods |
|
I believe that |
|
Yes you are correct. I'll be opening a PR that fixes this. |
|
Thank you! |
|
I can also test on a more complex project once you have a patch ready. |
|
@plu I tried it on your SearchPath project and it fixes it. Would be good to get a more complicated project :) |
|
I can test this on a more complicated project (3 framework targets, each with unit tests and an app target with unit and UI tests; all in one project), but I'd need some instructions on how to get the dev version working on my machine. |
|
@YANOUSHek if you have a Or you can choose the master branch now since it was merged. |
|
@dnkoutso I think I've managed to get the current master branch installed and unfortunately it does not solve the problem. When I migrated my Podfile to use search path inheritance it still doesn't add the static libs into the project. I'm not really sure if I'm running the version from master (they both report the same version number – 1.2.0) but I'm pretty sure I managed to get the newest one. |
what do you mean by that? Its not supposed to add pre-built static libraries or frameworks to your project. Can you please open a new issue with a sample project instead so we don't post on a closed issue. |
|
A simple test case that now fails (again, after working in 1.2 beta 3) is to include Google Analytics into things. It fails to build any test target with the inherit paths option set. This regression is seriously problematic. Especially after it took so long to be solved in the first place. I had the luxery of it working for maybe a month at most 😞 I know this issue is closed, but a test case or something is needed to support it not happening again. |
|
@krider2010 tests were added and this was further improved and merged to master. It will most likely ship with 1.2.1. |
|
@krider2010 I had the same problem with Google Analytics, where my test target failed to build. I posted a question about this on stackoverflow before I saw this thread. The good news is that after upgrading to Cocoapods 1.2.1.beta.1, and re-running pod install, my test target now builds. |
|
Yeap this is fixed in 1.2.1.beta.1 thanks for verifying again. |
|
@dnkoutso Thank you so much for resolving this. |
|
@Oyashirox please do not respond to a close issue. File a new one and provide a sample project. |
Fix #5512
I assume this will break some tests. Before fixing/adapting them I'd like to see your approval that this is the correct fix :).