Skip to content

Change how libraries are specified to the linker when using searched libs #890

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Change how libraries are specified to the linker when using searched libs #890

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion Sources/SWBCore/SpecImplementations/LinkerSpec.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ open class LinkerSpec : CommandLineToolSpec, @unchecked Sendable {
/// The path to the privacy file, if one exists.
public let privacyFile: Path?

public init(kind: Kind, path: Path, mode: Mode, useSearchPaths: Bool, swiftModulePaths: [String: Path], swiftModuleAdditionalLinkerArgResponseFilePaths: [String: Path], explicitDependencies: [Path] = [], topLevelItemPath: Path? = nil, dsymPath: Path? = nil, xcframeworkSourcePath: Path? = nil, privacyFile: Path? = nil) {
public let libPrefix: String?

public init(kind: Kind, path: Path, mode: Mode, useSearchPaths: Bool, swiftModulePaths: [String: Path], swiftModuleAdditionalLinkerArgResponseFilePaths: [String: Path], prefixes: [String] = [], explicitDependencies: [Path] = [], topLevelItemPath: Path? = nil, dsymPath: Path? = nil, xcframeworkSourcePath: Path? = nil, privacyFile: Path? = nil) {
self.kind = kind
self.path = path
self.mode = mode
Expand All @@ -101,6 +103,7 @@ open class LinkerSpec : CommandLineToolSpec, @unchecked Sendable {
self.dsymPath = dsymPath
self.xcframeworkSourcePath = xcframeworkSourcePath
self.privacyFile = privacyFile
self.libPrefix = prefixes.first
Copy link
Collaborator

@owenv owenv Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we only consider the first element here?

Copy link
Contributor Author

@daveinglis daveinglis Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ya, I was thinking about that and I just couldn't think of a case where there would ever be more that one so I forego the extract logic of handling multiple and just went with the lazy handle one method. Do you feel its worth handling multiple?

Copy link
Collaborator

@owenv owenv Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only handling one sounds ok to me, but I think we should then either only allow one in the spec or error out if we parse more than one

}
}

Expand Down
6 changes: 4 additions & 2 deletions Sources/SWBCore/SpecImplementations/Specs.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,9 @@ public class FileTypeSpec : Spec, SpecType, @unchecked Sendable {
/// Returns `true` if the `isWrapperFolder` value is set in the XCSpec for the file spec.
public let isWrapper: Bool

/// Returns any common prefix this file may have (currently used when specifying searched libraries to linker)
public let prefixes: [String]

required init(_ parser: SpecParser, _ basedOnSpec: Spec?) {
let basedOnFileTypeSpec = basedOnSpec as? FileTypeSpec ?? nil

Expand All @@ -318,8 +321,8 @@ public class FileTypeSpec : Spec, SpecType, @unchecked Sendable {
self.isEmbeddableInProduct = parser.parseBool("IsEmbeddable") ?? false
self.validateOnCopy = parser.parseBool("ValidateOnCopy") ?? false
self.codeSignOnCopy = parser.parseBool("CodeSignOnCopy") ?? false

self.isWrapper = parser.parseBool("IsWrapperFolder") ?? false
self.prefixes = parser.parseStringList("Prefix") ?? []

// Parse and ignore keys we have no use for.
//
Expand Down Expand Up @@ -358,7 +361,6 @@ public class FileTypeSpec : Spec, SpecType, @unchecked Sendable {
parser.parseStringList("MIMETypes")
parser.parseString("Permissions")
parser.parseString("PlistStructureDefinition")
parser.parseStringList("Prefix")
parser.parseBool("RemoveHeadersOnCopy")
parser.parseBool("RequiresHardTabs")
parser.parseString("UTI")
Expand Down
89 changes: 37 additions & 52 deletions Sources/SWBCore/SpecImplementations/Tools/LinkerTools.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1289,41 +1289,15 @@ public final class LdLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @unchec
private static func computeLibraryArgs(_ libraries: [LibrarySpecifier], scope: MacroEvaluationScope) -> (args: [String], inputs: [Path]) {
// Construct the library arguments.
return libraries.compactMap { specifier -> (args: [String], inputs: [Path]) in
let basename = specifier.path.basename

// FIXME: This isn't a good system, we need to redesign how we talk to the linker w.r.t. search paths and our notion of paths.
switch specifier.kind {
case .static:
if specifier.useSearchPaths, basename.hasPrefix("lib"), basename.hasSuffix(".a") {
return (specifier.searchPathFlagsForLd(basename.withoutPrefix("lib").withoutSuffix(".a")), [])
}
return (specifier.absolutePathFlagsForLd(), [specifier.path])
case .dynamic:
let suffix = ".\(scope.evaluate(BuiltinMacros.DYNAMIC_LIBRARY_EXTENSION))"
if specifier.useSearchPaths, basename.hasPrefix("lib"), basename.hasSuffix(suffix) {
return (specifier.searchPathFlagsForLd(basename.withoutPrefix("lib").withoutSuffix(suffix)), [])
}
return (specifier.absolutePathFlagsForLd(), [specifier.path])
case .textBased:
if specifier.useSearchPaths, basename.hasPrefix("lib"), basename.hasSuffix(".tbd") {
// .merge and .reexport are not supported for text-based libraries.
return (specifier.searchPathFlagsForLd(basename.withoutPrefix("lib").withoutSuffix(".tbd")), [])
}
return (specifier.absolutePathFlagsForLd(), [specifier.path])
case .framework:
let frameworkName = Path(basename).withoutSuffix
case .static, .dynamic, .textBased, .framework:
if specifier.useSearchPaths {
return (specifier.searchPathFlagsForLd(frameworkName), [])
}
let absPathArgs = specifier.absolutePathFlagsForLd()
let returnPath: Path
if let pathArg = absPathArgs.last, Path(pathArg).basename == frameworkName {
returnPath = Path(pathArg)
}
else {
returnPath = specifier.path
let args = specifier.searchPathFlagsForLd()
if !args.isEmpty { // no search args, fallback to absolute path to library
return (args, [])
}
}
return (absPathArgs, [returnPath])
return (specifier.absolutePathFlagsForLd(), [specifier.path])
case .object:
// Object files are added to linker inputs in the sources task producer.
return ([], [])
Expand Down Expand Up @@ -1559,35 +1533,44 @@ public final class LdLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @unchec

/// Extensions to `LinkerSpec.LibrarySpecifier` specific to the dynamic linker.
fileprivate extension LinkerSpec.LibrarySpecifier {
func searchPathFlagsForLd(_ name: String) -> [String] {
func searchPathFlagsForLd() -> [String] {
let strippedName: String
Copy link
Collaborator

@owenv owenv Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question here, should we warn if we get a filename format we don't recognize here / don't think will be found?

Copy link
Contributor Author

@daveinglis daveinglis Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, rethinking this.... I think we need to fallback to using absolute path linking when that happens, this would be the same as how it was before....

Copy link
Contributor Author

@daveinglis daveinglis Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are also tests that would have this warning (and fail due to it), but since we will fallback to an absolute path, not sure the warning it necessary now.

Copy link
Collaborator

@owenv owenv Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

falling back w/ no warning makes sense to me

if let prefix = libPrefix, path.basename.hasPrefix(prefix) {
strippedName = Path(path.basename).withoutSuffix.withoutPrefix(prefix)
} else {
if libPrefix != nil { // we need a prefix for linking with search paths
return [] // this will fallback to using absolute paths
}
strippedName = Path(path.basename).withoutSuffix
}
switch (kind, mode) {
case (.dynamic, .normal):
return ["-l" + name]
return ["-l" + strippedName]
case (.dynamic, .reexport):
return ["-Xlinker", "-reexport-l" + name]
return ["-Xlinker", "-reexport-l" + strippedName]
case (.dynamic, .merge):
return ["-Xlinker", "-merge-l" + name]
return ["-Xlinker", "-merge-l" + strippedName]
case (.dynamic, .reexport_merge):
return ["-Xlinker", "-no_merge-l" + name]
return ["-Xlinker", "-no_merge-l" + strippedName]
case (.dynamic, .weak):
return ["-weak-l" + name]
return ["-weak-l" + strippedName]
case (.static, .weak),
(.textBased, .weak):
return ["-weak-l" + name]
return ["-weak-l" + strippedName]
case (.static, _),
(.textBased, _):
// Other modes are not supported for these kinds.
return ["-l" + name]
return ["-l" + strippedName]
case (.framework, .normal):
return ["-framework", name]
return ["-framework", strippedName]
case (.framework, .reexport):
return ["-Xlinker", "-reexport_framework", "-Xlinker", name]
return ["-Xlinker", "-reexport_framework", "-Xlinker", strippedName]
case (.framework, .merge):
return ["-Xlinker", "-merge_framework", "-Xlinker", name]
return ["-Xlinker", "-merge_framework", "-Xlinker", strippedName]
case (.framework, .reexport_merge):
return ["-Xlinker", "-no_merge_framework", "-Xlinker", name]
return ["-Xlinker", "-no_merge_framework", "-Xlinker", strippedName]
case (.framework, .weak):
return ["-weak_framework", name]
return ["-weak_framework", strippedName]
case (.object, _):
// Object files are added to linker inputs in the sources task producer.
return []
Expand Down Expand Up @@ -1724,15 +1707,17 @@ public final class LibtoolLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @u
delegate.warning("Product \(cbc.output.basename) cannot weak-link \(specifier.kind) \(basename)")
}

if specifier.useSearchPaths, basename.hasPrefix("lib"), basename.hasSuffix(".a") {
if specifier.useSearchPaths {
Copy link
Collaborator

@owenv owenv Nov 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we emit some kind of warning here if the library name doesn't have one of the prefixes we think will allow it to be found via search paths?

// Locate using search paths: Add a -l option and *don't* add the path to the library as an input to the task.
return ["-l" + basename.withoutPrefix("lib").withoutSuffix(".a")]
}
else {
// Locate using an absolute path: Add the path as an option and as an input to the task.
inputPaths.append(specifier.path)
return [specifier.path.str]
let basename = specifier.path.basename
let expectedPrefix = specifier.libPrefix ?? "lib"
if basename.hasPrefix(expectedPrefix) {
return ["-l" + Path(basename).withoutSuffix.withoutPrefix(expectedPrefix)]
}
}
// Locate using an absolute path: Add the path as an option and as an input to the task.
inputPaths.append(specifier.path)
return [specifier.path.str]

case .object:
// Object files are added to linker inputs in the sources task producer and so end up in the link-file-list.
Expand Down
10 changes: 10 additions & 0 deletions Sources/SWBGenericUnixPlatform/Specs/Unix.xcspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,14 @@
IconNamePrefix = "TargetPlugin";
DefaultTargetName = "Object File";
},
{
Domain = generic-unix;
Type = FileType;
Identifier = compiled.mach-o.dylib;
BasedOn = compiled.mach-o;
Prefix = (lib);
Extensions = (so);
IsLibrary = YES;
IsDynamicLibrary = YES;
}
)
32 changes: 23 additions & 9 deletions Sources/SWBTaskConstruction/TaskProducers/BuildPhaseTaskProducers/SourcesTaskProducer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
useSearchPaths: useSearchPaths,
swiftModulePaths: swiftModulePaths,
swiftModuleAdditionalLinkerArgResponseFilePaths: swiftModuleAdditionalLinkerArgResponseFilePaths,
prefixes: fileType.prefixes,
privacyFile: privacyFile
)
} else if fileType.conformsTo(context.lookupFileType(identifier: "compiled.mach-o.dylib")!) {
Expand All @@ -517,6 +518,7 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
useSearchPaths: useSearchPaths,
swiftModulePaths: [:],
swiftModuleAdditionalLinkerArgResponseFilePaths: [:],
prefixes: fileType.prefixes,
privacyFile: privacyFile
)
} else if fileType.conformsTo(context.lookupFileType(identifier: "sourcecode.text-based-dylib-definition")!) {
Expand All @@ -527,17 +529,18 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
useSearchPaths: useSearchPaths,
swiftModulePaths: [:],
swiftModuleAdditionalLinkerArgResponseFilePaths: [:],
prefixes: fileType.prefixes,
privacyFile: privacyFile
)
} else if fileType.conformsTo(context.lookupFileType(identifier: "wrapper.framework")!) {
func kindFromSettings(_ settings: Settings) -> LinkerSpec.LibrarySpecifier.Kind? {
func kindFromSettings(_ settings: Settings) -> (kind: LinkerSpec.LibrarySpecifier.Kind, prefixes: [String])? {
switch settings.globalScope.evaluate(BuiltinMacros.MACH_O_TYPE) {
case "staticlib":
return .static
return (.static, context.lookupFileType(identifier: "archive.ar")?.prefixes ?? [])
case "mh_dylib":
return .dynamic
return (.dynamic, context.lookupFileType(identifier: "compiled.mach-o.dylib")?.prefixes ?? [])
case "mh_object":
return .object
return (.object, [])
default:
return nil
}
Expand All @@ -547,9 +550,11 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
let path: Path
let dsymPath: Path?
let topLevelItemPath: Path?
let prefixes: [String]
if let settingsForRef, let presumedKind = kindFromSettings(settingsForRef), !useSearchPaths {
// If we have a Settings from a cross-project reference, use the _actual_ library path. This prevents downstream code from reconstituting the framework path by joining the framework path with the basename of the framework, which won't be correct for deep frameworks which also need the Versions/A path component.
kind = presumedKind
kind = presumedKind.kind
prefixes = presumedKind.prefixes
path = settingsForRef.globalScope.evaluate(BuiltinMacros.TARGET_BUILD_DIR).join(settingsForRef.globalScope.evaluate(BuiltinMacros.EXECUTABLE_PATH)).normalize()
topLevelItemPath = absolutePath
if shouldGenerateDSYM(settingsForRef.globalScope) {
Expand All @@ -563,6 +568,7 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
path = absolutePath
topLevelItemPath = nil
dsymPath = nil
prefixes = []
}

return LinkerSpec.LibrarySpecifier(
Expand All @@ -572,6 +578,7 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
useSearchPaths: useSearchPaths,
swiftModulePaths: [:],
swiftModuleAdditionalLinkerArgResponseFilePaths: [:],
prefixes: prefixes,
topLevelItemPath: topLevelItemPath,
dsymPath: dsymPath,
privacyFile: privacyFile
Expand All @@ -581,7 +588,7 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
kind: .object,
path: absolutePath,
mode: buildFile.shouldLinkWeakly ? .weak : .normal,
useSearchPaths: useSearchPaths,
useSearchPaths: false,
swiftModulePaths: swiftModulePaths,
swiftModuleAdditionalLinkerArgResponseFilePaths: swiftModuleAdditionalLinkerArgResponseFilePaths,
privacyFile: privacyFile
Expand Down Expand Up @@ -621,10 +628,16 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
}

let libraryKind: LinkerSpec.LibrarySpecifier.Kind
let prefixes: [String]
switch library.libraryType {
case .framework: libraryKind = .framework; break
case .dynamicLibrary: libraryKind = .dynamic; break
case .staticLibrary: libraryKind = .static; break
case .framework: libraryKind = .framework; prefixes = []
case .dynamicLibrary:
libraryKind = .dynamic;
prefixes = context.lookupFileType(identifier: "compiled.mach-o.dylib")?.prefixes ?? []
case .staticLibrary:
libraryKind = .static
prefixes = context.lookupFileType(identifier: "archive.ar")?.prefixes ?? []
break
case let .unknown(fileExtension):
// An error of type this type should have already been manifested.
assertionFailure("unknown xcframework type: \(fileExtension)")
Expand All @@ -651,6 +664,7 @@ package final class SourcesTaskProducer: FilesBasedBuildPhaseTaskProducerBase, F
useSearchPaths: useSearchPaths,
swiftModulePaths: [:],
swiftModuleAdditionalLinkerArgResponseFilePaths: [:],
prefixes: prefixes,
explicitDependencies: outputFilePaths,
xcframeworkSourcePath: xcframeworkPath,
privacyFile: nil
Expand Down
7 changes: 5 additions & 2 deletions Sources/SWBTestSupport/RunDestinationTestSupport.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ extension RunDestinationInfo {
/// An `Environment` object with `PATH` or `LD_LIBRARY_PATH` set appropriately pointing into the toolchain to be able to run a built Swift binary in tests.
///
/// - note: On macOS, the OS provided Swift runtime is used, so `DYLD_LIBRARY_PATH` is never set for Mach-O destinations.
package func hostRuntimeEnvironment(_ core: Core, initialEnvironment: Environment = Environment()) -> Environment {
package func hostRuntimeEnvironment(_ core: Core, initialEnvironment: Environment = Environment()) throws -> Environment {
var environment = initialEnvironment
guard let toolchain = core.toolchainRegistry.defaultToolchain else {
return environment
Expand All @@ -329,7 +329,10 @@ extension RunDestinationInfo {
case .elf:
environment.prependPath(key: "LD_LIBRARY_PATH", value: toolchain.path.join("usr/lib/swift/\(platform)").str)
case .pe:
environment.prependPath(key: .path, value: core.developerPath.path.join("Runtimes").join(toolchain.version.description).join("usr/bin").str)
let currentPath = Environment.current[.path] ?? ""
let runtimePath = core.developerPath.path.join("Runtimes").join(toolchain.version.description).join("usr/bin").str
let newPath = currentPath.isEmpty ? "\(runtimePath)" : "\(runtimePath);\(currentPath)"
environment[.path] = newPath
case .macho:
// Fall back to the OS provided Swift runtime
break
Expand Down
1 change: 1 addition & 0 deletions Sources/SWBUniversalPlatform/Specs/ProductTypes.xcspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
FULL_PRODUCT_NAME = "$(EXECUTABLE_NAME)";
MACH_O_TYPE = "mh_dylib";
REZ_EXECUTABLE = YES;
EXECUTABLE_PREFIX = "lib";
EXECUTABLE_SUFFIX = ".$(EXECUTABLE_EXTENSION)";
EXECUTABLE_EXTENSION = "$(DYNAMIC_LIBRARY_EXTENSION:default=dylib)";
PUBLIC_HEADERS_FOLDER_PATH = "/usr/local/include";
Expand Down
2 changes: 2 additions & 0 deletions Sources/SWBUniversalPlatform/Specs/StandardFileTypes.xcspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,7 @@
Class = PBXMachOFileType;
BasedOn = compiled.mach-o;
Extensions = (dylib);
Prefix = (lib);
IsLibrary = YES;
IsDynamicLibrary = YES;
CodeSignOnCopy = YES;
Expand Down Expand Up @@ -939,6 +940,7 @@
Identifier = sourcecode.text-based-dylib-definition;
BasedOn = sourcecode;
Extensions = (tbd);
Prefix = (lib);
IsLibrary = YES;
IsDynamicLibrary = YES;
CodeSignOnCopy = YES;
Expand Down
Loading
Loading