-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Auto generate module map file for C libs #219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
26b56cf
060204b
7fb0539
8986238
0b8e728
c47c6b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import PackageDescription | ||
|
||
let package = Package( | ||
name: "ModuleMapGenerationCases", | ||
targets: [ | ||
Target(name: "Baz", dependencies: ["FlatInclude", "UmbrellaHeader", "UmbellaModuleNameInclude", "UmbrellaHeaderFlat"])] | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import UmbellaModuleNameInclude | ||
import FlatInclude | ||
import UmbrellaHeader | ||
import UmbrellaHeaderFlat | ||
|
||
let _ = foo() | ||
let _ = bar() | ||
let _ = jaz() | ||
let _ = umbrellaHeaderFlat() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
#include "include/FlatIncludeHeader.h" | ||
|
||
int bar() { | ||
int a = 6; | ||
int b = a; | ||
a = b; | ||
return a; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
int bar(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
|
||
int noDir() { | ||
int a = 6; | ||
int b = a; | ||
a = b; | ||
return a; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
#include "include/UmbellaModuleNameInclude/Paz.h" | ||
|
||
int jaz() { | ||
int a = 6; | ||
int b = a; | ||
a = b; | ||
return a; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
int jaz(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
#include "include/UmbrellaHeader/UmbrellaHeader.h" | ||
|
||
int foo() { | ||
int a = 5; | ||
int b = a; | ||
a = b; | ||
return a; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
int foo(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
#include "include/UmbrellaHeaderFlat.h" | ||
|
||
int umbrellaHeaderFlat() { | ||
int a = 5; | ||
int b = a; | ||
a = b; | ||
return a; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
int umbrellaHeaderFlat(); |
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,9 @@ | |
|
||
import func POSIX.getenv | ||
import func POSIX.popen | ||
import func POSIX.mkdir | ||
import func POSIX.fopen | ||
import func libc.fclose | ||
import PackageType | ||
import Utility | ||
|
||
|
@@ -29,8 +32,104 @@ func platformFrameworksPath() throws -> String { | |
} | ||
|
||
extension CModule { | ||
|
||
var moduleMap: String { | ||
return "module.modulemap" | ||
} | ||
|
||
var moduleMapPath: String { | ||
return Path.join(path, "module.modulemap") | ||
return Path.join(path, moduleMap) | ||
} | ||
} | ||
|
||
extension ClangModule { | ||
|
||
public enum ModuleMapError: ErrorProtocol { | ||
case UnsupportedIncludeLayoutForModule(String) | ||
} | ||
|
||
///FIXME: we recompute the generated modulemap's path | ||
///when building swift modules in `XccFlags(prefix: String)` | ||
///there shouldn't be need to redo this there but is difficult | ||
///in current architecture | ||
public func generateModuleMap(inDir wd: String) throws { | ||
|
||
///Return if module map is already present | ||
guard !moduleMapPath.isFile else { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens when a new header file is added and the modulemap file is checked in? Won't that not generate a new modulemap and thus not make the new header file available? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah there are two problems currently...
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, I did not realize we were not generating them to the build directory. We should be doing that if we are going to generate at all. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will move the generation to build dir |
||
return | ||
} | ||
|
||
let includeDir = path | ||
|
||
///Warn and return if no include directory | ||
guard includeDir.isDirectory else { | ||
print("warning: No include directory found, a library can not be imported without any public headers.") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unrelated, but we should start to get a proper diagnostics infrastructure instead of just embedding print() statements... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. |
||
return | ||
} | ||
|
||
let walked = walk(includeDir, recursively: false).map{$0} | ||
|
||
let files = walked.filter{$0.isFile && $0.hasSuffix(".h")} | ||
let dirs = walked.filter{$0.isDirectory} | ||
|
||
///We generate modulemap for a C module `foo` if: | ||
///* `umbrella header "path/to/include/foo/foo.h"` exists and `foo` is the only | ||
/// directory under include directory | ||
///* `umbrella header "path/to/include/foo.h"` exists and include contains no other | ||
/// directory | ||
///* `umbrella "path/to/include"` in all other cases | ||
|
||
let umbrellaHeaderFlat = Path.join(includeDir, "\(c99name).h") | ||
if umbrellaHeaderFlat.isFile { | ||
guard dirs.isEmpty else { throw ModuleMapError.UnsupportedIncludeLayoutForModule(name) } | ||
try createModuleMap(inDir: wd, type: .Header(umbrellaHeaderFlat)) | ||
return | ||
} | ||
diagnoseInvalidUmbrellaHeader(includeDir) | ||
|
||
let umbrellaHeader = Path.join(includeDir, c99name, "\(c99name).h") | ||
if umbrellaHeader.isFile { | ||
guard dirs.count == 1 && files.isEmpty else { throw ModuleMapError.UnsupportedIncludeLayoutForModule(name) } | ||
try createModuleMap(inDir: wd, type: .Header(umbrellaHeader)) | ||
return | ||
} | ||
diagnoseInvalidUmbrellaHeader(Path.join(includeDir, c99name)) | ||
|
||
try createModuleMap(inDir: wd, type: .Directory(includeDir)) | ||
} | ||
|
||
///warn user if in case module name and c99name are different and there a `name.h` umbrella header | ||
private func diagnoseInvalidUmbrellaHeader(path: String) { | ||
let umbrellaHeader = Path.join(path, "\(c99name).h") | ||
let invalidUmbrellaHeader = Path.join(path, "\(name).h") | ||
if c99name != name && invalidUmbrellaHeader.isFile { | ||
print("warning: \(invalidUmbrellaHeader) should be renamed to \(umbrellaHeader) to be used as an umbrella header") | ||
} | ||
} | ||
|
||
private enum UmbrellaType { | ||
case Header(String) | ||
case Directory(String) | ||
} | ||
|
||
private func createModuleMap(inDir wd: String, type: UmbrellaType) throws { | ||
try POSIX.mkdir(wd) | ||
let moduleMapFile = Path.join(wd, self.moduleMap) | ||
let moduleMap = try fopen(moduleMapFile, mode: .Write) | ||
defer { fclose(moduleMap) } | ||
|
||
var output = "module \(c99name) {\n" | ||
switch type { | ||
case .Header(let header): | ||
output += " umbrella header \"\(header)\"\n" | ||
case .Directory(let path): | ||
output += " umbrella \"\(path)\"\n" | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this would be easier to read if it simply computed the exact line to use instead of appending to a line starting with "umbrella ". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
output += " link \"\(c99name)\"\n" | ||
output += " export *\n" | ||
output += "}\n" | ||
|
||
try fputs(output, moduleMap) | ||
} | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this is obvious, but I am not seeing how this doesn't fail for non-library C language targets?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think it is unfortunate that we are more or less recomputing state here which is independently computed by
generateModuleMap
. I don't think this is fixable in the current SwiftPM architecture, but it would be good to include a comment which points out that the behavior here is tied to that of the other function (and vice versa), so that if someone updates one they know to look at the other.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they wont really be in dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding comment, but will push once CI returns