Skip to content

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented May 14, 2019

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 14, 2019
@alexcrichton
Copy link
Member

Hm ok so on the surface I have extremely little information as to why this is being proposed. The PR title, PR message, and commit message don't give any information and there's simply a comment in the code saying we're shipping something on Windows for rust-ptx-linker. The context I'm missing is why we're doing this change at all, why it's only on Windows, and why it's the best solution to a problem I'd probably have to dig around to find.

Can this be expanded more? This seems like an odd thing we'd do to ship unrelated artifacts that are likely somewhat unstable. We do that currently in *-preview components like llvm-tools-preview, but I'm not sure we'd ship that with the standard components.

@Zoxc
Copy link
Contributor Author

Zoxc commented May 15, 2019

Currently rust-ptx-linker uses LLVM's C API through rustc_codegen_llvm, but #59752 changes rustc_codegen_llvm to no longer export this API.

We do currently ship the full LLVM library on macOS and Linux as part of the rustc component, which rust-ptx-linker can link to instead, but we don't ship anything on Windows.

I also see that we rename the LLVM lib to libLLVM-8-rust-1.35.0-nightly.so, which this PR doesn't do for the DLL.

@alexcrichton
Copy link
Member

That all makes sense but it's still missing why. Why are we shipping this for rust-ptx-linker?

We aren't shipping the full LLVM library for rust-ptx-linker to use on other platforms, we're using it since it's a technical requirement of our distribution artifacts.

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 16, 2019
@nagisa
Copy link
Member

nagisa commented May 16, 2019

This seems like a repeat of #35113 to me. The comments there seem as applicable today as they were in 2016.

@jonas-schievink
Copy link
Contributor

Visiting for triage. Is this something desirable or should this PR be closed?

@Zoxc Zoxc closed this Jun 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants