[5.5] Build PackageDescription and PackagePlugin universal on macOS platform (#3536)

abertelrud · web-flow · commit 0a2ac74ea62c · 2021-07-06T09:57:08.000-07:00
* [5.5] Bootstrap script needs to build PackageDescription and PackagePlugin libraries universal when cross-compiling. Also unify and clean up some of the logic by making the helper function that installs libSwiftPM be more generic, and also apply to PackageDescription and PackagePlugin. rdar://75186958 (cherry picked from commit 637fa7a) * [5.5] Only pass `-enable-library-evolution` for PackageDescription and PackagePlugin on macOS Because PackageDescription unintentionally exports Foundation (for which a fix was attempted but then reverted after it broke some packages), we can only enable library evolution on Darwin platforms. rdar://78827075 (cherry picked from commit 6e7bb87) * [5.5] Enable swift module interfaces if the package author enables library evolution via unsafe flags There isn't currently a way for package authors to enable library evolution or module interfaces from the package manifest. They can pass `-enable-library-evolution` in their unsafe flags, but because `-emit-module-interface` requires a path parameter, it isn't something that can be set in the manifest. This adds a way to infer XCBuild settings based on values set in manifest-declared settings. The idea is to implement semantics appropriately for each platform based on generalized flags passed from the manifest. rdar://78773077 (cherry picked from commit 4569c7f) * Filter out `*.swiftmodule` and `Project` entries when installing binaries, since these should not be part of the installed toolchain. (cherry picked from commit 3741792)
diff --git a/Fixtures/Miscellaneous/LibraryEvolution/Package.swift b/Fixtures/Miscellaneous/LibraryEvolution/Package.swift
@@ -0,0 +1,11 @@
+// swift-tools-version:5.1
+import PackageDescription
+
+let package = Package(
+    name: "LibraryEvolution",
+    products: [
+    ],
+    targets: [
+        .target(name: "A", dependencies: [], swiftSettings: [.unsafeFlags(["-enable-library-evolution"])]),
+        .target(name: "B", dependencies: ["A"], swiftSettings: [.unsafeFlags(["-enable-library-evolution"])]),
+    ])
diff --git a/Fixtures/Miscellaneous/LibraryEvolution/Sources/A/A.swift b/Fixtures/Miscellaneous/LibraryEvolution/Sources/A/A.swift
diff --git a/Fixtures/Miscellaneous/LibraryEvolution/Sources/B/B.swift b/Fixtures/Miscellaneous/LibraryEvolution/Sources/B/B.swift
diff --git a/Package.swift b/Package.swift
@@ -120,7 +120,7 @@ let package = Package(
             name: "PackageDescription",
             swiftSettings: [
                 .unsafeFlags(["-package-description-version", "999.0"]),
-                .unsafeFlags(["-enable-library-evolution"])
+                .unsafeFlags(["-enable-library-evolution"], .when(platforms: [.macOS]))
             ]),
 
         // The `PackagePlugin` target provides the API that is available to
@@ -130,7 +130,7 @@ let package = Package(
             name: "PackagePlugin",
             swiftSettings: [
                 .unsafeFlags(["-package-description-version", "999.0"]),
-                .unsafeFlags(["-enable-library-evolution"])
+                .unsafeFlags(["-enable-library-evolution"], .when(platforms: [.macOS]))
             ]),
 
         // MARK: SwiftPM specific support libraries
diff --git a/Sources/Build/BuildPlan.swift b/Sources/Build/BuildPlan.swift
@@ -783,14 +783,14 @@ public final class SwiftTargetBuildDescription {
             args += ["-color-diagnostics"]
         }
 
-        // Add the output for the `.swiftinterface`, if requested.
-        if buildParameters.enableParseableModuleInterfaces {
-            args += ["-emit-parseable-module-interface-path", parseableModuleInterfaceOutputPath.pathString]
-        }
-
         // Add agruments from declared build settings.
         args += self.buildSettingsFlags()
 
+        // Add the output for the `.swiftinterface`, if requested or if library evolution has been enabled some other way.
+        if buildParameters.enableParseableModuleInterfaces || args.contains("-enable-library-evolution") {
+            args += ["-emit-module-interface-path", parseableModuleInterfaceOutputPath.pathString]
+        }
+
         // User arguments (from -Xswiftc) should follow generated arguments to allow user overrides
         args += buildParameters.swiftCompilerFlags
         return args
diff --git a/Sources/PackageDescription/CMakeLists.txt b/Sources/PackageDescription/CMakeLists.txt
@@ -21,10 +21,10 @@ add_library(PackageDescription
 
 target_compile_options(PackageDescription PUBLIC
   $<$<COMPILE_LANGUAGE:Swift>:-package-description-version$<SEMICOLON>999.0>)
-target_compile_options(PackageDescription PUBLIC
-  $<$<COMPILE_LANGUAGE:Swift>:-enable-library-evolution>)
 
 if(CMAKE_HOST_SYSTEM_NAME STREQUAL Darwin)
+  target_compile_options(PackageDescription PUBLIC
+    $<$<COMPILE_LANGUAGE:Swift>:-enable-library-evolution>)
   set(SWIFT_INTERFACE_PATH ${CMAKE_BINARY_DIR}/pm/ManifestAPI/PackageDescription.swiftinterface)
   target_compile_options(PackageDescription PUBLIC
     $<$<COMPILE_LANGUAGE:Swift>:-emit-module-interface-path$<SEMICOLON>${SWIFT_INTERFACE_PATH}>)
diff --git a/Sources/PackagePlugin/CMakeLists.txt b/Sources/PackagePlugin/CMakeLists.txt
@@ -17,10 +17,10 @@ add_library(PackagePlugin
 
 target_compile_options(PackagePlugin PUBLIC
   $<$<COMPILE_LANGUAGE:Swift>:-package-description-version$<SEMICOLON>999.0>)
-target_compile_options(PackagePlugin PUBLIC
-  $<$<COMPILE_LANGUAGE:Swift>:-enable-library-evolution>)
   
 if(CMAKE_HOST_SYSTEM_NAME STREQUAL Darwin)
+  target_compile_options(PackagePlugin PUBLIC
+    $<$<COMPILE_LANGUAGE:Swift>:-enable-library-evolution>)
   set(SWIFT_INTERFACE_PATH ${CMAKE_BINARY_DIR}/pm/PluginAPI/PackagePlugin.swiftinterface)
   target_compile_options(PackagePlugin PUBLIC
     $<$<COMPILE_LANGUAGE:Swift>:-emit-module-interface-path$<SEMICOLON>${SWIFT_INTERFACE_PATH}>)
diff --git a/Sources/XCBuildSupport/PIF.swift b/Sources/XCBuildSupport/PIF.swift
@@ -936,6 +936,7 @@ public enum PIF {
             case WATCHOS_DEPLOYMENT_TARGET
             case MARKETING_VERSION
             case CURRENT_PROJECT_VERSION
+            case SWIFT_EMIT_MODULE_INTERFACE
         }
 
         public enum MultipleValueSetting: String, Codable {
diff --git a/Sources/XCBuildSupport/PIFBuilder.swift b/Sources/XCBuildSupport/PIFBuilder.swift
@@ -812,6 +812,21 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder {
 
         return bundleName
     }
+    
+    // Add inferred build settings for a particular value for a manifest setting and value.
+    private func addInferredBuildSettings(
+        for setting: PIF.BuildSettings.MultipleValueSetting,
+        value: [String],
+        platform: PIF.BuildSettings.Platform? = nil,
+        configuration: BuildConfiguration,
+        settings: inout PIF.BuildSettings
+    ) {
+        // Automatically set SWIFT_EMIT_MODULE_INTERFACE if the package author uses unsafe flags to enable
+        // library evolution (this is needed until there is a way to specify this in the package manifest).
+        if setting == .OTHER_SWIFT_FLAGS && value.contains("-enable-library-evolution") {
+            settings[.SWIFT_EMIT_MODULE_INTERFACE] = "YES"
+        }
+    }
 
     // Apply target-specific build settings defined in the manifest.
     private func addManifestBuildSettings(
@@ -833,8 +848,10 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder {
                             switch configuration {
                             case .debug:
                                 debugSettings[setting, for: platform, default: ["$(inherited)"]] += value
+                                addInferredBuildSettings(for: setting, value: value, platform: platform, configuration: .debug, settings: &debugSettings)
                             case .release:
                                 releaseSettings[setting, for: platform, default: ["$(inherited)"]] += value
+                                addInferredBuildSettings(for: setting, value: value, platform: platform, configuration: .release, settings: &releaseSettings)
                             }
                         }
 
@@ -847,8 +864,10 @@ final class PackagePIFProjectBuilder: PIFProjectBuilder {
                         switch configuration {
                         case .debug:
                             debugSettings[setting, default: ["$(inherited)"]] += value
+                            addInferredBuildSettings(for: setting, value: value, configuration: .debug, settings: &debugSettings)
                         case .release:
                             releaseSettings[setting, default: ["$(inherited)"]] += value
+                            addInferredBuildSettings(for: setting, value: value, configuration: .release, settings: &releaseSettings)
                         }
                     }
 
diff --git a/Tests/CommandsTests/BuildToolTests.swift b/Tests/CommandsTests/BuildToolTests.swift
@@ -253,6 +253,18 @@ final class BuildToolTests: XCTestCase {
         }
     }
 
+    func testAutomaticParseableInterfacesWithLibraryEvolution() {
+        fixture(name: "Miscellaneous/LibraryEvolution") { path in
+            do {
+                let result = try build([], packagePath: path)
+                XCTAssert(result.binContents.contains("A.swiftinterface"))
+                XCTAssert(result.binContents.contains("B.swiftinterface"))
+            } catch SwiftPMProductError.executionFailure(_, _, let stderr) {
+                XCTFail(stderr)
+            }
+        }
+    }
+
     func testBuildCompleteMessage() {
         fixture(name: "DependencyResolution/Internal/Simple") { path in
             do {
diff --git a/Tests/XCBuildSupportTests/PIFBuilderTests.swift b/Tests/XCBuildSupportTests/PIFBuilderTests.swift
@@ -2224,6 +2224,68 @@ class PIFBuilderTests: XCTestCase {
             }
         }
     }
+    
+    /// Tests that the inference of XCBuild build settings based on the package manifest's declared unsafe settings
+    /// works as expected.
+    func testUnsafeFlagsBuildSettingInference() throws {
+        let fs = InMemoryFileSystem(emptyFiles:
+            "/MyLib/Sources/MyLib/Foo.swift"
+        )
+
+        let diagnostics = DiagnosticsEngine()
+        let graph = try loadPackageGraph(
+            fs: fs,
+            diagnostics: diagnostics,
+            manifests: [
+                Manifest.createManifest(
+                    name: "MyLib",
+                    path: "/MyLib",
+                    packageKind: .root,
+                    packageLocation: "/MyLib",
+                    v: .v5,
+                    products: [
+                        .init(name: "MyLib", type: .library(.automatic), targets: ["MyLib"]),
+                    ],
+                    targets: [
+                        .init(name: "MyLib", settings: [
+                            .init(
+                                tool: .swift,
+                                name: .unsafeFlags,
+                                value: ["-enable-library-evolution"],
+                                condition: .init(config: "release")),
+                        ]),
+                    ]),
+            ],
+            shouldCreateMultipleTestProducts: true
+        )
+
+        let builder = PIFBuilder(graph: graph, parameters: .mock(), diagnostics: diagnostics)
+        let pif = try builder.construct()
+
+        XCTAssertNoDiagnostics(diagnostics)
+
+        PIFTester(pif) { workspace in
+            workspace.checkProject("PACKAGE:/MyLib") { project in
+                project.checkTarget("PACKAGE-TARGET:MyLib") { target in
+                    target.checkBuildConfiguration("Debug") { configuration in
+                        configuration.checkBuildSettings { settings in
+                            // Check that the `-enable-library-evolution` setting for Release didn't affect Debug.
+                            XCTAssertEqual(settings[.SWIFT_EMIT_MODULE_INTERFACE], nil)
+                            XCTAssertEqual(settings[.OTHER_SWIFT_FLAGS], nil)
+                        }
+                    }
+                    target.checkBuildConfiguration("Release") { configuration in
+                        configuration.checkBuildSettings { settings in
+                            // Check that the `-enable-library-evolution` setting for Release also set SWIFT_EMIT_MODULE_INTERFACE.
+                            XCTAssertEqual(settings[.SWIFT_EMIT_MODULE_INTERFACE], "YES")
+                            XCTAssertEqual(settings[.OTHER_SWIFT_FLAGS], ["$(inherited)", "-enable-library-evolution"])
+                        }
+                    }
+                }
+            }
+        }
+    }
+
   #endif
 }
 
diff --git a/Utilities/bootstrap b/Utilities/bootstrap
@@ -15,8 +15,6 @@
 from __future__ import print_function
 
 import argparse
-from distutils import dir_util
-from distutils import file_util
 import json
 import os
 import platform
@@ -27,10 +25,11 @@ from helpers import note, error, symlink_force, mkdir_p, call, call_output
 
 g_macos_deployment_target = '10.15'
 
+g_shared_lib_prefix = "lib"
 if platform.system() == 'Darwin':
-    g_shared_lib_ext = ".dylib"
+    g_shared_lib_suffix = ".dylib"
 else:
-    g_shared_lib_ext = ".so"
+    g_shared_lib_suffix = ".so"
 
 def main():
     parser = argparse.ArgumentParser(description="""
@@ -380,7 +379,7 @@ def install(args):
             "PackageGraph", "SPMBuildCore", "Build",
             "Xcodeproj", "Workspace"
         ]
-        install_libswiftpm_dylib(args, "SwiftPM", args.libswiftpm_install_dir, libswiftpm_modules)
+        install_dylib(args, "SwiftPM", args.libswiftpm_install_dir, libswiftpm_modules)
 
     # Install libSwiftPMDataModel if an install directory was provided.
     if args.libswiftpmdatamodel_install_dir:
@@ -391,82 +390,59 @@ def install(args):
             "PackageGraph", "SPMBuildCore",
             "Xcodeproj", "Workspace"
         ]
-        install_libswiftpm_dylib(args, "SwiftPMDataModel", args.libswiftpmdatamodel_install_dir, libswiftpmdatamodel_modules)
+        install_dylib(args, "SwiftPMDataModel", args.libswiftpmdatamodel_install_dir, libswiftpmdatamodel_modules)
 
+# Installs the SwiftPM tools and runtime support libraries.
 def install_swiftpm(prefix, args):
     # Install swiftpm binaries.
     for binary in ["swift-build", "swift-test", "swift-run", "swift-package", "swift-package-collection"]:
         dest = os.path.join(prefix, "bin")
         install_binary(args, binary, dest)
 
+    # On Darwin, also install the swiftpm-xctest-helper tool.
     if platform.system() == 'Darwin':
         dest = os.path.join(prefix, "libexec", "swift", "pm")
         install_binary(args, "swiftpm-xctest-helper", dest)
 
-    # Install PackageDescription runtime libraries.
-    runtime_lib_dest = os.path.join(prefix, "lib", "swift", "pm")
-    runtime_lib_src = os.path.join(args.bootstrap_dir, "pm")
+    # Install the PackageDescription library and associated modules.
+    dest = os.path.join(prefix, "lib", "swift", "pm", "ManifestAPI")
+    install_dylib(args, "PackageDescription", dest, ["PackageDescription"])
 
-    files_to_install = ["libPackageDescription" + g_shared_lib_ext]
-    if platform.system() == 'Darwin':
-        files_to_install.append("PackageDescription.swiftinterface")
-    else:
-        files_to_install.append("PackageDescription.swiftmodule")
-    files_to_install.append("PackageDescription.swiftdoc")
-
-    for file in files_to_install:
-        src = os.path.join(runtime_lib_src, "ManifestAPI", file)
-        dest = os.path.join(runtime_lib_dest, "ManifestAPI", file)
-        mkdir_p(os.path.dirname(dest))
-
-        note("Installing %s to %s" % (src, dest))
-
-        file_util.copy_file(src, dest, update=1)
-
-    files_to_install = ["libPackagePlugin" + g_shared_lib_ext]
-    if platform.system() == 'Darwin':
-        files_to_install.append("PackagePlugin.swiftinterface")
-    else:
-        files_to_install.append("PackagePlugin.swiftmodule")
-    files_to_install.append("PackagePlugin.swiftdoc")
-
-    for file in files_to_install:
-        src = os.path.join(runtime_lib_src, "PluginAPI", file)
-        dest = os.path.join(runtime_lib_dest, "PluginAPI", file)
-        mkdir_p(os.path.dirname(dest))
-
-        note("Installing %s to %s" % (src, dest))
-
-        file_util.copy_file(src, dest, update=1)
+    # Install the PackagePlugin library and associated modules.
+    dest = os.path.join(prefix, "lib", "swift", "pm", "PluginAPI")
+    install_dylib(args, "PackagePlugin", dest, ["PackagePlugin"])
 
 
-def install_libswiftpm_dylib(args, library_name, install_dir, module_names):
-    # FIXME: Don't hardcode the prefix and suffix.
-    install_binary(args, "lib" + library_name + ".dylib", install_dir)
+# Helper function that installs a dynamic library and a set of modules to a particular directory.
+def install_dylib(args, library_name, install_dir, module_names):
+    # Install the dynamic library itself.
+    install_binary(args, g_shared_lib_prefix + library_name + g_shared_lib_suffix, install_dir)
 
-    # Install the swiftmodule and swiftdoc files.
+    # Install the swiftmodule/swiftinterface and swiftdoc files for all the modules.
     for module in module_names:
-        install_binary(args, module + ".swiftmodule", install_dir)
-        if not args.cross_compile_hosts: # When compiling for multiple arches, swiftdoc is part of the swiftmodule directory
+        # If we're cross-compiling, we expect the .swiftmodule to be a directory that contains everything.
+        if args.cross_compile_hosts:
+            install_binary(args, module + ".swiftmodule", install_dir, ['Project', '*.swiftmodule'])
+        else:
+            # Otherwise we have either a .swiftinterface or a .swiftmodule, plus a .swiftdoc.
+            if os.path.exists(os.path.join(args.bin_dir, module + ".swiftinterface")):
+                install_binary(args, module + ".swiftinterface", install_dir)
+            else:
+                install_binary(args, module + ".swiftmodule", install_dir)
             install_binary(args, module + ".swiftdoc", install_dir)
 
-    # Install the C headers.
-    tscclibc_include_dir = os.path.join(args.tsc_source_dir, "Sources/TSCclibc/include")
-    tscclibc_include_dir_dest = os.path.join(install_dir, "TSCclibc")
-    dir_util.copy_tree(tscclibc_include_dir, tscclibc_include_dir_dest)
-
 
-def install_binary(args, binary, dest_dir):
+# Helper function that installs a single built artifact to a particular directory. The source may be either a file or a directory.
+def install_binary(args, binary, dest_dir, ignored_patterns=[]):
     src = os.path.join(args.bin_dir, binary)
     dest = os.path.join(dest_dir, binary)
 
     note("Installing %s to %s" % (src, dest))
-
     mkdir_p(os.path.dirname(dest))
-    if os.path.isdir(src) and args.cross_compile_hosts: # Handle swiftmodule directories if compiling for multiple arches.
-        dir_util.copy_tree(src, dest)
+    if os.path.isdir(src):
+        shutil.copytree(src, dest, ignore=shutil.ignore_patterns(*ignored_patterns))
     else:
-        file_util.copy_file(src, dest, update=1)
+        shutil.copy2(src, dest)
 
 # -----------------------------------------------------------
 # Build functions
@@ -653,6 +629,7 @@ def build_swiftpm_with_swiftpm(args, integrated_swift_driver):
     if integrated_swift_driver:
         swiftpm_args.append("--use-integrated-swift-driver")
 
+    # Build SwiftPM, including libSwiftPM, all the command line tools, and the current variant of PackageDescription.
     call_swiftpm(args, swiftpm_args)
 
     # Setup symlinks that'll allow using swiftpm from the build directory.
@@ -778,7 +755,7 @@ def get_swiftpm_flags(args):
         swift_library_rpath_prefix = "$ORIGIN/../"
     platform_path = None
     for path in args.target_info["paths"]["runtimeLibraryPaths"]:
-        platform_path = re.search(r"(lib/swift/[^/]+)$", path)
+        platform_path = re.search(r"(lib/swift/([^/]+))$", path)
         if platform_path:
             build_flags.extend(
                 [
@@ -788,6 +765,15 @@ def get_swiftpm_flags(args):
                     swift_library_rpath_prefix + platform_path.group(1),
                 ]
             )
+            if platform.system() == 'Linux':
+                build_flags.extend(
+                    [
+                        "-Xlinker",
+                        "-rpath",
+                        "-Xlinker",
+                        swift_library_rpath_prefix + '../' + platform_path.group(2),
+                    ]
+                )
             break
 
     if not platform_path:

Original file line number	Diff line number	Diff line change
`@@ -936,6 +936,7 @@ public enum PIF {`
`936`	`936`	`case WATCHOS_DEPLOYMENT_TARGET`
`937`	`937`	`case MARKETING_VERSION`
`938`	`938`	`case CURRENT_PROJECT_VERSION`
	`939`	`+ case SWIFT_EMIT_MODULE_INTERFACE`
`939`	`940`	`}`
`940`	`941`
`941`	`942`	`public enum MultipleValueSetting: String, Codable {`