Skip to content
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

Use the msvc target crates for gnullvm targets #1961

Closed
wants to merge 1 commit into from

Conversation

glandium
Copy link
Contributor

@glandium glandium commented Aug 14, 2022

The import libraries in crates/target/_gnullvm only differ from the
ones in crates/target/
_msvc the following ways:

  • their file name
  • they don't contain the second "/" linker member with symbol names in
    lexical order.
  • the members are not in the same order.

That's about it. Both binutils ld and lld will happily find $lib.lib
import libraries as well as lib$lib.a ones, so the file name doesn't
matter. The other differences also don't have any practical consequence.

This means it is not necessary to add extra crates for the purpose of
the gnullvm target, especially when e.g. cargo vendor will want to
vendor them all, and we can reuse the msvc ones, although their name
could cause some confusion.

Fixes #1964

The import libraries in crates/target/*_gnullvm only differ from the
ones in crates/target/*_msvc the following ways:
- their file name
- they don't contain the second "/" linker member with symbol names in
  lexical order.
- the members are not in the same order.

That's about it. Both binutils ld and lld will happily find $lib.lib
import libraries as well as lib$lib.a ones, so the file name doesn't
matter. The other differences also don't have any practical consequence.

This means it is not necessary to add extra crates for the purpose of
the gnullvm target, especially when e.g. `cargo vendor` will want to
vendor them all, and we can reuse the msvc ones, although their name
could cause some confusion.
@riverar
Copy link
Collaborator

riverar commented Aug 14, 2022

Hey @glandium, we explicitly asked for the separate crate #1846 (review).

  • We don't think making LLVM ingest a non native lib format is a good idea long term
  • Target specific crates lets us leverage optimizations, flags, tricks, etc. that may be specific to that target/toolset

But we're open minded. Is there a problem with what we're doing today that we should consider? Please create an issue to discuss!

See also:

@glandium
Copy link
Contributor Author

I will file an issue shortly for various changes I have pending wrt the import libraries. I wanted to get this one out of the way first, before the extra crates end up published.

@glandium glandium marked this pull request as draft August 17, 2022 08:12
@kennykerr
Copy link
Collaborator

Thanks for the contribution! Sorry I've been gone for a few weeks. I'll close this PR for now but let's continue the discussion #1964 as there appears to be a lot of open questions.

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.

Reducing the size of the import library crates
3 participants