Skip to content

[Windows] Don't prepend lib prefix for static library #31338

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

Conversation

kateinoigakukun
Copy link
Member

Currently swiftc emits static archives with name prefixed with "lib" for Windows target.
But when clang linker received -lA, clang doesn't prepend "lib" prefix for actual library filename. https://github.com/llvm/llvm-project/blob/ff5264f0c6f026b25e2d91d0f5d5377a156c1f40/clang/lib/Driver/ToolChains/MSVC.cpp#L474-L481

So this PR removed the "lib" prefix from static archives on Windows.

ref: #31146 (comment)

Resolves SR-NNNN.

@theblixguy theblixguy requested a review from compnerd April 27, 2020 15:50
@compnerd
Copy link
Member

compnerd commented Apr 27, 2020

I'm not sure I understand why you are referencing the code in clang. The rendering of the command says nothing to the behaviour of link. It does not consider the fact that the rendered argument is just an argument to link and that the behaviour of link may be different ;-). I hadn't had my coffee yet in my earlier response and mixed up things in writing.

Static libraries should have the lib prefix.
Import libraries should not have the lib prefix.
Runtime components (DLLs) do not have the lib prefix.

libucrt.lib - static ucrt
ucrt.lib - import library for ucrt
ucrtbase.dll - runtime component for ucrt

@kateinoigakukun
Copy link
Member Author

@compnerd I understood the naming convention for those kinds of libraries.

My current understanding is "link" just accepts input files without adding prefix out suffix.
So I referred clang code to know how to name libraries because swift driver depends on clang to invoke linkers.

Then, should the driver add the "lib" prefix to libraries specified with "-l" before passing them to clang linker?

or the users should specify the static library with "lib" prefix like "-llibucrt"?


But as far as I investigated, It seems that adding the "lib" prefix to static library is not a common convention.
For example, Visual Studio's "Static Library" template doesn't add the "lib" prefix to file name.

I'm really inexperienced in Windows, so I may overlook something.

@compnerd
Copy link
Member

If you are passing the options with -l then you do not add the lib prefix. That is, libucrt.lib would be -lucrt. Alternatively, you can pass the absolute path to the .lib as .../libucrt.lib. Adding lib to the static library is a common convention, you may be confusing import libraries and static libraries I suspect.

@kateinoigakukun
Copy link
Member Author

kateinoigakukun commented May 4, 2020

@compnerd

Adding lib to the static library is a common convention

I understood that convention.

But I'm thinking of how to implement that.

"link" accepts actual library filenames like libStaticLib.lib.

So when driver accepts -lStaticLib option, driver needs to normalize the name to actual filename libStaticLib.lib

Static library and import library have different naming convention, so the driver have to distinguish them to determine the file name should have "lib" prefix or not.

However, the -l option format can't distinguish between import and static libraries.

So do we need two new options, one accepts only import library and another accepts only static library?

What do you think about this?

@compnerd
Copy link
Member

compnerd commented May 4, 2020

The traditional way on Windows is that static libraries just get absolute paths (since static libraries should not be redistributed - they are a problem for ABI compatibility). Why doesn’t that work? All the linkers are happy enough with that.

@kateinoigakukun
Copy link
Member Author

The traditional way on Windows is that static libraries just get absolute paths (since static libraries should not be redistributed - they are a problem for ABI compatibility).

Ahh, I see! I didn't know that traditional way. OK, this is not a valuable change. Sorry for taking up your time 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants