Skip to content

Commit b1d769d

Browse files
committed
Fix warning bug from previous commit description
- This commit fixes the warning described in the previous commit. - Basically, instead of overlaying the generated umbrella header in the public headers directory, it is placed in a new directory within the build directory. The benefit here is that _a mixed target with a custom module map that includes an umbrella header_ will not cause a warning that the umbrella header does not include the generated umbrella header. - Updated the tests the test this behavior. - One weird issue that arose is the umbrella header does not work as expected when imported into a client. Despite being in the HSP and containing absolute paths to each public header, it will throw a weird error like such: ``` error: declaration of 'Driver' must be imported from module 'BasicMixedTarget' before it is required @Property(nullable) Driver *driver; ^ /Users/nickcooke/Developer/swift-package-manager/Fixtures/MixedTargets/BasicMixedTargets/Sources/BasicMixedTarget/include/Driver.h:4:12: note: declaration here is not visible @interface Driver : NSObject ``` - The next step is to figure out if I can exclude the generated umbrella's HSP when compiling individual clang files... - ALTERNATIVE APPROACH: An alternative approach that might be better is to parse the custom module map and add `header $(ModuleName)/$(ModuleName).h`
1 parent 7cdad18 commit b1d769d

File tree

8 files changed

+144
-104
lines changed

8 files changed

+144
-104
lines changed

Fixtures/MixedTargets/BasicMixedTargets/Sources/ClangTargetDependsOnMixedTarget/JunkYard.m

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@
22

33
#import "include/JunkYard.h"
44

5+
// Import the mixed target's module.
56
@import BasicMixedTarget;
67

78
@interface JunkYard ()
89
// The below types come from the `BasicMixedTarget` module.
910
@property(nullable) Engine *engine;
1011
@property(nullable) Driver *driver;
1112
@property(nullable) OldCar *oldCar;
13+
@property(nullable) CarPart *carPart;
1214
@end
1315

1416
@implementation JunkYard
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#import <Foundation/Foundation.h>
2+
3+
@interface RecyclingCenter : NSObject
4+
@end
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#import <Foundation/Foundation.h>
2+
3+
#import "RecyclingCenter.h"
4+
5+
// Import the mixed target's public headers. Both "..." and <...> style imports
6+
// should resolve.
7+
#import "CarPart.h"
8+
#import <CarPart.h>
9+
#import "Driver.h"
10+
#import <Driver.h>
11+
#import "OldCar.h"
12+
#import <OldCar.h>
13+
#import "BasicMixedTarget-Swift.h"
14+
#import <BasicMixedTarget-Swift.h>
15+
16+
@interface RecyclingCenter ()
17+
// The below types come from the `BasicMixedTarget` module.
18+
@property(nullable) Engine *engine;
19+
@property(nullable) Driver *driver;
20+
@property(nullable) OldCar *oldCar;
21+
@property(nullable) CarPart *carPart;
22+
@end
23+
24+
@implementation RecyclingCenter
25+
@end

Fixtures/MixedTargets/BasicMixedTargets/Sources/MixedTargetDependsOnMixedTarget/OldBoat.m

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#import "include/OldBoat.h"
44

5+
// Import the mixed target's module.
56
@import BasicMixedTarget;
67

78
@interface OldBoat ()
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#import <Foundation/Foundation.h>
2+
3+
@interface SpeedBoat : NSObject
4+
@end
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#import <Foundation/Foundation.h>
2+
3+
#import "SpeedBoat.h"
4+
#import <SpeedBoat.h>
5+
6+
// Import the mixed target's public headers. Both "..." and <...> style imports
7+
// should resolve.
8+
#import "CarPart.h"
9+
#import <CarPart.h>
10+
#import "Driver.h"
11+
#import <Driver.h>
12+
#import "OldCar.h"
13+
#import <OldCar.h>
14+
#import "BasicMixedTarget-Swift.h"
15+
#import <BasicMixedTarget-Swift.h>
16+
17+
@interface SpeedBoat ()
18+
19+
// The below types comes from the `BasicMixedTarget` module`.
20+
@property(nonatomic, strong) Engine *engine;
21+
@property(nonatomic, strong) Driver *driver;
22+
@end
23+
24+
@implementation SpeedBoat
25+
@end

Fixtures/MixedTargets/BasicMixedTargets/Tests/BasicMixedTargetTests/ObjcBasicMixedTargetTestsViaBridgingHeader.m

Lines changed: 0 additions & 24 deletions
This file was deleted.

Sources/Build/BuildPlan.swift

Lines changed: 83 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -1336,8 +1336,10 @@ public final class MixedTargetBuildDescription {
13361336

13371337
/// The path to the VFS overlay file that overlays the public headers of
13381338
/// the Clang part of the target over the target's build directory.
1339-
// TODO(ncooke3): Review to see if it should be non-optional.
1340-
let allProductHeadersOverlay: AbsolutePath?
1339+
let allProductHeadersOverlay: AbsolutePath
1340+
1341+
/// The paths to the targets's public headers.
1342+
let publicHeaderPaths: [AbsolutePath]
13411343

13421344
/// The modulemap file for this target.
13431345
let moduleMap: AbsolutePath
@@ -1411,13 +1413,16 @@ public final class MixedTargetBuildDescription {
14111413

14121414
let interopHeaderPath = swiftTargetBuildDescription.objCompatibilityHeaderPath
14131415

1414-
// A mixed target's build directory uses two subdirectories to
1416+
// A mixed target's build directory uses three subdirectories to
14151417
// distinguish between build artifacts:
14161418
// - Intermediates: Stores artifacts used during the target's build.
14171419
// - Product: Stores artifacts used by clients of the target.
1420+
// - InteropSupport: If needed, stores a generated umbrella header
1421+
// for use during the target's build and by clients of the target.
14181422
let tempsPath = buildParameters.buildPath.appending(component: target.c99name + ".build")
14191423
let intermediatesDirectory = tempsPath.appending(component: "Intermediates")
14201424
let productDirectory = tempsPath.appending(component: "Product")
1425+
let interopSupportDirectory: AbsolutePath?
14211426

14221427
// Filenames for VFS overlay files.
14231428
let allProductHeadersFilename = "all-product-headers.yaml"
@@ -1432,33 +1437,35 @@ public final class MixedTargetBuildDescription {
14321437
fileSystem: fileSystem
14331438
)
14341439

1435-
// MARK: Conditionally generate an umbrella header
1436-
1437-
// When the Swift compiler creates the generated interop header for
1438-
// Objective-C compatible Swift API (via `-emit-objc-header`), any
1439-
// Objective-C symbol that cannot be forward declared (e.g. superclass,
1440-
// protocol, etc.) will attempt to be imported via a bridging header.
1441-
// Unfortunately, a custom bridging header can not be specified because
1442-
// the target is evaluated as a framework target (as opposed to an app
1443-
// target), and framework target do not support bridging headers. So,
1444-
// the compiler defaults to importing the following in the generated
1445-
// interop header ($(ModuleName)-Swift.h):
1440+
// MARK: Conditionally generate an umbrella header for interoptability
1441+
1442+
// When the Swift compiler creates the generated interop header
1443+
// (`$(ModuleName)-Swift.h`) for Objective-C compatible Swift API
1444+
// (via `-emit-objc-header`), any Objective-C symbol that cannot be
1445+
// forward declared (e.g. superclass, protocol, etc.) will attempt to
1446+
// be imported via a bridging or umbrella header. Since the compiler
1447+
// evaluates the target as a framework (as opposed to an app), the
1448+
// compiler assumes* an umbrella header exists in a subdirectory (named
1449+
// after the module) within the public headers directory:
14461450
//
14471451
// #import <$(ModuleName)/$(ModuleName).h>
14481452
//
1449-
// The compiler assumes that the above path can be resolved. Instead of
1450-
// forcing package authors to structure their packages around that
1451-
// constraint, the package manager generates an umbrella header if
1452-
// needed and will later use a VFS overlay to make the generated header
1453-
// appear in the proper location so the bridging header import in the
1454-
// generated interop header can be resolved during the build.
1455-
let generatedUmbrellaHeaderPath: AbsolutePath?
1456-
let relativeUmbrellaHeaderPath =
1457-
RelativePath("\(mixedTarget.c99name)/\(mixedTarget.c99name).h")
1458-
let potentialUmbrellaHeaderPath =
1459-
mixedTarget.clangTarget.includeDir.appending(relativeUmbrellaHeaderPath)
1460-
if !fileSystem.isFile(potentialUmbrellaHeaderPath) {
1461-
generatedUmbrellaHeaderPath = tempsPath.appending(component: "\(mixedTarget.c99name).h")
1453+
// The compiler assumes that the above path can be resolved relative to
1454+
// the public header directory. Instead of forcing package authors to
1455+
// structure their packages around that constraint, the package manager
1456+
// generates an umbrella header if needed and will pass it along as a
1457+
// header search path when building the target.
1458+
//
1459+
// *: https://developer.apple.com/documentation/swift/importing-objective-c-into-swift
1460+
let umbrellaHeaderPathComponents = [mixedTarget.c99name, "\(mixedTarget.c99name).h"]
1461+
let potentialUmbrellaHeadersPath = mixedTarget.clangTarget.includeDir
1462+
.appending(components: umbrellaHeaderPathComponents)
1463+
// Check if an umbrella header at
1464+
// `PUBLIC_HDRS_DIR/$(ModuleName)/$(ModuleName).h` already exists.
1465+
if !fileSystem.isFile(potentialUmbrellaHeadersPath) {
1466+
interopSupportDirectory = tempsPath.appending(component: "InteropSupport")
1467+
let generatedUmbrellaHeaderPath = interopSupportDirectory!
1468+
.appending(components: umbrellaHeaderPathComponents)
14621469
// Populate a stream that will become the generated umbrella header.
14631470
let stream = BufferedOutputByteStream()
14641471
mixedTarget.clangTarget.headers
@@ -1480,13 +1487,25 @@ public final class MixedTargetBuildDescription {
14801487
}
14811488

14821489
try fileSystem.writeFileContentsIfNeeded(
1483-
generatedUmbrellaHeaderPath!,
1490+
generatedUmbrellaHeaderPath,
14841491
bytes: stream.bytes
14851492
)
14861493
} else {
1487-
generatedUmbrellaHeaderPath = nil
1494+
// An umbrella header in the desired format already exists so the
1495+
// interop support directory is not needed.
1496+
interopSupportDirectory = nil
14881497
}
14891498

1499+
// TODO(ncooke3): Is there a way to remove the generated umbrella
1500+
// header when compiling individual clang files? This will reduce the
1501+
// misuse of importing the generated umbrella header.
1502+
1503+
// Clients will later depend on the public header directory, and, if an
1504+
// umbrella header was created, the header's root directory.
1505+
self.publicHeaderPaths = interopSupportDirectory != nil ?
1506+
[mixedTarget.clangTarget.includeDir, interopSupportDirectory!] :
1507+
[mixedTarget.clangTarget.includeDir]
1508+
14901509
// MARK: Generate products to be used by client of the target.
14911510

14921511
// Path to the module map used by clients to access the mixed target's
@@ -1537,6 +1556,7 @@ public final class MixedTargetBuildDescription {
15371556
self.moduleMap = customModuleMapPath
15381557
self.allProductHeadersOverlay = productDirectory.appending(component: allProductHeadersFilename)
15391558

1559+
// TODO(ncooke3): Probably can remove the builder pattern now.
15401560
#if swift(>=5.4)
15411561
try VFSOverlay(roots: [
15421562
VFSOverlay.Directory(name: customModuleMapPath.parentDirectory.pathString) {
@@ -1549,23 +1569,8 @@ public final class MixedTargetBuildDescription {
15491569
name: interopHeaderPath.basename,
15501570
externalContents: interopHeaderPath.pathString
15511571
)
1552-
1553-
// TODO(ncooke3): Unfortunately, this can trigger an
1554-
// umbrella header warning that the umbrella header does
1555-
// not include the header in this directory. So this
1556-
// overlay needs to be pulled out of the public header
1557-
// path and placed elsewhere. This also means there will be
1558-
// two public header search paths.
1559-
if let generatedUmbrellaHeaderPath = generatedUmbrellaHeaderPath {
1560-
VFSOverlay.Directory(name: mixedTarget.c99name) {
1561-
VFSOverlay.File(
1562-
name: generatedUmbrellaHeaderPath.basename,
1563-
externalContents: generatedUmbrellaHeaderPath.pathString
1564-
)
1565-
}
1566-
}
15671572
}
1568-
]).write(to: self.allProductHeadersOverlay!, fileSystem: fileSystem)
1573+
]).write(to: self.allProductHeadersOverlay, fileSystem: fileSystem)
15691574
#endif
15701575

15711576
// When the mixed target does not have a custom module map, one will be
@@ -1593,19 +1598,8 @@ public final class MixedTargetBuildDescription {
15931598
name: interopHeaderPath.basename,
15941599
externalContents: interopHeaderPath.pathString
15951600
)
1596-
1597-
// If an umbrella header was generated, it needs to be
1598-
// overlayed within the public headers directory.
1599-
if let generatedUmbrellaHeaderPath = generatedUmbrellaHeaderPath {
1600-
VFSOverlay.Directory(name: mixedTarget.c99name) {
1601-
VFSOverlay.File(
1602-
name: generatedUmbrellaHeaderPath.basename,
1603-
externalContents: generatedUmbrellaHeaderPath.pathString
1604-
)
1605-
}
1606-
}
16071601
}
1608-
]).write(to: self.allProductHeadersOverlay!, fileSystem: fileSystem)
1602+
]).write(to: self.allProductHeadersOverlay, fileSystem: fileSystem)
16091603
#endif
16101604
}
16111605

@@ -1675,15 +1669,6 @@ public final class MixedTargetBuildDescription {
16751669
name: interopHeaderPath.basename,
16761670
externalContents: interopHeaderPath.pathString
16771671
)
1678-
1679-
if let generatedUmbrellaHeaderPath = generatedUmbrellaHeaderPath {
1680-
VFSOverlay.Directory(name: mixedTarget.c99name) {
1681-
VFSOverlay.File(
1682-
name: generatedUmbrellaHeaderPath.basename,
1683-
externalContents: generatedUmbrellaHeaderPath.pathString
1684-
)
1685-
}
1686-
}
16871672
}
16881673
]).write(to: allProductHeadersPath, fileSystem: fileSystem)
16891674
#endif
@@ -1735,6 +1720,18 @@ public final class MixedTargetBuildDescription {
17351720
// generated header can be imported.
17361721
"-I", intermediatesDirectory.pathString
17371722
]
1723+
1724+
// If a generated umbrella header was created, add it's root directory
1725+
// as a header search path. This will resolve its import within the
1726+
// generated interop header.
1727+
if let interopSupportDirectory = interopSupportDirectory {
1728+
swiftTargetBuildDescription.appendClangFlags(
1729+
"-I", interopSupportDirectory.pathString
1730+
)
1731+
clangTargetBuildDescription.additionalFlags += [
1732+
"-I", interopSupportDirectory.pathString
1733+
]
1734+
}
17381735
}
17391736
}
17401737

@@ -2766,20 +2763,22 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
27662763
}
27672764
}
27682765
case let target as MixedTarget where target.type == .library:
2769-
// Add the public headers of the dependency.
2770-
clangTarget.additionalFlags += ["-I", target.clangTarget.includeDir.pathString]
2771-
27722766
// Add the modulemap of the dependency.
27732767
if case let .mixed(dependencyTargetDescription)? = targetMap[dependency] {
2768+
// Add the dependency's modulemap.
27742769
clangTarget.additionalFlags.append(
27752770
"-fmodule-map-file=\(dependencyTargetDescription.moduleMap.pathString)"
27762771
)
27772772

2778-
if let allProductHeadersOverlay = dependencyTargetDescription.allProductHeadersOverlay {
2779-
clangTarget.additionalFlags += [
2780-
"-ivfsoverlay", allProductHeadersOverlay.pathString
2781-
]
2773+
// Add the dependency's public headers.
2774+
dependencyTargetDescription.publicHeaderPaths.forEach {
2775+
clangTarget.additionalFlags += [ "-I", $0.pathString ]
27822776
}
2777+
2778+
// Add the dependency's public VFS overlay.
2779+
clangTarget.additionalFlags += [
2780+
"-ivfsoverlay", dependencyTargetDescription.allProductHeadersOverlay.pathString
2781+
]
27832782
}
27842783
case let target as SystemLibraryTarget:
27852784
clangTarget.additionalFlags += ["-fmodule-map-file=\(target.moduleMapPath.pathString)"]
@@ -2835,18 +2834,22 @@ public class BuildPlan: SPMBuildCore.BuildPlan {
28352834
guard case let .mixed(target)? = targetMap[dependency] else {
28362835
throw InternalError("unexpected mixed target \(underlyingTarget)")
28372836
}
2838-
// Add the dependency's modulemap and public headers.
2837+
2838+
// Add the dependency's modulemap.
28392839
swiftTarget.appendClangFlags(
2840-
"-fmodule-map-file=\(target.moduleMap.pathString)",
2841-
"-I", target.clangTargetBuildDescription.clangTarget.includeDir.pathString
2840+
"-fmodule-map-file=\(target.moduleMap.pathString)"
28422841
)
28432842

2844-
if let allProductHeadersOverlay = target.allProductHeadersOverlay {
2845-
swiftTarget.appendClangFlags(
2846-
"-ivfsoverlay", allProductHeadersOverlay.pathString
2847-
)
2843+
// Add the dependency's public headers.
2844+
target.publicHeaderPaths.forEach {
2845+
swiftTarget.appendClangFlags("-I", $0.pathString)
28482846
}
28492847

2848+
// Add the dependency's public VFS overlay.
2849+
swiftTarget.appendClangFlags(
2850+
"-ivfsoverlay", target.allProductHeadersOverlay.pathString
2851+
)
2852+
28502853
default:
28512854
break
28522855
}

0 commit comments

Comments
 (0)