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

Consistent Library Names #405

Closed
UebelAndre opened this issue Sep 15, 2020 · 3 comments
Closed

Consistent Library Names #405

UebelAndre opened this issue Sep 15, 2020 · 3 comments

Comments

@UebelAndre
Copy link
Collaborator

I've come across this enough and feel like I should post an issue here.

Can someone provide some justification for why libraries produced by rust_library include a hash in them (see @io_bazel_rules_rust//rust/private:rust.bzl). My expectation is that the name would simply be the name of the target plus the appropriate extension.

// Current
"libmy_target--451414955.so"

vs

// Expectation
"libmy_target.so"

I can appreciate the hash but I feel like that should instead be a symlink target or maybe a symlink should be created that uses the simple name. Maybe the hash can just be some metadata file generated with the library. Either way, I find this to be more cumbersome than helpful.

Interested in hearing other peoples thoughts though. Again, I'd love to hear why this is the way that it is.

@acmcarther
Copy link
Collaborator

acmcarther commented Sep 15, 2020

Yo, undead maintainer here. Thanks for your comments and PRs. This change dates back to this PR.
#47

I can't remember the exact problem this solves, but it's something like "rustc can't handle libraries with the exact same filename as build inputs". Before that PR, nobody knew it was an issue because nobody could compile crates from the cargo ecosystem. In the early version of raze, I found this issue, and implemented this hack.

This comment is probably the most specific framing of it
#47 (comment)

EDIT: Oh, one more thing.

This isn't usually a problem, except when you bring in cargo crates, because cargo makes the design choice of permitting multiple versions of the same crate in the build tree. This means that it's not uncommon for some root level crate to have multiple versions of some libCRATE_NAME.so in the dependency list.


Side thing. In this case, I knew I was probably the only person that could answer this, but didn't know exactly what the answer was. Here's how I found the answer from my past self:

  1. From your link I found the "determine_output_hash", which I knew was the most relevant snippet. I hit the blame button and saw Marco's PR here: Separate api / impls, split out rustdoc.bzl #123 with this commit: d33bf1d
  2. I knew from my vague recollection that this wasn't the initial PR (you'd have to determine this yourself in a similar situation), so I went to the commit, clicked the "parent", which is this commit: eb46819
  3. From the parent, I browsed files
  4. Then I clicked around the files (any search would work better) and found "determine_output_hash" in the ancient rust.bzl here:
    def _determine_output_hash(lib_rs):
  5. Hit blame again
  6. Landed on the commit, here f3a46b1 with the PR here Supply crate metadata to rustc and uniqueify --extern'ed crates #47

In my undead maintainer state, I'm not really against changing this, but it probably can't just result in the libCRATE_NAME.so or we'll have the problem that that commit was solving I think
.

@ddeville
Copy link
Contributor

ddeville commented Dec 14, 2021

I've been running into a few issues related to this and I thought I'd mention them here. In short, for top-level crates (that is for things like cdylib and bin crates) I think the renaming is a hassle and I don't think it's necessary in order to prevent the bug that was mentioned above (since these targets will not be dependencies into a rust library and if they were I imagine it would either be as a CcInfo (for cdylib) or as a bazel dep rather than something that rustc deals with (for build scripts for example)).

Aside from the obvious annoyance that one needs a genrule/copy_file on top of every rust cdylib/bin that needs to be bundled into an application, it is actually a real problem for Windows DLLs. Building a DLL also generates a .lib file that tells the linker how to link said DLL and refers to it by its name, that is its name from when it was built. Having to change this name requires generating a new LIB file (which is doable but that I wouldn't call it easy or fun), potentially PDB, etc...
Same thing for debug files. If my overall build system needs to go find a PDB file to upload to some storage after the build has completed, it now needs to go fishing for this file in the out directory since we don't know what the hash that gets appended to it will be ahead of time.

I understand the rationale for adding this hash in the first place but I wonder if others believe that it is something that we might be able to relax for cdylib and bin crates.

Edit: actually just cdylib since this doesn't apply to bin crates I'm pretty sure.

@UebelAndre
Copy link
Collaborator Author

I think #1066 satisfies the request here since I opened it while trying to load cdylib files. I'm happy to reopen this if someone wants to mention another use case for this 😄

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

No branches or pull requests

3 participants