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 [--git?] ignores .cargo/config.toml, making packages publishable-but-unbuildable, allthewhile they work when built normally #14001

Open
nabijaczleweli opened this issue Jun 2, 2024 · 8 comments
Labels
A-configuration Area: cargo config files and env vars C-bug Category: bug Command-install S-triage Status: This issue is waiting on initial triage.

Comments

@nabijaczleweli
Copy link
Contributor

Problem

As given in title.

Steps

  1. git clone --branch v2.0.0 https://github.com/thecoshman/http
  2. cargo build it from the directory (works)
  3. cargo install --git --branch v2.0.0 https://github.com/thecoshman/http (or whatever the spelling is, this is currently HEAD) –
   Compiling rand v0.7.3
error[E0554]: `#![feature]` may not be used on the stable release channel
 --> /home/nabijaczleweli/.cargo/registry/src/index.crates.io-6f17d22bba15001f/brotli-6.0.0/src/lib.rs:8:39
  |
8 | #![cfg_attr(feature = "simd", feature(portable_simd))]
  |                                       ^^^^^^^^^^^^^
  1. Removing features = simd from brotli fixes it.

Possible Solution(s)

No response

Notes

May have something to do with .cargo/config.toml setting env.RUSTC_BOOTSTRAP = 1? But the thing it's used for in the crate itself does work.

Version

$ cargo version --verbose
cargo 1.78.0 (54d8815d0 2024-03-26)
release: 1.78.0
commit-hash: 54d8815d04fa3816edc207bbc4dd36bf18014dbc
commit-date: 2024-03-26
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.2 vendored)
libcurl: 8.6.0-DEV (sys:0.4.72+curl-8.6.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Debian 12 (bookworm) [64-bit]
@nabijaczleweli nabijaczleweli added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Jun 2, 2024
@Eh2406
Copy link
Contributor

Eh2406 commented Jun 3, 2024

The config env.RUSTC_BOOTSTRAP = 1 is telling stable rust to ignore that error. And config files are ignored for install. That is why you are seeing the error. RUSTC_BOOTSTRAP is a very heavy hammer, one only intended for building the compiler itself.

@nabijaczleweli
Copy link
Contributor Author

And yet it is the only way to achieve feature parity of win32 targets with unix ones, since I use it for st_dev and st_ino (rust-lang/rust#63010). But, whatever.

Indeed, I haven't tested cargo install --git on windows, but this does also break that:

error[E0554]: `#![feature]` may not be used on the stable release channel
 --> src/main.rs:1:44
  |
1 | #![cfg_attr(target_os = "windows", feature(windows_by_handle))]
  |                                            ^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0554`.
error: could not compile `https` (bin "http") due to 1 previous error
warning: build failed, waiting for other jobs to finish...
error: could not compile `https` (bin "httplz") due to 1 previous error
error: failed to compile `https v2.0.1 (http://github.com/thecoshman/http#152f4d70)`, intermediate artifacts can be found at `T:\-_-TEM~1\cargo-installbcOmti`.
To reuse those artifacts with a future compilation, set the environment variable `CARGO_TARGET_DIR` to that path.

So this is not "RUSTC_BOOTSTRAP doesn't inherit", as I thought, but
"cargo expressly ignores package configuration when installing", so just make it not do that?

Fixing "we choose to make certain packages publishable-but-unbuildable by ignoring configuration found in the distribution that allows them to be built in the usual case" is done by just not doing that.

Or, as I have just tested, having users set RUSTC_BOOTSTRAP=1 before installing. Not great, but sure.

@nabijaczleweli nabijaczleweli changed the title Building in-tree allows brotli feature simd in dependency; building with install --git errors "error[E0554]: #![feature] may not be used on the stable release channel" cargo install [--git?] ignores .cargo/config.toml, making packages publishable-but-unbuildable, allthewhile they work when built normally Jun 3, 2024
nabijaczleweli added a commit to thecoshman/http that referenced this issue Jun 3, 2024
nabijaczleweli added a commit to thecoshman/http that referenced this issue Jun 3, 2024
…ursuant to the aforementioned. Import Debian repository info
@epage epage added A-configuration Area: cargo config files and env vars Command-install labels Jun 3, 2024
@epage
Copy link
Contributor

epage commented Jun 3, 2024

So if I'm understanding, you are wanting cargo install to use the config file from the --git repo, right?

Config is environmental and not tied to the project (#2930, see #12738 for finding ways the right abstraction for representing config in the project). According to these rules, cargo install should use the config of your current-dir independent of where the source is. You can see this if you do cargo check --manifest-path <some-dir> that the config from <some-dir> won't be used, instead requiring something like #10098. For various reasons, cargo install varies from this pattern and only the user-wide configuration is used and not the config for the current-dir.

@nabijaczleweli
Copy link
Contributor Author

nabijaczleweli commented Jun 3, 2024

Well, if I publish a package (be it via git or by uploading a tarball to crates.io), the fundamental expectation is that it will be built in the same way on the user's machine. This expectation is not being up-held when using the cargo install short-hand but it is if the user explicitly does what cargo install promises to fuse: downloading the tarball (or checking out a worktree), cargo build --release, mv target/... $CARGO_BINS.

If you already have an exception for cargo install for "various reasons", then this means that you can append this issue to those reasons and edit the exception to use the package's configuration, rather than none at all.

@epage
Copy link
Contributor

epage commented Jun 3, 2024

Well, if I publish a package (be it via git or by uploading a tarball to crates.io), the fundamental expectation is that it will be built in the same way on the user's machine.

That is not a fundamental expectation. cargo build does dev profile by default while cargo install does release profile. See also #7169.

btw #11036 is a related issue.

And as mentioned, that this is behaving something like cargo build in that its like

$ cargo build --manifest-path ~/.cargo/git/checkouts/somedep-e7ff1db891893a9e/d04355a/Cargo.toml

(ie we aren't changing the current-dir on the users behalf)

The difference is that current-dir is ignored.

This gets to the core of problem: config is environmental and not scoped to a project. If you aren't in the environment then Cargo doesn't load the config.

If you already have an exception for cargo install for "various reasons", then this means that you can append this issue to those reasons and edit the exception to use the package's configuration, rather than none at all.

This is something that could be re-evaluated. It would take some research to see what the trade offs would be. However, more likely to move forward is efforts like #12738.

Either way, this would be use-case driven and env.RUSTC_BOOTSTRAP = 1 is not a sympathetic use case to me as you are publishing a package that purportedly can work on any stable release but fundamentally can't. This is instead a package that requires a nightly toolchain and should recognize that.

@nabijaczleweli
Copy link
Contributor Author

if you're trying to imply that because cargo install defaults to --release (which you, in your belaboured expansion, also omitted, like me, because it's irrelevant) while cargo build doesn't, and this is ideologically significant, then idk what to tell you except "lol"

If it's ignored, then unignore it and use the one next to the manifest. or chdir to the build root. this seems very easy to fix.

And no, I'm publishing a package that can work on stable releases which include the windows_by_handle feature in its current or similar form, which is all of them since 1.38 (if I'm reading github's git tag --contains equivalent right; the code, after moving twice, is unchanged and still blames back to the original merge from exactly 5 years ago); this is no different to using any other feature, other than you don't like it.
Usually this is just called MSRV and ignored because it's implicit and fluctuates by accident. I opted into it explicitly. Egad!
I doubt you'll find many packages conforming to your much more stringent "works on any stable release" criterion precisely for this reason.

It's also obviously false that it requires a nightly toolchain, as proven by me using a range of stable toolchains to build it for the past seven years.

@epage
Copy link
Contributor

epage commented Jun 3, 2024

if you're trying to imply that because cargo install defaults to --release (which you, in your belaboured expansion, also omitted, like me, because it's irrelevant) while cargo build doesn't, and this is ideologically significant, then idk what to tell you except "lol"

I recommend to please respectfully engage with others and assume they are trying to also participate in good faith with you. I am going to step away from this conversation for several days (at least) to let things calm down before further participation.

@technetos
Copy link
Member

Hey folks, please treat each other with kindness and respect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-configuration Area: cargo config files and env vars C-bug Category: bug Command-install S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests

4 participants