-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Updates to get executables building and running on windows #9130
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
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -237,31 +237,25 @@ extension PackagePIFProjectBuilder { | |
) throws -> (PackagePIFBuilder.ModuleOrProduct, resourceBundleName: String?) { | ||
precondition(sourceModule.isSourceModule) | ||
|
||
let pifProductName: String | ||
let executableName: String | ||
let productType: ProjectModel.Target.ProductType | ||
|
||
switch desiredModuleType { | ||
case .dynamicLibrary: | ||
// We are re-using this default for dynamic targets as well. | ||
if pifBuilder.createDylibForDynamicProducts { | ||
pifProductName = "lib\(sourceModule.name).dylib" | ||
executableName = pifProductName | ||
productType = .dynamicLibrary | ||
} else { | ||
pifProductName = sourceModule.name + ".framework" | ||
executableName = sourceModule.name | ||
productType = .framework | ||
} | ||
|
||
case .staticLibrary, .executable: | ||
pifProductName = "\(sourceModule.name).o" | ||
executableName = pifProductName | ||
#if os(Windows) // Temporary until we get a new productType in swift-build | ||
productType = .staticArchive | ||
#else | ||
productType = .objectFile | ||
#endif | ||
|
||
case .macro: | ||
pifProductName = sourceModule.name | ||
executableName = pifProductName | ||
productType = .hostBuildTool | ||
} | ||
|
||
|
@@ -280,8 +274,8 @@ extension PackagePIFProjectBuilder { | |
ProjectModel.Target( | ||
id: sourceModule.pifTargetGUID(suffix: targetSuffix), | ||
productType: productType, | ||
name: "\(sourceModule.name)", | ||
productName: pifProductName, | ||
name: sourceModule.name, | ||
productName: "$(EXECUTABLE_NAME)", | ||
approvedByUser: approvedByUser | ||
) | ||
} | ||
|
@@ -407,35 +401,22 @@ extension PackagePIFProjectBuilder { | |
settings.configureDynamicSettings( | ||
productName: sourceModule.name, | ||
targetName: sourceModule.name, | ||
executableName: executableName, | ||
packageIdentity: package.identity, | ||
packageName: sourceModule.packageName, | ||
createDylibForDynamicProducts: pifBuilder.createDylibForDynamicProducts, | ||
installPath: "/usr/local/lib", | ||
delegate: pifBuilder.delegate | ||
delegate: pifBuilder.delegate, | ||
) | ||
} else { | ||
settings[.TARGET_NAME] = sourceModule.name | ||
settings[.PRODUCT_NAME] = "$(TARGET_NAME)" | ||
settings[.PRODUCT_MODULE_NAME] = sourceModule.c99name | ||
settings[.PRODUCT_BUNDLE_IDENTIFIER] = "\(self.package.identity).\(sourceModule.name)" | ||
.spm_mangledToBundleIdentifier() | ||
settings[.EXECUTABLE_NAME] = executableName | ||
settings[.CLANG_ENABLE_MODULES] = "YES" | ||
settings[.GENERATE_PRELINK_OBJECT_FILE] = "NO" | ||
settings[.STRIP_INSTALLED_PRODUCT] = "NO" | ||
|
||
// Macros build as executables, so they need slightly different | ||
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. Why did this block need to be dropped? 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. so correct me if I'm wrong but setting the MACH_O_TYPE should not be required to be set in the PIF as the product type definition in swift-build sets it for us, and with that said i feel CLANG_COVERAGE_MAPPING_LINKER_ARGS should be handled that same way 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. that's a good point, I wonder why this was put here originally... and yeah, lets sink the CLANG_COVERAGE_MAPPING_LINKER_ARGS change into Swift Build |
||
// build settings from other module types which build a "*.o". | ||
if desiredModuleType == .macro { | ||
settings[.MACH_O_TYPE] = "mh_execute" | ||
} else { | ||
settings[.MACH_O_TYPE] = "mh_object" | ||
// Disable code coverage linker flags since we're producing .o files. | ||
// Otherwise, we will run into duplicated symbols when there are more than one targets that produce .o | ||
// as their product. | ||
settings[.CLANG_COVERAGE_MAPPING_LINKER_ARGS] = "NO" | ||
} | ||
settings[.SWIFT_PACKAGE_NAME] = sourceModule.packageName | ||
|
||
if desiredModuleType == .executable { | ||
|
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.
Will removing createDylibForDynamicProducts break the use case of xcodebuild building dylibs? We might be missing some tests there...
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.
ya, I think @owenv was saying offline that there is some history here we just don't know about as to why this was like this. I think we should try and move forward with this as it cleans things up and I imagine we should be able to address any fallout quickly if need be...