bazelbuild / rules_rust

Rust rules for Bazel
https://bazelbuild.github.io/rules_rust/
Apache License 2.0
666 stars 428 forks source link

crate_universe unable to use Cargo workspaces with globbed members #1199

Open gautamg795 opened 2 years ago

gautamg795 commented 2 years ago

Given a workspace defined in Cargo.toml as:

[workspace]
members = ["crates/*"]

crate_universe is unable to build crates as it seems to parse the crates/* string as the name of a member, and complains that this crate's manifest is not properly included as such:

Error: Some manifests are not being tracked. Please add the following labels to the `manifests` key: {
    ":crates/*/Cargo.toml",
}

I think it's a separate issue in that the manifests attribute of the crates_repository rule cannot accept a glob pattern so perhaps there isn't an easy fix for this, but the error message could be more clear to indicate that the workspace members must be explicitly listed out in Cargo.toml

UebelAndre commented 2 years ago

I wasn't aware that was something even cargo supported. Can you link to some documentation on that? The goal is for crate_universe to be able to ingest valid cargo manifests. If cargo supports globs then crate_universe should be able to ingest manifests with globs.

I think it's a separate issue in that the manifests attribute of the crates_repository rule cannot accept a glob pattern so perhaps there isn't an easy fix for this, but the error message could be more clear to indicate that the workspace members must be explicitly listed out in Cargo.toml

However, for globs in the manifests attribute, this would require some change to Bazel since I don't otherwise know how to glob in a WORKSPACE file. I would prefer this to be the case over potentially replacing the attribute type from label_list to string_list and then attempting to do some manual handling of glob patterns. I'd like to track that as a separate issue though. If it's something you feel strongly about can you open a separate issue for it?

cameron-martin commented 2 years ago

You can find documention on this here: https://doc.rust-lang.org/cargo/reference/workspaces.html#the-workspace-section

The members list also supports globs to match multiple paths, using typical filename glob patterns like * and ?.

ismell commented 1 year ago

I think this is related, but maybe not. The crates_vendor target requires that the list of manifests contain all the members defined in the workspace. Is there a reason for that? If we support globbing members we would still need to update the crates_vendor target.

cameron-martin commented 1 year ago

I think this is so the repository rule is re-run when those files change, even though it seems redundant. This reveals an issue with supporting globs too, because there is no way of ensuring the repository rule is re-run when a manifest is added.

gautamg795 commented 11 months ago

Just to confirm I understand -- globs will/can not be supported, but the only remaining issue is to show a more clear error when the workspace specification does contain globs?