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

Request to support disabling path checks when using cargo install #14276

Open
jdknight opened this issue Jul 20, 2024 · 5 comments
Open

Request to support disabling path checks when using cargo install #14276

jdknight opened this issue Jul 20, 2024 · 5 comments
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-install S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.

Comments

@jdknight
Copy link

Problem

Been using a couple of tools which manage building multiple packages for a target. For a project, it may use a package which is Cargo-based and will eventually call cargo build/cargo install when processed. These tools will install Cargo packages into a target that is not in the path, but Cargo will generate a warning:

warning: be sure to add `<path>` to your PATH to be able to run the installed binaries

Such a warning is not applicable for these install cases, and it would be nice to have a way to ignore them. An example includes a Buildroot run that uses a cargo-based package can have logs that shows the warning:

>>> tealdeer 1.6.1 Installing to target
cd /home/autobuild/autobuild/instance-15/output-1/build/tealdeer-1.6.1/ && ...
  Installing tealdeer v1.6.1 (/home/autobuild/autobuild/instance-15/output-1/build/tealdeer-1.6.1)
    Finished release [optimized] target(s) in 2.35s
  Installing /home/autobuild/autobuild/instance-15/output-1/target/usr/bin/tldr
   Installed package `tealdeer v1.6.1 (/home/autobuild/autobuild/instance-15/output-1/build/tealdeer-1.6.1)` (executable `tldr`)
warning: be sure to add `/home/autobuild/autobuild/instance-15/output-1/target/usr/bin` to your PATH to be able to run the installed binaries

Proposed Solution

It would be nice to have an environment option or a command line flag to suppress such a warning.

A suggested reference implementation can be seen in #14274.

Notes

I made the unfortunate decision to create the pull request before submitting this issue (:bow:). To pull over an initial comment already brought up the in the pull request:

We have an unstable [lints.cargo] table under development.

I do not believe the prospect [lints.cargo] table is related to this proposed no-path-check configuration hint. The proposed option is more so tailored for users managing installs for a series Cargo packages and not specific to an individual package.

It appears to me that the focus of #12235 is for linting-specific handling. And since I believe a proposed no-path-check is not part of this scope, I have created this new issue. Feel free to correct me if I'm wrong here.

Note that the reference implementation does provide an example supporting a command line option, a configuration flag and an environment variable. Personally, I would be all for just an environment variable (e.g. CARGO_INSTALL_NO_PATH_CHECK), since these scenarios typically have means to manage the environment easily. I only provided the extended options since I did not observe any non-internal flag case where only an environment flag was used.

@jdknight jdknight added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage. labels Jul 20, 2024
@weihanglo
Copy link
Member

Have mixed feeling about suppressing this warning. It's simple enough that we can just do it. OTOH it also bloats the CLI and configuration interface for such a non-harmful warning (it is not even an error).

In terms of the implementation, I don't think we need a standalone flag in cargo-install. Maybe the configuration is sufficient, as it is able to specify configuration via --config install.no-patch-check=true

@weihanglo weihanglo added Command-install S-needs-team-input Status: Needs input from team on whether/how to proceed. and removed S-triage Status: This issue is waiting on initial triage. labels Aug 1, 2024
@weihanglo
Copy link
Member

So maybe adding a install.path-check = <boolean> configuration is good enough?
I don't think anyone needs it as a hard error, so no need to have enum options like diabled|warn|forbid.

@epage
Copy link
Contributor

epage commented Aug 1, 2024

I agree that a CLI flag is overkill for this.

Note that we get env variables "for free" when adding a config field.

@epage
Copy link
Contributor

epage commented Aug 1, 2024

Maybe there is something I'm missing but I don't understand why this is a problem that needs fixing.

Even if it is, I wonder if a one-off solution is the right approach. We have been working on a linting system and --warnings=deny support. All of that is geared towards workspace-related lints. We've not discussed how we want to handle non-workspace diagnostics, like config or CLI. I worry that we'll do a one off here that won't align well with what we actually want long term. If this was impactful enough, it might be worth a short term solution, but I don't feel this is.

@weihanglo weihanglo added S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. and removed S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Aug 9, 2024
@jdknight
Copy link
Author

The goal is to remove a warning that is not applicable to a certain environments (cross compiling for a target). If the installed binaries were in the path for such an environment, it will most likely be an error. Having one less warning message to look at can be nice for builders.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-install S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.
Projects
None yet
Development

No branches or pull requests

3 participants