-
Notifications
You must be signed in to change notification settings - Fork 105
Add initial support for object library product type #631
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
base: main
Are you sure you want to change the base?
Conversation
@swift-ci test |
b70a2c0
to
bcd4b93
Compare
@swift-ci test |
@@ -1500,6 +1506,8 @@ fileprivate extension LinkerSpec.LibrarySpecifier { | |||
case (.object, _): | |||
// Object files are added to linker inputs in the sources task producer. | |||
return [] | |||
case (.objectLibrary, _): | |||
return ["@\(path.join("args.resp").str)"] |
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.
Not all tools on all platforms support response files; we already have CLANG_USE_RESPONSE_FILE / LIBTOOL_USE_RESPONSE_FILE for that reason. I think we need some checks here?
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.
hmm, this is going to be annoying because determining the full list of object files a dependency target will produce from the dependent is tricky
@@ -1601,6 +1609,10 @@ public final class LibtoolLinkerSpec : GenericLinkerSpec, SpecIdentifierType, @u | |||
// Object files are added to linker inputs in the sources task producer and so end up in the link-file-list. | |||
return [] | |||
|
|||
case .objectLibrary: | |||
inputPaths.append(specifier.path) | |||
return ["@\(specifier.path.join("args.resp").str)"] |
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.
This should respect LIBTOOL_USE_RESPONSE_FILE (see above)
} | ||
|
||
// Return the default linker. | ||
identifier = isStaticLib ? LibtoolLinkerSpec.identifier : LdLinkerSpec.identifier | ||
let identifier: String | ||
if isObjectLibrary { |
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.
Use a switch (isObjectLibrary, isStaticLib)
to explicitly handle all cases? It's a bit ambiguous what happens if both are true; actually I think that should probably an error because a while ago we switched to MACH_O_TYPE consistently being the source of truth (since people set it, so there are dylib product types with MACH_O_TYPE set to staticlib, and static lib product types with it set to mh_dylib).
That said, should we also consider driving this entirely off a new MACH_O_TYPE value like "objectlib" for consistency and code simplicity? It's not like "staticlib" is an actual Mach-O object type anyways, so it's already overloaded and this doesn't correspond 1:1 with the Mach-O file format header's file type field (and is has way more values anyways).
return "assemble-object-library" | ||
} | ||
|
||
private struct Options: ParsableArguments { |
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.
Been waiting forever to be able to use this in task action implementations ❤️
@@ -1028,6 +1031,8 @@ package final class TestStandardTarget: TestInternalTarget, Sendable { | |||
return "lib\(name).a" | |||
case .objectFile: | |||
return "\(name).o" | |||
case .objectLibrary: | |||
return "\(name).objlib" |
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.
Is .objlib what CMake uses? Doesn't particularly matter, just wondering if there's any prior art at all
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.
No precedent, I just made this up
These are similar to the CMake concept of object libraries. They're a directory with a .objlib extension containing object files for the target. They also contain an args.resp file with the linker arguments required by consumers. This adds initial support for creating and consuming object libraries. The intent is to use these to move towards properly support the Windows linkage model in SwiftPM