Merge pull request #195 from aciidb0mb3r/patch-23

Improve external dependency discovery logic

Author: aciidgh

Fixtures/DependencyResolution/External/CUsingCDep2/Bar/Package.swift

@@ -0,0 +1,8 @@
+import PackageDescription
+
+let package = Package(
+    name: "Bar",
+    dependencies: [
+        .Package(url: "../Foo", majorVersion: 1)
+	]
+)

Fixtures/DependencyResolution/External/CUsingCDep2/Bar/Sources/SeaLover/Sea.c

@@ -0,0 +1,5 @@
+#include <Foo/Foo.h>
+
+void cool() {
+    foo();
+}

Fixtures/DependencyResolution/External/CUsingCDep2/Bar/Sources/SeaLover/include/Sea.h

@@ -0,0 +1,5 @@
+module SeaLover {
+  header "Sea/Sea.h"
+  link "SeaLover"
+  export *
+}

Fixtures/DependencyResolution/External/CUsingCDep2/Bar/Sources/SeaLover/include/module.modulemap

@@ -0,0 +1,3 @@
+import Foo
+
+foo()

Fixtures/DependencyResolution/External/CUsingCDep2/Bar/Sources/SwiftExec/main.swift

@@ -0,0 +1,3 @@
+void foo() {
+    
+}

Fixtures/DependencyResolution/External/CUsingCDep2/Foo/Package.swift

@@ -0,0 +1 @@
+void foo();

Fixtures/DependencyResolution/External/CUsingCDep2/Foo/Sources/Foo.c

@@ -0,0 +1,5 @@
+module Foo {
+  header "Foo/Foo.h"
+  link "Foo"
+  export *
+}

Fixtures/DependencyResolution/External/CUsingCDep2/Foo/Sources/include/Foo/Foo.h

@@ -15,7 +15,7 @@ import POSIX
 //FIXME: Incremental builds
 
 extension Command {
-    static func compile(clangModule module: ClangModule, configuration conf: Configuration, prefix: String) -> (Command, Command) {
+    static func compile(clangModule module: ClangModule, externalModules: Set<Module>, configuration conf: Configuration, prefix: String) -> (Command, Command) {
 
         let wd = Path.join(prefix, "\(module.c99name).build")
         let mkdir = Command.createDirectory(wd)
@@ -36,14 +36,8 @@ extension Command {
             //transitive closure of the target being built allowing the use of `#include "..."`
             //add `-I` argument to the include directory of every target outside the package in the
             //transitive closure of the target being built allowing the use of `#include <...>`
-            //FIXME: To detect external deps we're checking if their path's parent.parent directory
-            //is `Packages` as external deps will get copied to `Packages` dir. There should be a
-            //better way to do this.
-            if dep.path.parentDirectory.parentDirectory.basename == "Packages" {
-                includeFlag = "-I"
-            } else {
-                includeFlag = "-iquote"
-            }
+
+            includeFlag = externalModules.contains(dep) ? "-I" : "-iquote"
             args += [includeFlag, dep.path]
             args += ["-l\(dep.c99name)"] //FIXME: giving path to other module's -fmodule-map-file is not linking that module
         }

Fixtures/DependencyResolution/External/CUsingCDep2/Foo/Sources/include/module.modulemap

@@ -18,7 +18,7 @@ import Utility
 /**
   - Returns: path to generated YAML for consumption by the llbuild based swift-build-tool
 */
-public func describe(prefix: String, _ conf: Configuration, _ modules: [Module], _ products: [Product], Xcc: [String], Xld: [String], Xswiftc: [String]) throws -> String {
+public func describe(prefix: String, _ conf: Configuration, _ modules: [Module], _ externalModules: Set<Module> , _ products: [Product], Xcc: [String], Xld: [String], Xswiftc: [String]) throws -> String {
 
     guard modules.count > 0 else {
         throw Error.NoModules
@@ -50,7 +50,7 @@ public func describe(prefix: String, _ conf: Configuration, _ modules: [Module],
                 }
             }
 
-            let (compile, mkdir) = Command.compile(clangModule: module, configuration: conf, prefix: prefix)
+            let (compile, mkdir) = Command.compile(clangModule: module, externalModules: externalModules, configuration: conf, prefix: prefix)
             commands.append(compile)
             commands.append(mkdir)
             targets.main.cmds.append(compile)

Sources/Build/Command.compile(ClangModule).swift

@@ -68,7 +68,7 @@ do {
             let dirs = try directories()
             let (rootPackage, externalPackages) = try fetch(dirs.root)
             let (modules, externalModules, products) = try transmute(rootPackage, externalPackages: externalPackages)
-            let yaml = try describe(dirs.build, conf, modules, products, Xcc: opts.Xcc, Xld: opts.Xld, Xswiftc: opts.Xswiftc)
+            let yaml = try describe(dirs.build, conf, modules, Set<Module>(externalModules), products, Xcc: opts.Xcc, Xld: opts.Xld, Xswiftc: opts.Xswiftc)
             try build(YAMLPath: yaml, target: "default")
 
         case .Init(let initMode):

Sources/Build/describe().swift

@@ -14,9 +14,9 @@ import Build
 import POSIX
 
 #if os(Linux)
-public func describe(prefix: String, _ conf: Configuration, _ modules: [Module], _ products: [Product], Xcc: [String], Xld: [String], Xswiftc: [String]) throws -> String {
+public func describe(prefix: String, _ conf: Configuration, _ modules: [Module], _ externalModules: Set<Module>, _ products: [Product], Xcc: [String], Xld: [String], Xswiftc: [String]) throws -> String {
     do {
-        return try Build.describe(prefix, conf, modules, products, Xcc: Xcc, Xld: Xld, Xswiftc: Xswiftc)
+        return try Build.describe(prefix, conf, modules, externalModules, products, Xcc: Xcc, Xld: Xld, Xswiftc: Xswiftc)
     } catch {
 
         // it is a common error on Linux for clang++ to not be installed, but

Sources/swift-build/main.swift

@@ -14,7 +14,7 @@ import XCTest
 final class DescribeTests: XCTestCase {
     func testDescribingNoModulesThrows() {
         do {
-            let _ = try describe("foo", .Debug, [], [], Xcc: [], Xld: [], Xswiftc: [])
+            let _ = try describe("foo", .Debug, [], [], [], Xcc: [], Xld: [], Xswiftc: [])
             XCTFail("This call should throw")
         } catch Build.Error.NoModules {
             XCTAssert(true, "This error should be throw")

Sources/swift-build/xp.swift

@@ -73,6 +73,15 @@ class TestClangModulesTestCase: XCTestCase {
             XCTAssertEqual(output, "hello 5")
         }
     }
+
+    func testCUsingCDep2() {
+        //The C dependency "Foo" has different layout
+        fixture(name: "DependencyResolution/External/CUsingCDep2") { prefix in
+            XCTAssertBuilds(prefix, "Bar")
+            XCTAssertFileExists(prefix, "Bar/.build/debug/libFoo.so")
+            XCTAssertDirectoryExists(prefix, "Bar/Packages/Foo-1.2.3")
+        }
+    }
 }