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

Add "crate-name={}" tag to Crate Universe targets #1787

Merged
merged 3 commits into from
Jan 19, 2023

Conversation

alexjpwalker
Copy link
Contributor

When interoperating with Cargo, it's useful to have knowledge of the original crate name for a crate downloaded via Crate Universe (e.g: for distributing a crate compiled via Bazel on crates.io).

Unfortunately this name is not currently exposed anywhere, besides (incidentally) via the auto-generated repository name: @crates__async-trait-0.36.0 (for the target @crates__async-trait-0.36.0//:async_trait) and via the alias, @crates//:async-trait, which (to the best of my knowledge) is not generally visible to a Bazel aspect, as the target field in an aspect implementation refers to the original target being aliased - in this case @crates__async-trait-0.36.0//:async_trait

As a workaround it is possible to retrieve the crate name through string parsing, but this feels like a hack. In this PR we add a tag to each Crate Universe target: for example "crate-name=async-trait".

Fixes #1783 .

@alexjpwalker alexjpwalker marked this pull request as ready for review January 18, 2023 20:51
@alexjpwalker
Copy link
Contributor Author

alexjpwalker commented Jan 18, 2023

NB I haven't found a way to test that this fixes the issue; the repo appears to be unusable (ie: when importing the source code artifact via http_archive) without going through the release process in GH Actions, so for now the code is more-or-less a best guess.

@illicitonion
Copy link
Collaborator

Thanks for the PR!

the repo appears to be unusable (ie: when importing the source code artifact via http_archive) without going through the release process in GH Actions

See https://github.com/bazelbuild/rules_rust/blob/main/crate_universe/DEVELOPMENT.md for development tips :)


We should also add this tag to generated cargo_build_script, rust_proc_macro and rust_binary targets - could you make that be the case, and add a test in rendering.rs?

@@ -766,6 +768,7 @@ mod test {

assert!(build_file_content.contains("rust_library("));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validate crate-name tag correctness for:

  1. rust_library
  2. cargo_build_script
  3. rust_proc_macro
  4. rust_binary

@@ -379,6 +379,7 @@ impl Renderer {
tags.insert("manual".to_owned());
tags.insert("noclippy".to_owned());
tags.insert("norustfmt".to_owned());
tags.insert(format!("crate-name={}", krate.name));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tag to cargo_build_scripts

@@ -514,6 +515,7 @@ impl Renderer {
tags.insert("manual".to_owned());
tags.insert("noclippy".to_owned());
tags.insert("norustfmt".to_owned());
tags.insert(format!("crate-name={}", krate.name));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tag to rust_binary, rust_library, rust_proc_macro

@alexjpwalker
Copy link
Contributor Author

Thanks for the PR!

the repo appears to be unusable (ie: when importing the source code artifact via http_archive) without going through the release process in GH Actions

See https://github.com/bazelbuild/rules_rust/blob/main/crate_universe/DEVELOPMENT.md for development tips :)

We should also add this tag to generated cargo_build_script, rust_proc_macro and rust_binary targets - could you make that be the case, and add a test in rendering.rs?

Done, tests all passing locally - thanks for the tip!

Copy link
Collaborator

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - thanks!

If you rebase past #1785 we should be able to merge :)

@alexjpwalker
Copy link
Contributor Author

Merged latest upstream changes.

@illicitonion illicitonion merged commit 91cd399 into bazelbuild:main Jan 19, 2023
@alexjpwalker alexjpwalker deleted the crate-name-tag branch January 19, 2023 16:49
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.

crate_universe: Expose the original crate name via a tag
2 participants