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

swift-certificate does not provide stable ABI #160

Closed
oliviermartin opened this issue Jan 10, 2024 · 7 comments
Closed

swift-certificate does not provide stable ABI #160

oliviermartin opened this issue Jan 10, 2024 · 7 comments

Comments

@oliviermartin
Copy link

To build a distributable binary library (aka XCFramework), you should build your library with `BUILD_LIBRARY_FOR_DISTRIBUTION: https://developer.apple.com/documentation/xcode/build-settings-reference#Build-Libraries-for-Distribution

Currently swift-certificate does not fit to this requirement. To verify it:

xcodebuild build -scheme swift-certificates -destination "generic/platform=macOS" BUILD_LIBRARY_FOR_DISTRIBUTION=YES

Clearly the main issue is the overuse of @inlinable - removing this compiler hint in many places fix the build.

The big picture is not to only build a binary library from swift-certificates alone but building my own third-party binary library that depends on swift-certificate.
But it looks like building my own library with BUILD_LIBRARY_FOR_DISTRIBUTION=YES requires all its dependencies to be able to build with BUILD_LIBRARY_FOR_DISTRIBUTION=YES.
An alternative seems to be the non documented option @_implementationOnly that might keep the benefit from @inlinable in swift-certificate but it does not seem to work.

To demonstrate @_implementationOnly is not working as I would expect is to replace all import SwiftASN1 with @_implementationOnly import SwiftASN1 in swift-certificates. The build phase also complains about the @inlinable use in SwiftASN1.

This issue also affects:

@FranzBusch
Copy link
Member

In general, the open source packages that you mentioned do not allow to be build with library evolution enabled since none of those packages have declared ABI stability. Making a package ABI stable is a huge undertaking and requires significant understanding of what that means for future evolvelability; hence, I am going to close this issue since we won't be making specific changes to enable build with library evolutions.

However, you should be able to use @_implementationOnly for this. It might require you to fork the projects and annotate the imports appropriately. Importantly enough make sure that your framework imports any of these packages also with the @_implementationOnly annotation. Lastly, you should check out the access level import Swift evolution proposal which provides an official replace for @_implementationOnly import via internal import.

@oliviermartin
Copy link
Author

Thanks @FranzBusch , unfortunately that what I explained in the second part of my issue. I replaced in my forked swift-certificates all import SwiftASN1 by @_implementationOnly import SwiftASN1 and when building swift-certificates with xcodebuild build -scheme swift-certificates -destination "generic/platform=macOS" BUILD_LIBRARY_FOR_DISTRIBUTION=YES, I still see errors related to @inlinable in swift-asn1.
Have I missed something?

@FranzBusch
Copy link
Member

You shouldn't need to compile ASN1 or certificates with library evolution enabled but just your end XCFramework. That XCFramework also needs to make sure to only import ASN1/certificates with @_implementationOnly. However, even then this might still not work. The errors that you see in ASN1 are mostly due to us using self in inits before self has been assigned. This appears to be a compiler bug and you ought to be able to fix this by changing some of the code in your fork.

Lastly, you have to be very careful with what you ship to your end users though. If you include an XCFramework of ASN1/certificates that means your users are not able to use the package version of them anymore.

@Lukasa
Copy link
Contributor

Lukasa commented Jan 10, 2024

To be very specific, you should compile swift-certificates and swift-asn1 without BUILD_LIBRARY_FOR_DISTRIBUTION, and compile them into static libraries. You can then link them into your ultimate framework. As @FranzBusch says, only the usage sites need to be compiled with BUILD_LIBRARY_FOR_DISTRIBUTION, and those places should use @_implementationOnly.

@oliviermartin
Copy link
Author

The errors that you see in ASN1 are mostly due to us using self in inits before self has been assigned. This appears to be a compiler bug and you ought to be able to fix this by changing some of the code in your fork.

@FranzBusch it is actually the issue I have mostly seen. Is there any public ticket for this bug and/or a fixed version for the compiler?

@FranzBusch
Copy link
Member

@FranzBusch it is actually the issue I have mostly seen. Is there any public ticket for this bug and/or a fixed version for the compiler?

After looking at this again the problem is not actually the self assignment but rather the @inlineable init on a non-frozen struct. The error diagnosis here is just a bit off. The problem is that an @inlinable init gets emitted into the public API/ABI of this module and user-code might inline that even further in their code. This means that the init must guarantee that all properties of self are initialised otherwise adding a new property would leave the struct partially initialised. There are two ways to work around this. Either make the struct @frozen which tells the compiler no new properties will be added or introduce a non-inlinable init.

@oliviermartin
Copy link
Author

Thanks @FranzBusch . That's what I also noticed, removing @inlineable was moving the problem away.

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

No branches or pull requests

3 participants