SE-0272 (Review #2): Package Manager Binary Dependencies

Hello Swift community,

The second review of SE-0272 “Package Manager Binary Dependencies" begins now and runs through December 20, 2019.

Reviews are an important part of the Swift evolution process. All reviews should be made in this thread on the Swift forums or, if you would like to keep your feedback private, directly in email to me as the review manager.

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  • What is your evaluation of the proposal?
  • Is the problem being addressed significant enough to warrant a change to the Swift Package Manager?
  • Does this proposal fit well with the feel and direction of Swift?
  • If you have used other languages, libraries, or package managers with a similar feature, how do you feel that this proposal compares to those?
  • How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available on the Swift Evolution website.

As always, thank you for participating in Swift Evolution.

Boris Buegling
Review Manager

6 Likes

When resolving a package that contains a binary dependency on non-Apple platforms, SwiftPM will throw an error and explicitly state that this dependency is not valid for the current platform.

How will this interact with conditional dependencies? I’m assuming no error will be thrown unless the binary target is declared as actually required?

Could you please post a link to the diff? It would simplify review for those of us who have read the initial version of the proposal.

Diff link:

https://github.com/apple/swift-evolution/commit/8cab81d6d2fa8b6a6aad374d2af4201b6634246d#diff-e9aade9898d80a80900542f3e5282a74

4 Likes

I think it'll depend on which proposal gets implemented first whether it needs to be handled here or if conditional dependencies also needs to consider binaries, but I agree that your assumption is how this should work once we have both features.

1 Like

I was thinking that SwiftPM would actually fail only if one tries to build a target/product which depends on a binary target on a non-Apple platform. If its implemented that way, then everything would automatically work, as the implementation of the conditional dependency proposal I wrote disregards those dependencies at build time.

1 Like

Btw, what do people think of disallowing references to .zip artefacts for local binary targets? That would greatly simplify the implementation and zips only seem useful for remote artefacts.

1 Like

This sounds reasonable to me. We want to use the latest binary artifact on disk for local packages and having them zipped would complicate that quite a bit.

I think a good workflow for local development of binary packages could be having two packages, one that includes the unzipped binary artifact that gets referenced locally and a second one for publishing with references the binary via HTTP. It seems like zipping up the binary would be an unnecessary complication for the user as well.

1 Like

Makes sense to me as well, I just kept it to keep the formats consistent but if we drop it and just use the unzipped format I am fine as well.

Would it make debugging a package locally a challenge?

I'm looking at this proposal from the perspective of "does this let us package Realm as a swift package?". For context, Realm has the following library structure:

  1. An open-source C++ library.
  2. A closed-source C++ library which depends on the open-source library.
  3. An open-source obj-c++ library which depends on the open-source c++ library and has an optional dependency on the closed-source library.
  4. An open-source Swift library which wraps the obj-c++ library.

The Swift library does not support non-Apple platforms (and cannot, due to the dependency on the obj-c runtime). Currently the closed-source optional dependency is simply not supported when using swiftpm.

If the proposal shipped as-is the answer would be (eventually) yes, but with some caveats:

  1. We'd ideally have the binary package be an optional dependency. Some users would prefer to only use the open-source components for legal or philosophical reasons. This is probably a separate proposal though, and could be added later.
  2. Because we share c++ standard library data structures between libraries, we need all libraries to be built with the same sanitizer settings. With Carthage this isn't an issue as the libraries are built completely separately from the app, and enabling sanitizers in the Xcode scheme settings only enables it for the app (which is fine). For CocoaPods we ship separate builds of the closed-source library which have asan or tsan enabled and link the appropriate ones based on the build settings. With SwiftPM enabling asan/tsan in Xcode's scheme options will build the open-source library with the sanitizer, then crash at runtime because it's passing data structures to a library built without it, and there doesn't appear to be any way to supply a different binary. This means that adopting a binary package will render our users unable to use sanitizers to find problems in their apps. An alternative option to being able to supply a different binary would be for us to be able to indicate in some way that a target should not inherit sanitizer settings from the project.
  3. We wouldn't actually be able to use any of this until we drop support for Xcode 11. This is just a general problem with how tool versions works for the manifest file and not anything specific to this proposal, though.

I wasn't aware of this issue and swift build --sanitize will presumably have the same problem. Is this specific to C++ code?

It would be nice if we could automatically handle this case somehow, e.g. opt out package dependencies which use binary artifacts from being build without sanitizer settings, but I guess that would still leave issues where a C++ binary is used directly by client code or where source-based targets from a package with binary artifacts are written in C++ and used by client code.

The review of SE-0272: Package Manager Binary Dependencies ran from December 13th through December 20th, 2019. The proposal is accepted with modifications . Thanks to everyone who participated!

Boris Buegling
Review Manager

2 Likes

Yes, swift build --sanitize also builds all dependencies with sanitizers enabled (which normally is a very good thing!).

With asan the problem is with container overflow checks. libc++ annotates types like std::vector so that reading past the end of the vector can be caught even if it happens to be inside a valid allocation. If the std::vector was allocated or resized by code not built with asan, this results in a false positive. Automatically running with ASAN_OPTIONS=detect_container_overflow=0 if a package has any binary dependencies is probably a perfectly acceptable solution from the perspective of SwiftPM?

tsan has similar problems where it doesn't know about any synchronization that happens in uninstrumented libraries. This means that in a hypothetical callWithLock { ... } where the actual locking happens inside a binary dependency tsan will incorrectly report a data race. This could theoretically happen even in a pure-swift setup where a swift app depends on a swift binary framework. However, this only matters if the synchronization doesn't go through something which compiler-rt has an interceptor for. In our case the only thing that's an issue is some uses of explicit memory barriers, and after looking at it again I think I can make it work with the closed-source library being built without tsan.

TL;DR: automatically setting ASAN_OPTIONS=detect_container_overflow=0 in swift run --sanitize=address (and test) if the target has a binary dependency is probably sufficient and this is less of an issue than I thought at first.

1 Like