-
Notifications
You must be signed in to change notification settings - Fork 481
Add new cargo creds feature #3385
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
base: main
Are you sure you want to change the base?
Conversation
|
||
// if cargo home set we symlink to cargo home. | ||
// This is so credentials.toml works. | ||
if let Ok(cargo_home) = std::env::var("CARGO_HOME") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@UebelAndre @illicitonion Is there some other way to check that im not aware of to see if the environment is isolated? Can i check CARGO_BAZEL_ISOLATED
is set, and if its false, we ignore this and just assume if its not set, or a true we can symlink credentials.toml
to cargo home? I could pass isolated thru to the splicing config but im not sure if thats desireable.
I think the way to go here would be to pass the isolated flag into the splicer and then check since isolated can be set by env or field on the vendor / from_cargo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we've had to care if the environment is isolated from within the cargo-bazel binary. Isn't the starlark responsible for setting up the isolated CARGO_HOME
? If you want credentials to be installed there (which would come from the user's CARGO_HOME
anyway), why not install it there?
I would pass an explicit flag indicating isolation is enabled though, that sounds fine 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@UebelAndre The symlink appears to happen whenever get_rust_tools
is called in starlark, and the extensions.bzl
does not use that call to get the rustc / cargo and i do not see a way to re use that func call using the module_ctx
as it and the repository_ctx
do not share the same attributes. This has confused me and caused alot of pain on my side because i didn't realize they diverged in code paths lol.
This part of the code i want to change to use the isolated flag, this was just a short circuit for me to make sure it worked and i could symlink to cargo home. We still will need to symlink to it for the extensions.bzl
side as module_ctx
does not have a symlink method like repository_ctx
that i can find documented.
def get_rust_tools(repository_ctx, host_triple): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is shared with both bzlmod and workspace modes
rules_rust/crate_universe/private/common_utils.bzl
Lines 64 to 82 in f525346
def new_cargo_bazel_fn( | |
repository_ctx, | |
cargo_bazel_path, | |
cargo_path, | |
rustc_path, | |
isolated = True, | |
quiet = False): | |
"""A helper function to allow executing cargo_bazel in repository rules module extensions. | |
Args: | |
repository_ctx (repository_ctx): The repository rule or module extension's context. | |
cargo_bazel_path (path): Path The path to a `cargo-bazel` binary | |
cargo_path (path): Path to a Cargo binary. | |
rustc_path (path): Path to a rustc binary. | |
isolated (bool): Enable isolation upon request. | |
quiet (bool): Whether or not to print output from the executable. | |
Returns: | |
A function that can be called to execute cargo_bazel. | |
""" |
Where if you follow through to
rules_rust/crate_universe/private/common_utils.bzl
Lines 144 to 153 in f525346
def _cargo_home_path(repository_ctx): | |
"""Define a path within the repository to use in place of `CARGO_HOME` | |
Args: | |
repository_ctx (repository_ctx): The rules context object | |
Returns: | |
path: The path to a directory to use as `CARGO_HOME` | |
""" | |
return repository_ctx.path(".cargo_home") |
You'll see here is where the isolated CARGO_HOME
is defined. Perhaps this would be a good spot to link credentials? Admittedly, this code became super complicated in an effort to avoid making large changes but resulted in higher complexity and diverging code paths. But given that workspace support is on it's way out, I wouldn't be opposed to a bzlmod-only introduction of this feature. Do you think there's a path forward here to do symlinking in starlark?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can look into it! I just got it working by passing isolated to the splicing cli.
from_cargo
and vendor calls.CARGO_BAZEL_ISOLATED=false
for it to work