Allow test targets to link against executable targets so they can import and test the code. Since compiler support isn't yet available, this implementation elides the _main symbol from the linked binary (using nmedit on Darwin and objcopy elsewhere). To produce alternate versions of .o files for the unit test to link against.

abertelrud · abertelrud · commit 1f58ae700c8e · 2021-01-10T12:05:58.000-08:00
diff --git a/Fixtures/Miscellaneous/TestableExe/Package.swift b/Fixtures/Miscellaneous/TestableExe/Package.swift
@@ -0,0 +1,25 @@
+// swift-tools-version:5.3
+import PackageDescription
+
+let package = Package(
+    name: "TestableExe",
+    targets: [
+        .target(
+            name: "TestableExe1"
+        ),
+        .target(
+            name: "TestableExe2"
+        ),
+        .target(
+            name: "TestableExe3"
+        ),
+        .testTarget(
+            name: "TestableExeTests",
+            dependencies: [
+                "TestableExe1",
+                "TestableExe2",
+                "TestableExe3",
+            ]
+        ),
+    ]
+)
diff --git a/Fixtures/Miscellaneous/TestableExe/Sources/TestableExe1/main.swift b/Fixtures/Miscellaneous/TestableExe/Sources/TestableExe1/main.swift
@@ -0,0 +1,5 @@
+public func GetGreeting1() -> String {
+    return "Hello, world"
+}
+
+print("\(GetGreeting1())!")
diff --git a/Fixtures/Miscellaneous/TestableExe/Sources/TestableExe2/main.swift b/Fixtures/Miscellaneous/TestableExe/Sources/TestableExe2/main.swift
@@ -0,0 +1,5 @@
+public func GetGreeting2() -> String {
+    return "Hello, planet"
+}
+
+print("\(GetGreeting2())!")
diff --git a/Fixtures/Miscellaneous/TestableExe/Sources/TestableExe3/include/TestableExe3.h b/Fixtures/Miscellaneous/TestableExe/Sources/TestableExe3/include/TestableExe3.h
@@ -0,0 +1 @@
+const char * GetGreeting3();
diff --git a/Fixtures/Miscellaneous/TestableExe/Sources/TestableExe3/main.c b/Fixtures/Miscellaneous/TestableExe/Sources/TestableExe3/main.c
@@ -0,0 +1,10 @@
+#include <stdio.h>
+#include "include/TestableExe3.h"
+
+const char * GetGreeting3() {
+    return "Hello, universe";
+}
+
+int main() {
+    printf("%s!\n", GetGreeting3());
+}
diff --git a/Fixtures/Miscellaneous/TestableExe/Tests/LinuxMain.swift b/Fixtures/Miscellaneous/TestableExe/Tests/LinuxMain.swift
@@ -0,0 +1,7 @@
+import XCTest
+
+import TestableExeTests
+
+var tests = [XCTestCaseEntry]()
+tests += TestableExeTests.allTests()
+XCTMain(tests)
diff --git a/Fixtures/Miscellaneous/TestableExe/Tests/TestableExeTests/TestableExeTests.swift b/Fixtures/Miscellaneous/TestableExe/Tests/TestableExeTests/TestableExeTests.swift
@@ -0,0 +1,54 @@
+import XCTest
+import TestableExe1
+import TestableExe2
+import TestableExe3
+import class Foundation.Bundle
+
+final class TestableExeTests: XCTestCase {
+    func testExample() throws {
+        // This is an example of a functional test case.
+        // Use XCTAssert and related functions to verify your tests produce the correct
+        // results.
+        
+        XCTAssertEqual(GetGreeting1(), "Hello, world")
+        XCTAssertEqual(GetGreeting2(), "Hello, planet")
+        XCTAssertEqual(String(cString: GetGreeting3()), "Hello, universe")
+
+        // Some of the APIs that we use below are available in macOS 10.13 and above.
+        guard #available(macOS 10.13, *) else {
+            return
+        }
+
+        let fooBinary = productsDirectory.appendingPathComponent("TestableExe1")
+
+        let process = Process()
+        process.executableURL = fooBinary
+
+        let pipe = Pipe()
+        process.standardOutput = pipe
+
+        try process.run()
+        process.waitUntilExit()
+
+        let data = pipe.fileHandleForReading.readDataToEndOfFile()
+        let output = String(data: data, encoding: .utf8)
+
+        XCTAssertEqual(output, "Hello, world!\n")
+    }
+
+    /// Returns path to the built products directory.
+    var productsDirectory: URL {
+      #if os(macOS)
+        for bundle in Bundle.allBundles where bundle.bundlePath.hasSuffix(".xctest") {
+            return bundle.bundleURL.deletingLastPathComponent()
+        }
+        fatalError("couldn't find the products directory")
+      #else
+        return Bundle.main.bundleURL
+      #endif
+    }
+
+    static var allTests = [
+        ("testExample", testExample),
+    ]
+}
diff --git a/Fixtures/Miscellaneous/TestableExe/Tests/TestableExeTests/XCTestManifests.swift b/Fixtures/Miscellaneous/TestableExe/Tests/TestableExeTests/XCTestManifests.swift
@@ -0,0 +1,9 @@
+import XCTest
+
+#if !canImport(ObjectiveC)
+public func allTests() -> [XCTestCaseEntry] {
+    return [
+        testCase(TestableExeTests.allTests),
+    ]
+}
+#endif
diff --git a/Sources/Build/BuildPlan.swift b/Sources/Build/BuildPlan.swift
@@ -259,7 +259,8 @@ public final class ClangTargetBuildDescription {
         self.derivedSources = Sources(paths: [], root: tempsPath.appending(component: "DerivedSources"))
 
         // Try computing modulemap path for a C library.  This also creates the file in the file system, if needed.
-        if target.type == .library {
+        // FIXME: Adding .executable here is probably not right, but is needed in order to test Clang executables.
+        if target.type == .library || target.type == .executable {
             // If there's a custom module map, use it as given.
             if case .custom(let path) = clangTarget.moduleMapType {
                 self.moduleMap = path
@@ -503,8 +504,7 @@ public final class SwiftTargetBuildDescription {
 
     /// The path to the swiftmodule file after compilation.
     var moduleOutputPath: AbsolutePath {
-        let dirPath = (target.type == .executable) ? tempsPath : buildParameters.buildPath
-        return dirPath.appending(component: target.c99name + ".swiftmodule")
+        return buildParameters.buildPath.appending(component: target.c99name + ".swiftmodule")
     }
 
     /// The path to the wrapped swift module which is created using the modulewrap tool. This is required
@@ -1003,7 +1003,7 @@ public final class ProductBuildDescription {
         return buildParameters.binaryPath(for: product)
     }
 
-    /// The objects in this product.
+    /// All object files to link into this product.
     ///
     // Computed during build planning.
     public fileprivate(set) var objects = SortedArray<AbsolutePath>()
@@ -1021,6 +1021,10 @@ public final class ProductBuildDescription {
     /// The list of Swift modules that should be passed to the linker. This is required for debugging to work.
     fileprivate var swiftASTs: SortedArray<AbsolutePath> = .init()
 
+    /// Ordered mapping of any mainless object files used by the product to the originals in the executable
+    /// targets that produce them.
+    public fileprivate(set) var executableObjects: OrderedDictionary<AbsolutePath, AbsolutePath> = .init()
+
     /// Paths to the binary libraries the product depends on.
     fileprivate var libraryBinaryPaths: Set<AbsolutePath> = []
 
@@ -1034,6 +1038,11 @@ public final class ProductBuildDescription {
         return tempsPath.appending(component: "Objects.LinkFileList")
     }
 
+    /// Path to the symbol removal list file (list of symbols to remove from any objects linked into this product).
+    var mainSymbolRemovalListFilePath: AbsolutePath {
+        return tempsPath.appending(component: "MainSymbol.SymbolList")
+    }
+
     /// Diagnostics Engine for emitting diagnostics.
     let diagnostics: DiagnosticsEngine
 
@@ -1191,6 +1200,16 @@ public final class ProductBuildDescription {
         try fs.writeFileContents(linkFileListPath, bytes: stream.bytes)
     }
 
+    /// Writes symbol removal list file to the filesystem.
+    func writeMainSymbolRemovalListFile(_ fs: FileSystem) throws {
+        let stream = BufferedOutputByteStream()
+        
+        stream <<< "_main\n"
+
+        try fs.createDirectory(mainSymbolRemovalListFilePath.parentDirectory, recursive: true)
+        try fs.writeFileContents(mainSymbolRemovalListFilePath, bytes: stream.bytes)
+    }
+
     /// Returns the build flags from the declared build settings.
     private func buildSettingsFlags() -> [String] {
         var flags: [String] = []
@@ -1482,14 +1501,14 @@ public class BuildPlan {
 
         // Link C++ if needed.
         // Note: This will come from build settings in future.
-        for target in dependencies.staticTargets {
+        for target in dependencies.staticTargets + dependencies.executableTargets {
             if case let target as ClangTarget = target.underlyingTarget, target.isCXX {
                 buildProduct.additionalFlags += self.buildParameters.toolchain.extraCPPFlags
                 break
             }
         }
 
-        for target in dependencies.staticTargets {
+        for target in dependencies.staticTargets + dependencies.executableTargets {
             switch target.underlyingTarget {
             case is SwiftTarget:
                 // Swift targets are guaranteed to have a corresponding Swift description.
@@ -1513,7 +1532,7 @@ public class BuildPlan {
             }
         }
 
-        buildProduct.staticTargets = dependencies.staticTargets
+        buildProduct.staticTargets = dependencies.staticTargets + dependencies.executableTargets
         buildProduct.dylibs = try dependencies.dylibs.map{
             guard let product = productMap[$0] else {
                 throw InternalError("unknown product \($0)")
@@ -1527,6 +1546,25 @@ public class BuildPlan {
             return target.objects
         }
         buildProduct.libraryBinaryPaths = dependencies.libraryBinaryPaths
+        
+        // If we're linking against any executable targets, we need to create versions of the .o files from those
+        // targets that elide the `_main` symbol.  We should look into whether linker options can be added to specify
+        // this on the command line.
+        if !dependencies.executableTargets.isEmpty {
+            // Creating a mapping from each .o file in each executable target to a corresponding modified .o file in
+            // our product directory.  This duplicates work if an executable is tested by more than one test product
+            // but has the advantage of keeping the executable target clean unless it's being used by a test target.
+            for target in dependencies.executableTargets.map({ targetMap[$0]! }) {
+                for object in target.objects {
+                    // FIXME: Plenty of opportunity for collisions here — how is this handled for regular object files?
+                    let mainlessObject = buildProduct.tempsPath.appending(components: "LinkedExecutableObjects", "\(target.target.c99name)_\(object.basename)")
+                    buildProduct.executableObjects[mainlessObject] = object
+                    buildProduct.objects.insert(mainlessObject)
+                }
+            }
+            // The symbol removal tool on some platforms requires a separate file list in the file system.
+            try buildProduct.writeMainSymbolRemovalListFile(fileSystem)
+        }
 
         // Write the link filelist file.
         //
@@ -1541,6 +1579,7 @@ public class BuildPlan {
     ) throws -> (
         dylibs: [ResolvedProduct],
         staticTargets: [ResolvedTarget],
+        executableTargets: [ResolvedTarget],
         systemModules: [ResolvedTarget],
         libraryBinaryPaths: Set<AbsolutePath>
     ) {
@@ -1568,6 +1607,7 @@ public class BuildPlan {
         // Create empty arrays to collect our results.
         var linkLibraries = [ResolvedProduct]()
         var staticTargets = [ResolvedTarget]()
+        var executableTargets = [ResolvedTarget]()
         var systemModules = [ResolvedTarget]()
         var libraryBinaryPaths: Set<AbsolutePath> = []
 
@@ -1577,7 +1617,14 @@ public class BuildPlan {
                 switch target.type {
                 // Include executable and tests only if they're top level contents
                 // of the product. Otherwise they are just build time dependency.
-                case .executable, .test:
+                case .executable:
+                    if product.targets.contains(target) {
+                        staticTargets.append(target)
+                    }
+                    else {
+                        executableTargets.append(target)
+                    }
+                case .test:
                     if product.targets.contains(target) {
                         staticTargets.append(target)
                     }
@@ -1612,7 +1659,7 @@ public class BuildPlan {
             }
         }
 
-        return (linkLibraries, staticTargets, systemModules, libraryBinaryPaths)
+        return (linkLibraries, staticTargets, executableTargets, systemModules, libraryBinaryPaths)
     }
 
     /// Plan a Clang target.
@@ -1657,7 +1704,8 @@ public class BuildPlan {
         // depends on.
         for case .target(let dependency, _) in try swiftTarget.target.recursiveDependencies(satisfying: buildEnvironment) {
             switch dependency.underlyingTarget {
-            case let underlyingTarget as ClangTarget where underlyingTarget.type == .library:
+            // FIXME: Adding .executable here is probably not right, but is needed in order to test Clang executables.
+            case let underlyingTarget as ClangTarget where underlyingTarget.type == .library || underlyingTarget.type == .executable:
                 guard case let .clang(target)? = targetMap[dependency] else {
                     fatalError("unexpected clang target \(underlyingTarget)")
                 }
diff --git a/Sources/Build/ManifestBuilder.swift b/Sources/Build/ManifestBuilder.swift
@@ -624,6 +624,40 @@ extension LLBuildManifestBuilder {
             outputs: [.file(target.wrappedModuleOutputPath)],
             args: moduleWrapArgs)
     }
+
+    /// Add a command to produce a new .o file that removes (or hides) the `_main` symbol from a compiled .o file.
+    /// This is used to modify .o files produced by executables so they can be linked into unit test products.  The
+    /// symbol list file is expected to only contain the symbol `_main` and is only needed because `nmedit` needs a
+    /// file with the names of the symbols.
+    private func addMainSymbolRemovalCmd(toolchain: Toolchain,
+                                         inputFile: AbsolutePath, outputFile: AbsolutePath,
+                                         mainSymbolListFile: AbsolutePath) {
+        let args: [String]
+        #if canImport(Darwin)
+            // On Darwin systems, use `nmedit` to remove the `main` symbol.
+            args = [
+                // FIXME: The toolchain should provide the path of the `nmedit` tool.
+                toolchain.swiftCompiler.parentDirectory.appending(component: "nmedit").pathString,
+                "-R", mainSymbolListFile.pathString,
+                inputFile.pathString,
+                "-o", outputFile.pathString
+            ]
+        #else
+            // On non-Darwin systems, use `objcopy` from `binutils` to mark the `main` symbol as local.
+            args = [
+                "objcopy",
+                "-L", "main",
+                inputFile.pathString,
+                outputFile.pathString
+            ]
+        #endif
+        manifest.addShellCmd(
+            name: outputFile.pathString,
+            description: "Eliding symbols from \(outputFile.basename)",
+            inputs: [.file(inputFile)],  // Note: we don't add the symbol file as an input since it's a constant
+            outputs: [.file(outputFile)],
+            args: args)
+    }
 }
 
 // MARK:- Compile C-family
@@ -793,6 +827,11 @@ extension LLBuildManifestBuilder {
                 outputs: [.file(buildProduct.binary)],
                 args: try buildProduct.linkArguments()
             )
+            
+            // Add a separate command to remove the main symbol.
+            for (mainlessObject, object) in buildProduct.executableObjects {
+                addMainSymbolRemovalCmd(toolchain: buildProduct.buildParameters.toolchain, inputFile: object, outputFile: mainlessObject, mainSymbolListFile: buildProduct.mainSymbolRemovalListFilePath)
+            }
         }
 
         // Create a phony node to represent the entire target.
diff --git a/Tests/BuildTests/BuildPlanTests.swift b/Tests/BuildTests/BuildPlanTests.swift
@@ -143,7 +143,7 @@ final class BuildPlanTests: XCTestCase {
             "@/path/to/build/debug/exe.product/Objects.LinkFileList",
             "-Xlinker", "-rpath", "-Xlinker", "/fake/path/lib/swift/macosx",
             "-target", "x86_64-apple-macosx10.10", "-Xlinker", "-add_ast_path",
-            "-Xlinker", "/path/to/build/debug/exe.build/exe.swiftmodule", "-Xlinker", "-add_ast_path",
+            "-Xlinker", "/path/to/build/debug/exe.swiftmodule", "-Xlinker", "-add_ast_path",
             "-Xlinker", "/path/to/build/debug/lib.swiftmodule",
         ]
       #else
@@ -784,7 +784,7 @@ final class BuildPlanTests: XCTestCase {
             "@/path/to/build/debug/exe.product/Objects.LinkFileList",
             "-Xlinker", "-rpath", "-Xlinker", "/fake/path/lib/swift/macosx",
             "-target", "x86_64-apple-macosx10.10",
-            "-Xlinker", "-add_ast_path", "-Xlinker", "/path/to/build/debug/exe.build/exe.swiftmodule",
+            "-Xlinker", "-add_ast_path", "-Xlinker", "/path/to/build/debug/exe.swiftmodule",
         ])
       #else
         XCTAssertEqual(try result.buildProduct(for: "exe").linkArguments(), [
@@ -994,7 +994,7 @@ final class BuildPlanTests: XCTestCase {
             "@/path/to/build/debug/exe.product/Objects.LinkFileList",
             "-Xlinker", "-rpath", "-Xlinker", "/fake/path/lib/swift/macosx",
             "-target", "x86_64-apple-macosx10.10",
-            "-Xlinker", "-add_ast_path", "-Xlinker", "/path/to/build/debug/exe.build/exe.swiftmodule",
+            "-Xlinker", "-add_ast_path", "-Xlinker", "/path/to/build/debug/exe.swiftmodule",
         ])
       #else
         XCTAssertEqual(try result.buildProduct(for: "exe").linkArguments(), [
@@ -1092,7 +1092,7 @@ final class BuildPlanTests: XCTestCase {
             "@/path/to/build/debug/Foo.product/Objects.LinkFileList",
             "-Xlinker", "-rpath", "-Xlinker", "/fake/path/lib/swift/macosx",
             "-target", "x86_64-apple-macosx10.10",
-            "-Xlinker", "-add_ast_path", "-Xlinker", "/path/to/build/debug/Foo.build/Foo.swiftmodule"
+            "-Xlinker", "-add_ast_path", "-Xlinker", "/path/to/build/debug/Foo.swiftmodule"
         ])
 
         XCTAssertEqual(barLinkArgs, [
@@ -2339,10 +2339,10 @@ final class BuildPlanTests: XCTestCase {
             XCTAssertMatch(contents, .contains("""
                   "/path/to/build/debug/exe.build/exe.swiftmodule.o":
                     tool: shell
-                    inputs: ["/path/to/build/debug/exe.build/exe.swiftmodule"]
+                    inputs: ["/path/to/build/debug/exe.swiftmodule"]
                     outputs: ["/path/to/build/debug/exe.build/exe.swiftmodule.o"]
                     description: "Wrapping AST for exe for debugging"
-                    args: ["/fake/path/to/swiftc","-modulewrap","/path/to/build/debug/exe.build/exe.swiftmodule","-o","/path/to/build/debug/exe.build/exe.swiftmodule.o","-target","x86_64-unknown-linux-gnu"]
+                    args: ["/fake/path/to/swiftc","-modulewrap","/path/to/build/debug/exe.swiftmodule","-o","/path/to/build/debug/exe.build/exe.swiftmodule.o","-target","x86_64-unknown-linux-gnu"]
                 """))
             XCTAssertMatch(contents, .contains("""
                   "/path/to/build/debug/lib.build/lib.swiftmodule.o":
diff --git a/Tests/FunctionalTests/MiscellaneousTests.swift b/Tests/FunctionalTests/MiscellaneousTests.swift
diff --git a/Tests/WorkspaceTests/InitTests.swift b/Tests/WorkspaceTests/InitTests.swift
