Skip to content

Commit 44377cf

Browse files
ncooke3ncooke3
committed
Resolve warning and update tests
- This commit fixed the warning from the previous commit. This resolves a TODO. - Since custom module maps are now supported, the unit tests needed to be updated to reflect the updated implementation.
1 parent 669225c commit 44377cf

File tree

6 files changed

+73
-65
lines changed

6 files changed

+73
-65
lines changed

‎Sources/Build/BuildPlan.swift

Lines changed: 34 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,9 @@ public final class ClangTargetBuildDescription {
356356

357357
// Write the modified contents to a new module map in the
358358
// build directory.
359-
let writePath = tempsPath.appending(component: moduleMapFilename)
359+
let writePath = tempsPath
360+
.appending(component: "Product")
361+
.appending(component: moduleMapFilename)
360362
try fileSystem.createDirectory(writePath.parentDirectory, recursive: true)
361363

362364
// If the file exists with the identical contents, we don't need to rewrite it.
@@ -380,7 +382,9 @@ public final class ClangTargetBuildDescription {
380382
// double as the unextended module map.
381383
try? fileSystem.copy(
382384
from: customModuleMapPath,
383-
to: tempsPath.appending(component: unextendedModuleMapFilename)
385+
to: tempsPath
386+
.appending(component: "Product")
387+
.appending(component: unextendedModuleMapFilename)
384388
)
385389
} else {
386390
// When not building within a mixed target, use the custom
@@ -397,14 +401,13 @@ public final class ClangTargetBuildDescription {
397401
fileSystem: fileSystem
398402
)
399403

400-
let generatedInteropHeaderPath = isWithinMixedTarget
401-
? tempsPath.appending(component: "\(target.c99name)-Swift.h") : nil
402-
403-
let moduleMapPath = tempsPath.appending(component: moduleMapFilename)
404+
let moduleMapPath = tempsPath
405+
.appending(component: "Product")
406+
.appending(component: moduleMapFilename)
404407
try moduleMapGenerator.generateModuleMap(
405408
type: generatedModuleMapType,
406409
at: moduleMapPath,
407-
interopHeaderPath: generatedInteropHeaderPath
410+
addSwiftSubmodule: true
408411
)
409412

410413
if isWithinMixedTarget {
@@ -416,7 +419,9 @@ public final class ClangTargetBuildDescription {
416419
// Swift part without the generated header being considered
417420
// an input (because it won't exist yet and is an output of
418421
// that compilation command).
419-
let unextendedModuleMapPath = tempsPath.appending(component: unextendedModuleMapFilename)
422+
let unextendedModuleMapPath = tempsPath
423+
.appending(component: "Product")
424+
.appending(component: unextendedModuleMapFilename)
420425
try moduleMapGenerator.generateModuleMap(
421426
type: generatedModuleMapType,
422427
at: unextendedModuleMapPath
@@ -1504,6 +1509,8 @@ public final class MixedTargetBuildDescription {
15041509
// with mappings to the target's module map and public headers.
15051510
let publicHeadersPath = clangTargetBuildDescription.clangTarget.includeDir
15061511
let buildArtifactDirectory = swiftTargetBuildDescription.tempsPath
1512+
let buildArtifactProductDirectory = buildArtifactDirectory.appending(component: "Product")
1513+
let generatedInteropHeaderPath = swiftTargetBuildDescription.objCompatibilityHeaderPath
15071514
let allProductHeadersPath = buildArtifactDirectory
15081515
.appending(component: "all-product-headers.yaml")
15091516

@@ -1513,12 +1520,13 @@ public final class MixedTargetBuildDescription {
15131520
.appending(component: "unextended-module-overlay.yaml")
15141521
try VFSOverlay(roots: [
15151522
VFSOverlay.Directory(
1516-
name: buildArtifactDirectory.pathString,
1523+
name: buildArtifactProductDirectory.pathString,
15171524
contents: [
15181525
VFSOverlay.File(
15191526
name: moduleMapFilename,
1520-
externalContents:
1521-
buildArtifactDirectory.appending(component: unextendedModuleMapFilename).pathString
1527+
externalContents: buildArtifactProductDirectory
1528+
.appending(component: unextendedModuleMapFilename)
1529+
.pathString
15221530
)
15231531
]
15241532
)
@@ -1528,7 +1536,7 @@ public final class MixedTargetBuildDescription {
15281536
// name other than `module.modulemap`?
15291537
try VFSOverlay(roots: [
15301538
VFSOverlay.Directory(
1531-
name: buildArtifactDirectory.pathString,
1539+
name: buildArtifactProductDirectory.pathString,
15321540
contents:
15331541
// Public headers
15341542
try VFSOverlay.overlayResources(
@@ -1540,23 +1548,23 @@ public final class MixedTargetBuildDescription {
15401548
// directory is used.
15411549
!$0.pathString.hasSuffix("module.modulemap")
15421550
}
1543-
)
1551+
) + [
1552+
VFSOverlay.File(
1553+
name: generatedInteropHeaderPath.basename,
1554+
externalContents: generatedInteropHeaderPath.pathString
1555+
)
1556+
]
15441557
)
15451558
]).write(to: allProductHeadersPath, fileSystem: fileSystem)
15461559

1547-
// The Objective-C umbrella is virtually placed in the build
1548-
// directory via a VFS overlay. This is done to preserve
1549-
// relative paths in custom module maps. But when resources
1550-
// exist, a warning will appear because the generated resource
1551-
// header (also in the build directory) is not included in the
1552-
// umbrella directory. Passing `-Wno-incomplete-umbrella` seems
1553-
// to prevent it but is not an ideal workaround.
1554-
// TODO(ncooke3): Investigate a way around this.
1555-
1556-
swiftTargetBuildDescription.additionalFlags.append(
1560+
swiftTargetBuildDescription.additionalFlags += [
15571561
// Builds Objective-C portion of module.
1558-
"-import-underlying-module"
1559-
)
1562+
"-import-underlying-module",
1563+
// Add the location of the module's Objective-C module map as
1564+
// a header search path.
1565+
"-I",
1566+
buildArtifactProductDirectory.pathString
1567+
]
15601568

15611569
swiftTargetBuildDescription.appendClangFlags(
15621570
// Pass VFS overlay to the underlying Clang compiler.
@@ -1571,7 +1579,7 @@ public final class MixedTargetBuildDescription {
15711579
// generated interop Swift header.
15721580
clangTargetBuildDescription.additionalFlags += [
15731581
"-I",
1574-
buildArtifactDirectory.pathString,
1582+
buildArtifactProductDirectory.pathString,
15751583
"-ivfsoverlay",
15761584
allProductHeadersPath.pathString
15771585
]

‎Sources/Build/LLBuildManifestBuilder.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -774,7 +774,9 @@ extension LLBuildManifestBuilder {
774774
// input to the Clang compile command, which therefore forces the
775775
// Swift half of the mixed target to be built first.
776776
if target.isWithinMixedTarget {
777-
inputs.append(Node.file(target.tempsPath.appending(component: "\(target.target.name)-Swift.h")))
777+
inputs += [
778+
.file(target.tempsPath.appending(component: "\(target.target.name)-Swift.h"))
779+
]
778780
}
779781

780782
func addStaticTargetInputs(_ target: ResolvedTarget) {

‎Sources/PackageLoading/ModuleMapGenerator.swift

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -181,12 +181,8 @@ public struct ModuleMapGenerator {
181181
}
182182

183183
/// Generates a module map based of the specified type, throwing an error if anything goes wrong. Any
184-
/// diagnostics are added to the receiver's diagnostics engine.
185-
///
186-
/// The `interopHeaderPath` is the path to the generated interop header used to access a
187-
/// module's Swift API in an Objective-C context (`$(ModuleName)-Swift.h`). If non-`nil`, the
188-
/// created module map will include a submodule to access interop header's API.
189-
public func generateModuleMap(type: GeneratedModuleMapType, at path: AbsolutePath, interopHeaderPath: AbsolutePath? = nil) throws {
184+
/// diagnostics are added to the receiver's diagnostics engine..
185+
public func generateModuleMap(type: GeneratedModuleMapType, at path: AbsolutePath, addSwiftSubmodule: Bool = false) throws {
190186
let stream = BufferedOutputByteStream()
191187
stream <<< "module \(moduleName) {\n"
192188
switch type {
@@ -197,9 +193,9 @@ public struct ModuleMapGenerator {
197193
}
198194
stream <<< " export *\n"
199195
stream <<< "}\n"
200-
if let interopHeaderPath = interopHeaderPath {
196+
if addSwiftSubmodule {
201197
stream <<< "module \(moduleName).Swift {\n"
202-
stream <<< " header \"" <<< interopHeaderPath.pathString <<< "\"\n"
198+
stream <<< " header \"\(moduleName)-Swift.h\"\n"
203199
stream <<< " requires objc\n"
204200
stream <<< "}\n"
205201
}

‎Tests/BuildTests/BuildPlanTests.swift

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,7 +1122,7 @@ final class BuildPlanTests: XCTestCase {
11221122
#endif
11231123
XCTAssertEqual(try ext.basicArguments(isCXX: false), args)
11241124
XCTAssertEqual(try ext.objects, [buildPath.appending(components: "extlib.build", "extlib.c.o")])
1125-
XCTAssertEqual(ext.moduleMap, buildPath.appending(components: "extlib.build", "module.modulemap"))
1125+
XCTAssertEqual(ext.moduleMap, buildPath.appending(components: "extlib.build", "Product", "module.modulemap"))
11261126

11271127
let exe = try result.target(for: "exe").clangTarget()
11281128
args = []
@@ -1145,9 +1145,9 @@ final class BuildPlanTests: XCTestCase {
11451145
args += [
11461146
"-I", Pkg.appending(components: "Sources", "exe", "include").pathString,
11471147
"-I", Pkg.appending(components: "Sources", "lib", "include").pathString,
1148-
"-fmodule-map-file=\(buildPath.appending(components: "lib.build", "module.modulemap"))",
1148+
"-fmodule-map-file=\(buildPath.appending(components: "lib.build", "Product", "module.modulemap"))",
11491149
"-I", ExtPkg.appending(components: "Sources", "extlib", "include").pathString,
1150-
"-fmodule-map-file=\(buildPath.appending(components: "extlib.build", "module.modulemap"))",
1150+
"-fmodule-map-file=\(buildPath.appending(components: "extlib.build", "Product", "module.modulemap"))",
11511151
]
11521152
#if !os(Windows) // FIXME(5473) - modules flags on Windows dropped
11531153
args += ["-fmodules-cache-path=\(buildPath.appending(components: "ModuleCache"))"]
@@ -1436,8 +1436,9 @@ final class BuildPlanTests: XCTestCase {
14361436
"-target", "\(defaultTargetTriple)", "-swift-version", "4",
14371437
"-enable-batch-mode", "-Onone", "-enable-testing", "-g", .equal(j),
14381438
"-DSWIFT_PACKAGE", "-DDEBUG", "-Xcc",
1439-
"-fmodule-map-file=/path/to/build/debug/lib.build/module.modulemap",
1440-
"-Xcc", "-I", "-Xcc", "/Pkg/Sources/lib/include",
1439+
"-fmodule-map-file=/path/to/build/debug/lib.build/Product/module.modulemap",
1440+
"-Xcc", "-ivfsoverlay", "-Xcc",
1441+
"/path/to/build/debug/lib.build/all-product-headers.yaml",
14411442
"-module-cache-path",
14421443
"\(buildPath.appending(components: "ModuleCache"))", .anySequence
14431444
])
@@ -1446,8 +1447,9 @@ final class BuildPlanTests: XCTestCase {
14461447
XCTAssertMatch(swiftPartOfLib, [
14471448
"-target", "\(defaultTargetTriple)", "-swift-version", "4",
14481449
"-enable-batch-mode", "-Onone", "-enable-testing", "-g", .equal(j),
1449-
"-DSWIFT_PACKAGE", "-DDEBUG", "-import-underlying-module", "-Xcc",
1450-
"-ivfsoverlay", "-Xcc", "/path/to/build/debug/lib.build/all-product-headers.yaml",
1450+
"-DSWIFT_PACKAGE", "-DDEBUG", "-import-underlying-module", "-I",
1451+
"/path/to/build/debug/lib.build/Product", "-Xcc", "-ivfsoverlay",
1452+
"-Xcc", "/path/to/build/debug/lib.build/all-product-headers.yaml",
14511453
"-Xcc", "-ivfsoverlay", "-Xcc",
14521454
"/path/to/build/debug/lib.build/unextended-module-overlay.yaml",
14531455
"-module-cache-path",
@@ -1460,8 +1462,8 @@ final class BuildPlanTests: XCTestCase {
14601462
XCTAssertMatch(clangPartOfLib, [
14611463
"-fobjc-arc", "-target", "x86_64-apple-macosx10.13", "-g", "-O0",
14621464
"-DSWIFT_PACKAGE=1", "-DDEBUG=1", "-fblocks", "-fmodules",
1463-
"-fmodule-name=lib", "-I", "/Pkg/Sources/lib/include",
1464-
"-I/path/to/build/debug/lib.build",
1465+
"-fmodule-name=lib", "-I", "/path/to/build/debug/lib.build/Product",
1466+
"-ivfsoverlay", "/path/to/build/debug/lib.build/all-product-headers.yaml",
14651467
"-fmodules-cache-path=/path/to/build/debug/ModuleCache"
14661468
])
14671469

@@ -1475,7 +1477,7 @@ final class BuildPlanTests: XCTestCase {
14751477

14761478
XCTAssertEqual(
14771479
try result.target(for: "lib").mixedTarget().moduleMap?.pathString,
1478-
buildPath.appending(components: "lib.build", "module.modulemap").pathString
1480+
buildPath.appending(components: "lib.build", "Product", "module.modulemap").pathString
14791481
)
14801482

14811483
XCTAssertEqual(try result.buildProduct(for: "exe").linkArguments(), [
@@ -1555,10 +1557,10 @@ final class BuildPlanTests: XCTestCase {
15551557
#endif
15561558
XCTAssertEqual(try lib.basicArguments(isCXX: false), args)
15571559
XCTAssertEqual(try lib.objects, [buildPath.appending(components: "lib.build", "lib.c.o")])
1558-
XCTAssertEqual(lib.moduleMap, buildPath.appending(components: "lib.build", "module.modulemap"))
1560+
XCTAssertEqual(lib.moduleMap, buildPath.appending(components: "lib.build", "Product", "module.modulemap"))
15591561

15601562
let exe = try result.target(for: "exe").swiftTarget().compileArguments()
1561-
XCTAssertMatch(exe, [.anySequence, "-swift-version", "4", "-enable-batch-mode", "-Onone", "-enable-testing", "-g", .equal(j), "-DSWIFT_PACKAGE", "-DDEBUG","-Xcc", "-fmodule-map-file=\(buildPath.appending(components: "lib.build", "module.modulemap"))", "-Xcc", "-I", "-Xcc", "\(Pkg.appending(components: "Sources", "lib", "include"))", "-module-cache-path", "\(buildPath.appending(components: "ModuleCache"))", .anySequence])
1563+
XCTAssertMatch(exe, [.anySequence, "-swift-version", "4", "-enable-batch-mode", "-Onone", "-enable-testing", "-g", .equal(j), "-DSWIFT_PACKAGE", "-DDEBUG","-Xcc", "-fmodule-map-file=\(buildPath.appending(components: "lib.build", "Product", "module.modulemap"))", "-Xcc", "-I", "-Xcc", "\(Pkg.appending(components: "Sources", "lib", "include"))", "-module-cache-path", "\(buildPath.appending(components: "ModuleCache"))", .anySequence])
15621564

15631565
#if os(macOS)
15641566
XCTAssertEqual(try result.buildProduct(for: "exe").linkArguments(), [
@@ -1691,7 +1693,7 @@ final class BuildPlanTests: XCTestCase {
16911693

16921694
let buildPath: AbsolutePath = plan.buildParameters.dataPath.appending(components: "debug")
16931695

1694-
XCTAssertEqual(try plan.createREPLArguments().sorted(), ["-I\(Dep.appending(components: "Sources", "CDep", "include"))", "-I\(buildPath)", "-I\(buildPath.appending(components: "lib.build"))", "-L\(buildPath)", "-lpkg__REPL", "repl"])
1696+
XCTAssertEqual(try plan.createREPLArguments().sorted(), ["-I\(Dep.appending(components: "Sources", "CDep", "include"))", "-I\(buildPath)", "-I\(buildPath.appending(components: "lib.build", "Product"))", "-L\(buildPath)", "-lpkg__REPL", "repl"])
16951697

16961698
XCTAssertEqual(plan.graph.allProducts.map({ $0.name }).sorted(), [
16971699
"Dep",
@@ -2519,7 +2521,7 @@ final class BuildPlanTests: XCTestCase {
25192521
XCTAssertEqual(try lib.basicArguments(isCXX: true), expectedLibBasicArgs)
25202522

25212523
XCTAssertEqual(try lib.objects, [buildPath.appending(components: "lib.build", "lib.cpp.o")])
2522-
XCTAssertEqual(lib.moduleMap, buildPath.appending(components: "lib.build", "module.modulemap"))
2524+
XCTAssertEqual(lib.moduleMap, buildPath.appending(components: "lib.build", "Product", "module.modulemap"))
25232525

25242526
#if os(macOS)
25252527
XCTAssertEqual(try result.buildProduct(for: "lib").linkArguments(), [
@@ -2958,10 +2960,10 @@ final class BuildPlanTests: XCTestCase {
29582960
]
29592961
XCTAssertEqual(try lib.basicArguments(isCXX: false), args)
29602962
XCTAssertEqual(try lib.objects, [buildPath.appending(components: "lib.build", "lib.c.o")])
2961-
XCTAssertEqual(lib.moduleMap, buildPath.appending(components: "lib.build", "module.modulemap"))
2963+
XCTAssertEqual(lib.moduleMap, buildPath.appending(components: "lib.build", "Product", "module.modulemap"))
29622964

29632965
let exe = try result.target(for: "exe").swiftTarget().compileArguments()
2964-
XCTAssertMatch(exe, ["-swift-version", "4", "-enable-batch-mode", "-Onone", "-enable-testing", "-g", .equal(j), "-DSWIFT_PACKAGE", "-DDEBUG","-Xcc", "-fmodule-map-file=\(buildPath.appending(components: "lib.build", "module.modulemap"))", "-Xcc", "-I", "-Xcc", "\(Pkg.appending(components: "Sources", "lib", "include"))", "-module-cache-path", "\(buildPath.appending(components: "ModuleCache"))", .anySequence])
2966+
XCTAssertMatch(exe, ["-swift-version", "4", "-enable-batch-mode", "-Onone", "-enable-testing", "-g", .equal(j), "-DSWIFT_PACKAGE", "-DDEBUG","-Xcc", "-fmodule-map-file=\(buildPath.appending(components: "lib.build", "Product", "module.modulemap"))", "-Xcc", "-I", "-Xcc", "\(Pkg.appending(components: "Sources", "lib", "include"))", "-module-cache-path", "\(buildPath.appending(components: "ModuleCache"))", .anySequence])
29652967

29662968
XCTAssertEqual(try result.buildProduct(for: "exe").linkArguments(), [
29672969
result.plan.buildParameters.toolchain.swiftCompilerPath.pathString,
@@ -3027,15 +3029,15 @@ final class BuildPlanTests: XCTestCase {
30273029
]
30283030
XCTAssertEqual(try lib.basicArguments(isCXX: false), args)
30293031
XCTAssertEqual(try lib.objects, [buildPath.appending(components: "lib.build", "lib.c.o")])
3030-
XCTAssertEqual(lib.moduleMap, buildPath.appending(components: "lib.build", "module.modulemap"))
3032+
XCTAssertEqual(lib.moduleMap, buildPath.appending(components: "lib.build", "Product", "module.modulemap"))
30313033

30323034
let exe = try result.target(for: "app").swiftTarget().compileArguments()
30333035
XCTAssertMatch(
30343036
exe,
30353037
[
30363038
"-swift-version", "4", "-enable-batch-mode", "-Onone", "-enable-testing", "-g",
30373039
.equal(j), "-DSWIFT_PACKAGE", "-DDEBUG","-Xcc",
3038-
"-fmodule-map-file=\(buildPath.appending(components: "lib.build", "module.modulemap"))",
3040+
"-fmodule-map-file=\(buildPath.appending(components: "lib.build", "Product", "module.modulemap"))",
30393041
"-Xcc", "-I", "-Xcc", "\(Pkg.appending(components: "Sources", "lib", "include"))",
30403042
"-module-cache-path", "\(buildPath.appending(components: "ModuleCache"))", .anySequence
30413043
]

0 commit comments

Comments
 (0)