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

cargo install does not respect .cargo/config.toml #11660

Closed
thoren-d opened this issue Jan 31, 2023 · 8 comments · Fixed by #11763
Closed

cargo install does not respect .cargo/config.toml #11660

thoren-d opened this issue Jan 31, 2023 · 8 comments · Fixed by #11763
Assignees
Labels
A-documenting-cargo-itself Area: Cargo's documentation C-bug Category: bug Command-install Command-uninstall E-easy Experience: Easy

Comments

@thoren-d
Copy link

thoren-d commented Jan 31, 2023

Problem

When I run cargo install ripgrep in a directory with a .cargo/config.toml overriding the install root (like below), it installs to $HOME/.cargo/bin instead.

The interesting thing is that cargo uninstall reads the configured root correctly, so if I immediately run cargo uninstall ripgrep in the same directory, it complains that package ID specification ripgrep did not match any packages.

Contents of .cargo/config.toml:

[install]
root = "./apps"

Steps

  1. Create a config file like the one above.
  2. Make an apps directory
  3. Run cargo install ripgrep (or anything)

Possible Solution(s)

No response

Notes

Also tried nightly just now with the same result.

Obviously I can just cargo install --root apps ..., but I was trying to experiment with patching dependencies and building with different compile options and it seems cargo install is completely ignoring the configuration file.

Version

cargo 1.67.0 (8ecd4f20a 2023-01-10)
release: 1.67.0
commit-hash: 8ecd4f20a9efb626975ac18a016d480dc7183d9b
commit-date: 2023-01-10
host: x86_64-pc-windows-msvc
libgit2: 1.5.0 (sys:0.16.0 vendored)
libcurl: 7.86.0-DEV (sys:0.4.59+curl-7.86.0 vendored ssl:Schannel)
os: Windows 10.0.22621 (Windows 10 Pro) [64-bit]
@thoren-d thoren-d added the C-bug Category: bug label Jan 31, 2023
@ehuss
Copy link
Contributor

ehuss commented Jan 31, 2023

Thanks for the report! It is intentional behavior that cargo install <registry-package> does not honor the configuration in the current working directory. I think there are two issues here:

  • That does not seem to be documented in the cargo install page.
  • cargo uninstall should probably have similar behavior to avoid the asymmetry.

@weihanglo
Copy link
Member

For the installation root precedence, the rule is not documented at all. It starts searching configurations upward from the given --path, or CARGO_HOME if no --path provided. This should probably be documented as well.

let path = args.value_of_path("path", config);
if let Some(path) = &path {
config.reload_rooted_at(path)?;
} else {
// TODO: Consider calling set_search_stop_path(home).
config.reload_rooted_at(config.home().clone().into_path_unlocked())?;
}

I wonder if we could reconsider calling set_search_stop_path(home), though it is definitely a breaking change, so 🤷🏾‍♂️.

@weihanglo weihanglo added E-easy Experience: Easy E-help-wanted labels Feb 19, 2023
@jofas
Copy link
Contributor

jofas commented Feb 22, 2023

Any reason why cargo install ignores the configuration in the current directory? It would probably be a good idea to add the reason behind this decision to the documentation.

@rustbot claim

@weihanglo
Copy link
Member

@jofas
See #6026. The basic idea here is cargo install is a system or user level command instead of project-local. For --path #6804 tells that it makes sense --path respecting project local configurations.

@jofas
Copy link
Contributor

jofas commented Feb 22, 2023

Thanks for the explanation!

@jofas
Copy link
Contributor

jofas commented Feb 22, 2023

I think in order to document the fact that cargo install (and in the future cargo uninstall) only consider the global configuration file in $CARGO_HOME/config.toml, I'd like to add a small note here:

- `install.root` Cargo [config value](../reference/config.html)

Something along the lines of

Note: only the global configuration file $CARGO_HOME/config.toml is considered

As the snippet is only used for the man pages of install and uninstall (and both will have the same behaviour), I think this should be okay?

Somwhere here on the cargo install man page :

Installing with `--path` will always build and install, unless there are
conflicting binaries from another package. The `--force` flag may be used to
force Cargo to always reinstall the package.

I'd add a note saying that using --path will change the default behaviour of cargo install to only consider the configuration in $CARGO_HOME/config.toml to the configuration at the given path.

Any thoughts anyone? @weihanglo?

@weihanglo
Copy link
Member

I am fine with your suggestion, though it's a bit complicated. Not only install.root is ignored but the entire configuration discovery starts from $CARGO_HOME/config.toml instead of current working directory. I am not sure if it is worth putting so much concepts here. The suggested fix of --path looks good but falls into the same situation as well.

In terms of the display on manpage, I only found one quote usage in man in cargo bench. It looks good to me btw via cargo help bench (the bad part is due to link rendering). If you don't feel satisfied with the way it displays, we can also fall back to starting with plain Note that….

@jofas
Copy link
Contributor

jofas commented Feb 23, 2023

I am not sure if it is worth putting so much concepts here.

That was my reservation, too. I just couldn't figure out a good spot where I could add the relevant information in the text on first glance. Probably one paragraph describing the behaviour a little more detailed than I suggested is better. I'll figure something out and create a PR where we can discuss further details.

bors added a commit that referenced this issue Feb 26, 2023
Added documentation for the configuration discovery of `cargo install` to the man pages

Fixes #11660.
@bors bors closed this as completed in 447ccb4 Feb 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documenting-cargo-itself Area: Cargo's documentation C-bug Category: bug Command-install Command-uninstall E-easy Experience: Easy
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants