Move CommonCrypto and zlib dependencies mapping to custom module map #520

turbulem · 2018-06-14T18:22:40Z

Hi!
I faced an issue similar to this and this this.
In my project I use Carthage and commit my artefacts into my repo. And CI(and any other machine) is not able to compile it without running 'carthage update'.
Basically the problem that to link Starscream.framework with other app or library you should provide module.map similar to the one, located in /zlib folder (but without header, which is required only for build).
To make this work I created new custom module map and added a wrapper for SHA1 algorithm used by Starscream. Also updated cocoapods spec to add custom module map + private headers.

Please consider this PR as a possible way of resolving issues linked above and also I'd be happy to hear from you any suggestions on this.

Thank you!
Cheers!

kotsaarev · 2018-06-24T16:10:20Z

Hello @turbulem.

Thank you for your solution! But I still have a problem.

I use GitLab CI, two build machines, cache server and Carthage. When I create cache on the first machine, and then use it on the second one, I get an error

Missing required modules: 'SSCZLib', 'SSCommonCrypto'

turbulem · 2018-06-25T09:29:48Z

@kotsaarev Thank you for feedback. Let me check

turbulem · 2018-06-25T10:45:15Z

I double checked. You're right. Looks like it actually doesn't solve an issue.
I have an alternative solution and will update this PR with it later this week.

…ing module maps for CommonCrypto and zlib

JeanAzzopardi · 2018-06-27T14:03:20Z

Just to add my 2c, I'm getting the same issue with Starscream on CircleCI - where I'm building carthage dependencies on a different lane and then using the cached dependencies for the test suite, and I get the same error: missing required modules: 'SSCZLib', 'SSCommonCrypto'

turbulem · 2018-06-27T14:15:52Z

@JeanAzzopardi Did you check the most recent version?
The latest version in my repo should work fine:
turbulem@8ca8df0

JeanAzzopardi · 2018-06-27T14:30:10Z

I haven't checked your version @turbulem, will try it out now and let you know how it goes.

JeanAzzopardi · 2018-06-27T15:07:03Z

@turbulem - I can confirm your solution works on CircleCI

turbulem · 2018-06-27T15:22:27Z

@JeanAzzopardi Thank you! Hope this PR will help you to fix the issue

JeanAzzopardi · 2018-06-27T16:43:16Z

However @turbulem , I just tested on device (compiled on my machine with Xcode 9.3) and swift 4.1.2, running on iOS 11, and I am getting a crash:

 *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[NSConcreteData _fastCStringContents:]: unrecognized selector sent to instance 0x1c4237600'

Crash is occurring in these lines:

private extension String {
    func sha1Base64() -> String {
        return (self as NSString).ss_SHA1Base64Digest()
    }
}

turbulem · 2018-06-27T17:03:55Z

@JeanAzzopardi Does this happen only on real device? Let me check later today on device. Thank you for your feedback!

JeanAzzopardi · 2018-06-27T22:10:05Z

I will check and get back to you. Could be related to SSL connection too.

turbulem · 2018-06-28T09:13:39Z

@JeanAzzopardi It may be related to the ss_SHA1Base64Digest function that I added in my PR. This is small wrapper around CC, but all tests passed and I didn't check on device. I will have some time later this week or on a weekend.
But if you can check, that would be wonderful!

JeanAzzopardi · 2018-06-28T10:12:22Z

@turbulem So I downloaded your branch and checked it out, the tests do pass, but trying the SimpleExample with the local websocket server on simulator gives me the same error.

turbulem · 2018-06-28T10:31:47Z

Actually yes, I made a mistake. I return NSData instead of NSString and somehow compiler let me get away with it, producing a warning I didn't pay attention to :(
I fixed it and updated PR.
Silly mistake, now it works well.
Thank you for your efforts to help!

JeanAzzopardi · 2018-06-28T12:00:47Z

Yes, it's working now, that was the issue!

No problem, glad to help out, looking forward for this P.R. to be merged.

JeanAzzopardi · 2018-07-25T08:58:25Z

@turbulem @daltoniam would it be possible to merge this? It's the last thing holding me back from upgrading to Starscream 3 <3

# Conflicts: # Starscream.xcodeproj/project.pbxproj

turbulem · 2018-07-25T09:50:55Z

I've updated my branch with new changes

kotsaarev · 2018-07-30T11:37:57Z

Hello @turbulem!
I think the problem with sha1Base64 still here, but occurs only in DEBUG mode. I checked out bd0732e and

*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[Swift._NSContiguousString ss_SHA1Base64Digest]: unrecognized selector sent to instance 0x108b21050'

Crash is occurring in same lines

private extension String {
    func sha1Base64() -> String {
        return (self as NSString).ss_SHA1Base64Digest()
    }
}

I tested with iOS 11.4, Xcode 9.4.1, Swift 4.1. For more info see crash file.

turbulem · 2018-07-30T11:42:33Z

@kotsaarev I will double check later this week. Thank you.

kotsaarev · 2018-07-31T08:20:25Z

@turbulem
I have more information. This occurs not only in DEBUG. This is after pull and merge master because with ca783db all fine.

turbulem · 2018-07-31T11:38:13Z

@kotsaarev So the final version should work fine. Let me know if I can help you anyhow.

dive · 2018-08-03T16:27:19Z

@kotsaarev, any chance that it will be merged in the near future?

JeanAzzopardi · 2018-08-06T09:39:46Z

@daltoniam Any chance that this could be merged? It's the only blocker preventing me from releasing an update as I use CircleCI for releases.

czater · 2018-09-03T08:48:18Z

any chance to merge?

gregiOS · 2018-09-06T10:09:59Z

Two things are missing here:
NSString+SHA1.m need to be added into Starscream (and test target)
Current solution is making app to crash /

Other linkers flag need to contains -all-load

https://github.com/gregiOS/Starscream/pull/1/
Here you can check those changes with tests

turbulem · 2018-09-12T16:30:52Z

@gregiOS Thank you for review. I made necessary changes. Now should be fine.

update to swift 4 manifest

gregiOS · 2018-09-26T10:38:02Z

Any chance for rebase @turbulem, and merge @daltoniam?

…ing module maps for CommonCrypto and zlib

# Conflicts: # Starscream.xcodeproj/project.pbxproj

fassko · 2018-09-27T11:15:09Z

@turbulem Can you please take a look on these merge conflicts. It will be easier to merge these changes. Thanks!

turbulem · 2018-09-27T11:46:07Z

Hi, I updated my branch, however there are some issues with new Swift and tests need to be updated. I will do this later this week.

fassko · 2018-09-27T11:46:51Z

OK let me know when I can check again.

fassko · 2018-09-28T08:38:51Z

@turbulem please check:

SSLClientCertificate.swift included twice in Sources folder
Xcode project doesn't have files from Sources/Starscream to build

fassko · 2018-09-28T09:14:05Z

File SSLClientCertificate.swift is in Sources and Sources/Starscream. I guess it should be just once in project. Thanks!

fassko · 2018-09-28T09:50:41Z

Package.swift

+                .library(name: "Starscream", targets: ["Starscream"])
+        ],
+        dependencies: [
+                .package(url: "https://github.com/daltoniam/zlib-spm.git", from: "1.1.0"),


I'm being picky here, but for consistency let's use same format without .git :)

This is some merge issue, I will update my branch and discards this changes.
I had no intention to change SPM config

Looks like it's some GH issue, because these files are equal:

https://github.com/daltoniam/Starscream/blob/master/Package.swift
https://github.com/turbulem/Starscream/blob/master/Package.swift

I will make another branch and refresh this PR in order to make it clear.

OK, I can adjust it later myself.

turbulem · 2018-09-28T09:59:51Z

@fassko Thank you for your instant reviews 👍

fassko · 2018-09-28T12:36:06Z

@turbulem Let me know when you're done and I will merge this.

turbulem · 2018-09-28T13:50:53Z

@fassko I opened new PR, please take a look.

Move CommonCrypto and zlib dependencies mapping to private module map

98e1d62

Add custom wrapper for SHA1 and custom modulemap for framework includ…

8ca8df0

…ing module maps for CommonCrypto and zlib

turbulem changed the title ~~Move CommonCrypto and zlib dependencies mapping to private module map~~ Move CommonCrypto and zlib dependencies mapping to custom module map Jun 25, 2018

Fix return type for SHA1 digest and cast warning

ca783db

vkedwardli mentioned this pull request Jul 24, 2018

Fix Redefinition of module error in Xcode ≧ 8.3, Take 2 #482

Closed

turbulem added 2 commits July 25, 2018 10:47

Pull master branch from daltoniam/Starscream

138cfa7

Merge branch 'master'

bd0732e

# Conflicts: # Starscream.xcodeproj/project.pbxproj

turbulem added 2 commits September 10, 2018 09:40

Fix project inclusion and add missing linker flag

8df9d1e

Add NSString+SHA1 to test target

7f12731

update to swift 4 manifest

a403aea

daltoniam mentioned this pull request Sep 25, 2018

update to swift 4 manifest #556

Merged

Merge pull request daltoniam#556 from nuclearace/spm-4

79cd8a2

update to swift 4 manifest

turbulem added 7 commits September 26, 2018 14:55

Move CommonCrypto and zlib dependencies mapping to private module map

94f8d58

Add custom wrapper for SHA1 and custom modulemap for framework includ…

70234bc

…ing module maps for CommonCrypto and zlib

Fix return type for SHA1 digest and cast warning

912e7da

Pull master branch from daltoniam/Starscream

74e3c63

Fix project inclusion and add missing linker flag

33846d1

Add NSString+SHA1 to test target

6371914

Merge branch 'master' of github.com:turbulem/Starscream

d629a65

# Conflicts: # Starscream.xcodeproj/project.pbxproj

Fix project settings

631c256

Remove old file

467334e

fassko requested changes Sep 28, 2018

View reviewed changes

fassko approved these changes Sep 28, 2018

View reviewed changes

turbulem mentioned this pull request Sep 28, 2018

Move CommonCrypto and zlib dependencies mapping to custom module map #559

Merged

turbulem closed this Sep 28, 2018

Move CommonCrypto and zlib dependencies mapping to custom module map #520

Move CommonCrypto and zlib dependencies mapping to custom module map #520

Conversation

turbulem commented Jun 14, 2018 • edited

kotsaarev commented Jun 24, 2018

turbulem commented Jun 25, 2018

turbulem commented Jun 25, 2018

JeanAzzopardi commented Jun 27, 2018

turbulem commented Jun 27, 2018

JeanAzzopardi commented Jun 27, 2018

JeanAzzopardi commented Jun 27, 2018

turbulem commented Jun 27, 2018

JeanAzzopardi commented Jun 27, 2018 • edited

turbulem commented Jun 27, 2018

JeanAzzopardi commented Jun 27, 2018 via email • edited

turbulem commented Jun 28, 2018

JeanAzzopardi commented Jun 28, 2018

turbulem commented Jun 28, 2018 • edited

JeanAzzopardi commented Jun 28, 2018

JeanAzzopardi commented Jul 25, 2018

turbulem commented Jul 25, 2018

kotsaarev commented Jul 30, 2018

turbulem commented Jul 30, 2018

kotsaarev commented Jul 31, 2018

turbulem commented Jul 31, 2018

dive commented Aug 3, 2018

JeanAzzopardi commented Aug 6, 2018

czater commented Sep 3, 2018

gregiOS commented Sep 6, 2018 • edited

turbulem commented Sep 12, 2018

gregiOS commented Sep 26, 2018

fassko commented Sep 27, 2018

turbulem commented Sep 27, 2018

fassko commented Sep 27, 2018

fassko commented Sep 28, 2018

fassko commented Sep 28, 2018

fassko Sep 28, 2018 • edited

Choose a reason for hiding this comment

turbulem Sep 28, 2018

Choose a reason for hiding this comment

turbulem Sep 28, 2018

Choose a reason for hiding this comment

fassko Sep 28, 2018

Choose a reason for hiding this comment

turbulem commented Sep 28, 2018

fassko commented Sep 28, 2018

turbulem commented Sep 28, 2018

turbulem commented Jun 14, 2018 •

edited

JeanAzzopardi commented Jun 27, 2018 •

edited

JeanAzzopardi commented Jun 27, 2018 via email •

edited

turbulem commented Jun 28, 2018 •

edited

gregiOS commented Sep 6, 2018 •

edited

fassko Sep 28, 2018 •

edited