Updates to get executables building and running on windows #9130
Conversation
| case .staticLibrary, .executable: | ||
| pifProductName = "\(sourceModule.name).o" | ||
| executableName = pifProductName | ||
| #if os(Windows) |
There was a problem hiding this comment.
We'll need to vary this based on the platform being built for rather than the host platform so we don't break cross-compilation. There's not a great way to conditionalize productType based on platform in the PIF today, so this is where having a new "swiftPMTarget" product type to abstract away the platform specific differences might be useful
There was a problem hiding this comment.
as discussed offline, this is only temporary (I can add a comment here indicating such) but I think if we can get some basic windows functionally working like building and running an executable which this change could get us there, with tests that now pass, we can then make the incremental changes like using a new product type and moving this kind of platform conditional into swift-build for that product type, and then have those tests ensure we don't regress with those change.
There was a problem hiding this comment.
How long is temporary - something on the order of a few weeks?
There was a problem hiding this comment.
I hope so, will be working on the new product type next.
| 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.
Why did this block need to be dropped?
There was a problem hiding this comment.
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.
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
| delegate: PackagePIFBuilder.BuildDelegate, | ||
| ) { | ||
| self[.TARGET_NAME] = targetName | ||
| self[.PRODUCT_NAME] = createDylibForDynamicProducts ? productName : executableName |
There was a problem hiding this comment.
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.
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...
daveinglis
force-pushed
the
pif_updates
branch
from
3d9e171 to
73a4710
Compare
|
@swift-ci please test |
daveinglis
force-pushed
the
pif_updates
branch
from
73a4710 to
6c94016
Compare
|
@swift-ci please test |
|
@swift-ci please test |
daveinglis
requested review from
bkhouri,
bripeticca,
cmcgee1024,
daveyc123,
dschaefer2,
plemarquand and
shawnhyam
as code owners
daveinglis
force-pushed
the
pif_updates
branch
from
6c94016 to
4aa3868
Compare
|
@swift-ci test |
daveinglis
force-pushed
the
pif_updates
branch
from
4aa3868 to
2edf621
Compare
|
@swift-ci test |
- the also changes the source modules on Windows to be a static lib since pre-linked .o is not supported.
daveinglis
force-pushed
the
pif_updates
branch
from
2edf621 to
1e660e0
Compare
|
@swift-ci test |
|
@swift-ci test windows |
3 participants
depends on: swiftlang/swift-build#795