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

Switch swift_version to 5.0 and add LIBRARY_SEARCH_PATHS in xcconfig #315

Closed
wants to merge 1 commit into from

Conversation

artyom-razinov
Copy link
Contributor

@artyom-razinov artyom-razinov commented Oct 16, 2019

This fixes running tests on iOS 10/11/12 with Cuckoo when tests are built using Xcode 11.

It is a minimal required fix. I checked this, nothing can be removed from these changes to fix the issue.

I think it is okay to switch to Swift 5 at the moment. It is the biggest change in terms of compatibility, it has stable ABI, etc. I was trying to keep my project in Swift 5, but I gave up after several days. You can make a tag named swift-4.2 for current HEAD in Cuckoo. People who use your project will still be able use your project.

I think there is no point in holding on to the past now.

It is a minimal required fix to avoid a runtime linking error while running tests on iOS versions prior to 13 after building with Xcode 11
@TadeasKriz
Copy link
Member

Thank you for the PR. I agree with you about upgrading to Swift 5.0. Hopefully it won't be a PITA in the long run.

@sanoj00b
Copy link

when I am using this commit in cart file I am getting error
A shell task (/usr/bin/env git checkout --quiet --force ebe3aec90afb8fe97465c8c6dab60660edf40e31 (launched in /Users/X/Library/Caches/org.carthage.CarthageKit/dependencies/Cuckoo)) failed with exit code 128: fatal: reference is not a tree: ebe3aec90afb8fe97465c8c6dab60660edf40e31
Could you please let me know how I can use this? I am not able to build with Xcode11 with Swift5

@MatyasKriz
Copy link
Collaborator

Hey, thanks for giving back to the Cuckoo community by creating a pull request.

The reason we've not merged these changes in yet is the Carthage situation.

I'll try to get it sorted out today.

Thanks again for the PR!

@sanoj00b
Copy link

Hey, thanks for giving back to the Cuckoo community by creating a pull request.

The reason we've not merged these changes in yet is the Carthage situation.

I'll try to get it sorted out today.

Thanks again for the PR!

I have raised this issue. Could you please look at it? is it related to this PR changes?
#316

@MatyasKriz
Copy link
Collaborator

I was responding to this PR's author @artyom-razinov. It was just coincidentally right after your comment. Lesson learned to use the handle next time.

@MatyasKriz
Copy link
Collaborator

@sanoj00b Carthage is broken in 1.2.0 thanks to its lightweight (read limited) functionality that probably forces us to separate the repository into swift mocking and ObjC mocking using OCMock.

@artyom-razinov
Copy link
Contributor Author

artyom-razinov commented Oct 24, 2019

when I am using this commit in cart file I am getting error

It is not in Brightify/Cuckoo repo yet

I have raised this issue. Could you please look at it? is it related to this PR changes?

No. The issue this PR fixes is that without these changes any test target that imports Cuckoo crashes on iOS 12 or lower if built with Xcode 10. It is a runtime issue. You have compile time issue.

@MatyasKriz
Copy link
Collaborator

The matter is a bit more complicated than I anticipated. I'll have to push this into next week.

@sanoj00b
Copy link

sanoj00b commented Nov 5, 2019

The matter is a bit more complicated than I anticipated. I'll have to push this into next week.

Are we planning this week?

@MatyasKriz
Copy link
Collaborator

The matter is a bit more complicated than I anticipated. I'll have to push this into next week.

Are we planning this week?

Yes.

@MatyasKriz
Copy link
Collaborator

@artyom-razinov, I'll be merging this pull request and releasing it shortly.

Even though this PR consists of only a few lines, I have no doubt that it took a significant amount of time of finding the leanest (and most elegant) solution to the problem.

On behalf of the Cuckoo community I thank you for the pull request and am looking forward to working with you in the future!

@MatyasKriz MatyasKriz closed this Nov 6, 2019
@artyom-razinov
Copy link
Contributor Author

We had some problems with this solution. It seems that this line:

'LIBRARY_SEARCH_PATHS' => '$(TOOLCHAIN_DIR)/usr/lib/swift-$(SWIFT_VERSION)/$(PLATFORM_NAME) $(inherited)'

caused a problem in a very old project (https://forums.swift.org/t/undefined-symbol-swift-getfunctionreplacement/30495) when was linked to a library with that line in podspec.

I've localized the issue to this exact line, but the issue is only with 1 project. Some settings seem to conflict with each other. This is just FYI.

It was working fine in a newer project, I'm investigating this now.

@MatyasKriz
Copy link
Collaborator

Thanks for the info. I jumped over Xcode 11.1 and my testing project that fetches Cuckoo was working the way it should.

Still, if there is an issue with the library search paths for more Cuckoo users, we'll see where we can go from there.

@artyom-razinov
Copy link
Contributor Author

The problem in our old project was solved by setting DEAD_CODE_STRIPPING to YES. Just fyi.

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.

4 participants