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

ATL-5386: Remove cocoapods so it works on the iOS #87

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

goncalo-frade-iohk
Copy link
Contributor

@goncalo-frade-iohk goncalo-frade-iohk commented Aug 2, 2023

Removed cocoapods for internal libraries and made them static.

The reason is if they are frameworks, the will not go within Apollo framework, instead they will be dependencies and expected to exist as a dependency, also Apollo itself should be a static framework so it can be easily reused on our SDK.

@atala-dev
Copy link
Contributor

atala-dev commented Aug 2, 2023

🦙 MegaLinter status: ⚠️ WARNING

Descriptor Linter Files Fixed Errors Elapsed time
⚠️ BASH bash-exec 1 1 0.01s
⚠️ BASH shfmt 1 1 0.01s
✅ EDITORCONFIG editorconfig-checker 41 0 0.14s
✅ KOTLIN ktlint 25 0 59.07s
⚠️ REPOSITORY devskim yes 6 1.41s
✅ REPOSITORY dustilock yes no 0.07s
✅ REPOSITORY gitleaks yes no 2.12s
✅ REPOSITORY git_diff yes no 0.04s
✅ REPOSITORY secretlint yes no 5.03s
✅ REPOSITORY syft yes no 0.5s
✅ REPOSITORY trivy yes no 6.22s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@goncalo-frade-iohk goncalo-frade-iohk force-pushed the feature/workingIOSLibs branch 3 times, most recently from 6303d96 to 25b0c3a Compare August 4, 2023 14:34
@goncalo-frade-iohk goncalo-frade-iohk changed the title Feature/working iOS libs still working on it but want to check build ATL-5386: Remove cocoapods so it works on the iOS Aug 4, 2023
@@ -7,8 +7,7 @@ val os: OperatingSystem = OperatingSystem.current()

plugins {
kotlin("multiplatform")
kotlin("native.cocoapods")
id("com.chromaticnoise.multiplatform-swiftpackage") version "2.0.3"
id("io.github.luca992.multiplatform-swiftpackage") version "2.0.5-arm64"
Copy link
Contributor

@hamada147 hamada147 Aug 7, 2023

Choose a reason for hiding this comment

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

Suggested change
id("io.github.luca992.multiplatform-swiftpackage") version "2.0.5-arm64"
val osName = System.getProperty("os.name").toLowerCase()
if (osName.contains("mac os x") || osName.contains("darwin") || osName.contains("osx")) {
if (System.getProperty("os.arch") != "x86_64") { // M1Chip
id("io.github.luca992.multiplatform-swiftpackage") version "2.0.5-arm64"
} else {
id("io.github.luca992.multiplatform-swiftpackage") version("2.0.3")
}
}

Copy link
Contributor Author

@goncalo-frade-iohk goncalo-frade-iohk Aug 7, 2023

Choose a reason for hiding this comment

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

Do we still need to add this? I don't think it makes a difference :) also I tried doing this and it was giving me an error when I add an If like so inside the plugins {...}

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I've update the code snippet.

Copy link
Contributor

Choose a reason for hiding this comment

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

I updated to code. This version should work inside the plugins block now.

Comment on lines 54 to 85
ios() {
swiftCinterop("IOHKCryptoKit", name)
swiftCinterop("IOHKSecureRandomGeneration", name)

binaries.framework {
baseName = currentModuleName
embedBitcode("disable")
}
}
macosX64() {
binaries.framework {
baseName = currentModuleName
embedBitcode("disable")
}

swiftCinterop("IOHKCryptoKit", name)
swiftCinterop("IOHKSecureRandomGeneration", name)
}

if (System.getProperty("os.arch") != "x86_64") { // M1Chip
iosSimulatorArm64()
iosSimulatorArm64() {
binaries.framework {
baseName = currentModuleName
embedBitcode("disable")
}

swiftCinterop("IOHKCryptoKit", name)
swiftCinterop("IOHKSecureRandomGeneration", name)
}
// tvosSimulatorArm64()
// watchosSimulatorArm64()
macosArm64()
macosArm64() {
binaries.framework {
baseName = currentModuleName
embedBitcode("disable")
}

swiftCinterop("IOHKCryptoKit", name)
swiftCinterop("IOHKSecureRandomGeneration", name)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be inside of

if (os.isMacOsX)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Funny enough it should not :D the linux passes fine, and I took them out of it, because on linux the build would fail with the Swift Packages plugin and no Apple target.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have a strong opinion on this. So we can ignore it.

@goncalo-frade-iohk goncalo-frade-iohk force-pushed the feature/workingIOSLibs branch 3 times, most recently from 9af2bf2 to d8b1be5 Compare August 7, 2023 13:40
This change was needed because ios requires a dependency manager for any dependency.
Using cocoapods for internal xcode projects would not work quite right when trying to use Apollo as a framework.
Copy link
Contributor

@hamada147 hamada147 left a comment

Choose a reason for hiding this comment

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

LGTM

@goncalo-frade-iohk goncalo-frade-iohk merged commit 2865b24 into main Aug 8, 2023
3 checks passed
@goncalo-frade-iohk goncalo-frade-iohk deleted the feature/workingIOSLibs branch August 8, 2023 09:02
hamada147 added a commit that referenced this pull request May 20, 2024
Co-authored-by: Ahmed Moussa <[email protected]>
Signed-off-by: Goncalo Frade <[email protected]>
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