Enable armv7em and os none triples #6438
Conversation
rauhul
commented
Apr 14, 2023
rauhul
commented
- Adds preliminary knowledge of armv7em and no os environments to spm. Future diffs will be required to fully support these triples. This change allows a armv7em-apple-none-macho build via a destination v3 bundle to make it further through spm.
rauhul
requested review from
abertelrud,
neonichu,
tomerd and
MaxDesiatov
as code owners
rauhul
force-pushed
the
teach-spm-about-armv7em-and-os-none
branch
from
36b731e to
cf60251
Compare
rauhul
changed the title
|
@swift-ci please test |
|
@swift-ci please smoke test |
|
@swift-ci test windows |
Sources/Basics/Triple.swift
Outdated
| // Better as dynamic error or "fatalError", but it is a larger | ||
| // undertaking to teach spm that dylibs are not available on some | ||
| // platforms. | ||
| return ".os-none-dynamic-library" |
There was a problem hiding this comment.
I personally would prefer fatalError here
There was a problem hiding this comment.
IIRC, we had problems with that in the WASI case? I don't remember if they were specific to that or a workaround for dylib extensions to be eagerly asked for by SwiftPM.
There was a problem hiding this comment.
WASI just has static stdlib linking and everything else hardcoded to be enabled by default AFAIR
There was a problem hiding this comment.
Changed to fatal error, but I had to make the use of this string lazy to avoid crashing in normal use (for triple armv7em-apple-none-macho)
MaxDesiatov
changed the title
|
cc @compnerd who also previously worked on enabling ARM subarchitectures in a different PR |
| @@ -272,6 +280,8 @@ extension Triple { | |||
| return ".wasm" | |||
| case .windows: | |||
| return ".exe" | |||
| case .noneOS: | |||
| return "" | |||
There was a problem hiding this comment.
An argument could be made that it should be ".exe" or "". Embedded systems are really heterogenous and some recommend the .exe and some don't. I'm worried that loaders might care about the extension and we don't have a way to specify that in the Package Manifest.
There was a problem hiding this comment.
I could conditionalize this based on the vendor I guess? But for now I feel like "" is probably sufficient?
|
@swift-ci please smoke test |
- Adds preliminary knowledge of armv7em and no os environments to spm. Future diffs will be required to fully support these triples. This change allows a armv7em-apple-none-macho build via a destination v3 bundle to make it further through spm.
rauhul
force-pushed
the
teach-spm-about-armv7em-and-os-none
branch
from
444a1a0 to
3491aeb
Compare
|
@swift-ci please smoke test |
1 similar comment
|
@swift-ci please smoke test |
| @@ -391,18 +391,22 @@ public struct BuildParameters: Encodable { | |||
| return try buildPath.appending(binaryRelativePath(for: product)) | |||
| } | |||
|
|
|||
| /// Returns the path to the dynamic library of a product for the current build parameters. | |||
| internal func potentialDynamicLibraryPath(for product: ResolvedProduct) throws -> RelativePath { | |||
There was a problem hiding this comment.
nit: I don't think we use explicit internal in our codebase
| internal func potentialDynamicLibraryPath(for product: ResolvedProduct) throws -> RelativePath { | |
| func potentialDynamicLibraryPath(for product: ResolvedProduct) throws -> RelativePath { |
|
@swift-ci please smoke test |
|
@swift-ci test windows |
Adds preliminary knowledge of `armv7em` CPU and `none` OS environments to SwiftPM. Future diffs will be required to fully support these triples. This change allows a `armv7em-apple-none-macho` build via a destination v3 bundle to make it further through SwiftPM. (cherry picked from commit da7d339) # Conflicts: # Sources/SPMBuildCore/BuildParameters.swift
