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

rename_first_party_crates flag #1013

Closed
cfredric opened this issue Nov 12, 2021 · 6 comments
Closed

rename_first_party_crates flag #1013

cfredric opened this issue Nov 12, 2021 · 6 comments

Comments

@cfredric
Copy link
Contributor

This is a tracking issue for the idea described in #1008.

@cfredric
Copy link
Contributor Author

More detail:

Google has some Rust in its source repository, and we have realized that rules_rust's current approach to naming crates (i.e., using the target name as the crate name) will not work for us. We need to use a naming scheme that uniquely identifies each first-party crate, and associates it with the entire Bazel label that it was built from; for third-party crates, we can use the crate's actual name (without modification). The motivations for this are the following:

  • Facilitate automated updates of BUILD files based on .rs source file content, without requiring bazel query.
    • E.g., when the developer adds an import to a Rust source file and saves it, we can run tooling that scans the source, identifies the new dependency, and adds the dep to the correct target in the BUILD file.
  • Avoid the need for the aliases attribute to resolve naming conflicts.
    • E.g., if a crate has dependencies //foo:utils and //bar:utils, this will cause a conflict unless the deps are renamed via aliases in the BUILD file. Use of the aliases attribute requires repetition of each label being renamed, and is in the BUILD file. We expect developers to spend more time reading .rs files than BUILD files, so we'd prefer to move that mapping into the Rust source, and eliminate the need for the developer to manually update both occurrences of the dependency's label (i.e. in the deps and aliases attributes).
  • Facilitate tool-driven large-scale refactorings.
    • I.e., we'd like to be able to update relevant BUILD files without having to scan through all of them.

To achieve those goals, we need the following properties:

  • References to first-party crates must encode their whole Bazel label, in an invertible fashion. (This doesn't necessarily need to be human readable, since extern crate statements can be generated in Rust source by a proc macro. But it's nice to make it human readable for easier debugging, etc.)
  • References to third-party crates (in first-party code) must also encode the whole Bazel label, to enable automated BUILD file updates.
  • References to third-party crates (in third-party code) must not need to be changed. I.e., vendored third-party crates must not be renamed to encode the full Bazel label.

We're experimenting with a proc_macro to achieve this. Example usage of the macro could be the following:

import! {
  "//some_project:utils";
  "//some_project:ugly_name" as something_else;
  "//third_party/rust/serde";
  "//third_party/rust/clap" as aliased_clap;
}

This would expand to:

extern crate some_project_COLON_utils as utils;
extern crate some_project_COLON_ugly_name as something_else;
extern crate serde;
extern crate clap as aliased_clap;

Note here, that the source completely encodes its dependencies (so the BUILD file can be updated automatically without any other knowledge of the repo), but the vendored crates don't need to be renamed.

In order for this to work, we need to implement the above proc_macro, and to modify the implementation of rust_library to create crates that follow the above naming scheme. Since not every repository will need to follow this scheme (and since it'd be a breaking change), this naming scheme will be guarded behind a flag.

We will also add an associated flag to control the location of vendored deps, i.e. which targets are considered to be third-party and therefore exempt from renaming.

@UebelAndre
Copy link
Collaborator

Sorry, what specifically does rename_1p_crates mean? what is 1p? First party?

@cfredric
Copy link
Contributor Author

cfredric commented Dec 4, 2021

Yup, "first-party". Happy to use a different name if you have suggestions, I'm not set on this one in particular.

@UebelAndre
Copy link
Collaborator

I'm no stranger to verbosity so I'd opt for something like rename_first_party_crates. This is something I'd expect to live in a bazelrc somewhere, written only once. I think making it less cryptic would be better 😀

@cfredric
Copy link
Contributor Author

cfredric commented Dec 6, 2021

Sounds good - I've renamed the flag in #1056 :)

@cfredric
Copy link
Contributor Author

cfredric commented Mar 1, 2022

Tentatively closing this issue now, since I think all the parts we need have been implemented. I'll reopen if we discover any issues in our experimentation downstream.

@cfredric cfredric closed this as completed Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants