Making some build flags conditional on a package's `swift-tools-version`

I have been looking into fixing issues such as [SR-12758] C wrapper library requires custom module map in Swift 5.2 · Issue #4550 · apple/swift-package-manager · GitHub, which was an unintended side effect of a bugfix that required a new compiler flag to be passed when building a package.

The current version of the proposed fix is at [SR-12758] Clang targets imported by other Clang targets should only use modulemaps for tools version 5.2 or later by abertelrud · Pull Request #2844 · apple/swift-package-manager · GitHub. Since this is a general class of problem, and since there are some trade-offs in this specific case, I wanted to see if anyone has opinions about the nature of the proposed fix (or a better idea for how to address this type of problem).

Every Swift package specifies a swift-tools-version that tells Swift Package Manager what version of the PackageDescription API to use when parsing the manifest. This allows older packages to continue working even when API changes are introduced in new versions of SwiftPM. The tools version also affects the semantics used when interpreting the file system layout of the package.

Until now, the tools version has not affected the compiler or linker flags that are passed during a build. But in order to fix [SR-12758] C wrapper library requires custom module map in Swift 5.2 · Issue #4550 · apple/swift-package-manager · GitHub, and possibly in future cases, I think it would make sense to take the declared tools version into account when new build flags are added that change the semantics even for older packages.

Complicating this particular situation is that the original change went into SwiftPM 5.2, so it's been in use for a while. Since the flag is being passed unconditionally, this caused some older packages to no longer build with SwiftPM 5.2 and later. But if passing this flag is now made conditional on tools-version 5.2 or newer, it's possible that it might break some newer packages that specify a pre-5.2 tools-version but that rely on the 5.2 behavior (these packages could be fixed by specifying a 5.2 tools-version, however).

So: does anyone object to the general idea of conditionalizing build flags on the tools version as needed in order to preserve older package semantics? And does anyone have a better idea about how to address the specific issue of [SR-12758] C wrapper library requires custom module map in Swift 5.2 · Issue #4550 · apple/swift-package-manager · GitHub without risking breaking any newer packages that rely on post-5.2 behavior for this flag but that specify a pre-5.1 tools version?

It looks like we already shipped the change in question as part of Swift 5.1.2 (at least the one that was included in Xcode 11.2.1), this what I see with the example from SR-12758:

Foo $ swift -version
Apple Swift version 5.1.2 (swiftlang-1100.0.278 clang-1100.0.33.9)
Target: x86_64-apple-darwin19.4.0
Foo $ swift build 2>&1|head -n 5
While building module 'Clibuv' imported from /Users/neonacho1/Downloads/Foo/Sources/Foo/main.c:2:
In file included from <module-includes>:3:
/Users/neonacho1/Downloads/Foo/Sources/Clibuv/libuv/include/uv/bsd.h:25:9: warning: 'UV_PLATFORM_FS_EVENT_FIELDS' macro redefined [-Wmacro-redefined]
#define UV_PLATFORM_FS_EVENT_FIELDS                                           \

We did not land it in open source until Swift 5.2 which makes the situation even more tricky.

As a broad answer to @abertelrud, I think it’s completely reasonable to conditionalize potentially-breaking compile flags on tools version. I don’t think I understand the specific case well enough to provide feedback, but the general idea seems sound.

1 Like

That's unfortunate. That also means that the behavior is different between macOS and other platforms for 5.1.2 when the SwiftPM tool on macOS is coming from Xcode (which it often is).

Going forward, I think it's really important that the same SwiftPM version have the same semantics in open source and in any IDE that bundles it (to the greatest extent possible).

We should also try to make the semantics in any IDE that links against libSwiftPM be as close as possible to the semantics in the SwiftPM tool. That's trickier, since the IDE usually has its own build system that it's fitting packages into. But by exposing more of the semantic intent as return values from libSwiftPM package graph parsing (in this case about whether or not a Clang target should vend a module map to another Clang target), and by making the decisions be based on the tools version, there would at least be something unambiguous on which the IDE can base its decision about whether to pass the flag.

All that said: it's a tricky situation, but in this case we have to choose between leaving those packages broken, or making SwiftPM post-5.3 restore the previous semantics for 5.1 packages. Especially because the open source toolchain made the change at 5.2, I think 5.1 vs 5.2 is the right tools version to use to choose the new semantics.

We should also document the semantics implied by each tools version; I'll file a JIRA for that.

1 Like