You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
<rdar://63848226> [SR-12758] C wrapper library requires custom module map in Swift 5.2
The problem here is actually that the logic in SwiftPM always creates a module map with either an umbrella header or an umbrella directory for every C target, even though the documentation says that a module map will only be created for a C target under particular conditions. According to https://github.com/apple/swift-package-manager/blob/master/Documentation/Usage.md#creating-c-language-targets, here are the rules:
> Swift Package Manager will automatically generate a modulemap for each
> C language library target for these 3 cases:
>
> • If “include/Foo/Foo.h” exists and “Foo” is the only directory under the
> “include” directory, then “include/Foo/Foo.h” becomes the umbrella header.
>
> • If “include/Foo.h” exists and “include” contains no other subdirectory then
> “include/Foo.h” becomes the umbrella header.
>
> • Otherwise, if the “include” directory only contains header files and no
> other subdirectory, it becomes the umbrella directory.
>
> In case of complicated include layouts, a custom module.modulemap can be
> provided inside “include”. SwiftPM will error out if it can not generate
> a modulemap with respect to the above rules.
But the problem is that the third of those bullet points is not how the logic works. Any header layout that doesn't match one of the first two cases automatically became an umbrella directory in the generated module map.
This bug was masked because module map files weren’t passed from one C target to another until https://bugs.swift.org/browse/SR-10707 was fixed, at which point this started breaking. For C targets whose module maps were passed to Swift targets, it was always broken (but only surfaced as errors from the compiler as a result of trying to build a module from the non-modularized headers, not surfaced as errors from SwiftPM about an incorrect header layout).
We don’t want to undo the fix for SR-10707, nor is it right to tie this behavior to particular Swift tools versions, since that only postpones the problem. It’s perfectly legitimate to want to wrap an unmodified C or C++ library (with non-modularized header layout) in an authored C wrapper target that then presents a Swift-friendly modular interface to Swift targets, so non-modular C-to-C target imports should continue to be supported. The code also alludes to this, at BuildPlan.swift:409 in current main branch:
> // FIXME: We should probably only warn if we're unable to generate the
> // modulemap because the clang target is still a valid, it just can't be
> // imported from Swift targets.
This is above the line that does a `try` that ends up passing on any errors if there’s a problem with the layout.
After having gone through the code, and discussing this in some detail, and trying a couple of different approaches, I think the right thing to do is:
1. Fix the logic to only create an umbrella directory module map in the cases that are spelled out in the third bullet of the documentation (i.e. match the documented behavior).
2. But emit a warning, not an error, suggesting that the target add a custom module map. If we emitted an error as the documentation said, we would break a lot of packages.
3. Update the documentation to account for one other case that’s already being checked in the logic for case 1, which is that there are no headers next to the umbrella directory.
Instead of not generating a module map at all, we generate an empty module map (along with emitting the warning) in the cases in which an incorrect umbrella directory would previously have been generated into the module map. This has the least impact on the downstream logic.
// If 'include/ModuleName.h' exists, then use it as the umbrella header (this is case 2 at https://github.com/apple/swift-package-manager/blob/master/Documentation/Usage.md#creating-c-language-targets).
@@ -117,8 +118,10 @@ public struct ModuleMapGenerator {
117
118
}
118
119
diagnoseInvalidUmbrellaHeader(includeDir)
119
120
121
+
// If 'include/ModuleName/ModuleName.h' exists, then use it as the umbrella header (this is case 1 at Documentation/Usage.md#creating-c-language-targets).
// Otherwise, if 'include' contains only header files and no subdirectories, use it as the umbrella directory (this is case 3 at https://github.com/apple/swift-package-manager/blob/master/Documentation/Usage.md#creating-c-language-targets).
// Otherwise, the target's public headers are considered to be incompatible with modules. Other C targets can still import them, but Swift won't be able to see them. This is documented as an error, but because SwiftPM has previously allowed it (creating module maps that then cause errors when used), we instead emit a warning.
147
+
warningStream <<<"warning: the include directory of target '\(target.name)' has "
148
+
warningStream <<<"a layout that is incompatible with modules; no module map will "
149
+
warningStream <<<"be generated; consider adding a custom module map to the target"
150
+
warningStream.flush()
151
+
trycreateModuleMap(inDir: wd, type:.empty)
138
152
}
139
153
140
154
/// Warn user if in case target name and c99name are different and there is a
@@ -150,6 +164,7 @@ public struct ModuleMapGenerator {
150
164
}
151
165
152
166
privateenumUmbrellaType{
167
+
case empty
153
168
case header(AbsolutePath)
154
169
case directory(AbsolutePath)
155
170
}
@@ -158,6 +173,8 @@ public struct ModuleMapGenerator {
Copy file name to clipboardExpand all lines: Tests/PackageLoadingTests/ModuleMapGenerationTests.swift
+14-8Lines changed: 14 additions & 8 deletions
Original file line number
Diff line number
Diff line change
@@ -1,7 +1,7 @@
1
1
/*
2
2
This source file is part of the Swift.org open source project
3
3
4
-
Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
4
+
Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
5
5
Licensed under Apache License v2.0 with Runtime Library Exception
6
6
7
7
See http://swift.org/LICENSE.txt for license information
@@ -77,13 +77,6 @@ class ModuleMapGeneration: XCTestCase {
77
77
"/Foo.c")
78
78
checkExpected()
79
79
80
-
// FIXME: Should this be allowed?
81
-
fs =InMemoryFileSystem(emptyFiles:
82
-
"/include/Baz/Foo.h",
83
-
"/include/Bar/Bar.h",
84
-
"/Foo.c")
85
-
checkExpected()
86
-
87
80
fs =InMemoryFileSystem(emptyFiles:
88
81
"/include/Baz.h",
89
82
"/include/Bar.h",
@@ -111,6 +104,19 @@ class ModuleMapGeneration: XCTestCase {
111
104
result.check(value: expected.bytes)
112
105
result.checkDiagnostics("warning: /include/F-o-o.h should be renamed to /include/F_o_o.h to be used as an umbrella header")
113
106
}
107
+
108
+
fs =InMemoryFileSystem(emptyFiles:
109
+
"/include/Baz/Foo.h",
110
+
"/include/Bar/Bar.h",
111
+
"/Foo.c")
112
+
letexpected2=BufferedOutputByteStream()
113
+
expected2 <<<"module Foo {\n"
114
+
expected2 <<<" export *\n"
115
+
expected2 <<<"}\n"
116
+
ModuleMapTester("Foo", in: fs){ result in
117
+
result.check(value: expected2.bytes)
118
+
result.checkDiagnostics("warning: the include directory of target \'Foo\' has a layout that is incompatible with modules; no module map will be generated; consider adding a custom module map to the target")
0 commit comments