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.

Sign up for GitHub

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

Jump to bottom

Expose rust-version through env var #10713

Merged
merged 1 commit into from
Jun 6, 2022
Merged

Expose rust-version through env var #10713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/cargo/core/compiler/compilation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,10 @@ impl<'cfg> Compilation<'cfg> {
metadata.license_file.as_ref().unwrap_or(&String::new()),
)
.env("CARGO_PKG_AUTHORS", &pkg.authors().join(":"))
.env(
"CARGO_PKG_RUST_VERSION",
&pkg.rust_version().unwrap_or(&String::new()),
)
.cwd(pkg.root());

// Apply any environment variables from the config
Expand Down
3 changes: 3 additions & 0 deletions src/doc/src/reference/environment-variables.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,9 @@ corresponding environment variable is set to the empty string, `""`.
* `CARGO_PKG_REPOSITORY` — The repository from the manifest of your package.
* `CARGO_PKG_LICENSE` — The license from the manifest of your package.
* `CARGO_PKG_LICENSE_FILE` — The license file from the manifest of your package.
* `CARGO_PKG_RUST_VERSION` — The Rust version from the manifest of your package.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing going through my head is the question: "what principle governs us exposing the manifest as environment variables?".

For myself, exposing CARGO_PKG_RUST_VERSION feels very arbitrary from a cargo-design perspective. Why this field and not many of the others?

Copy link
Member Author

@flip1995 flip1995 May 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many of the other fields are already exposed. Like the version field is even exposed through 4 or 5 env vars. Most package metadata is exposed. I think this makes it more consistent.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also see #8758 where CARGO_PRIMARY_PACKAGE was added "primarily for Clippy".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many of the other fields are already exposed. Like the version field is even exposed through 4 or 5 env vars. Most package metadata is exposed. I think this makes it more consistent.

Except for CARGO_PRIMARY_PACKAGE , those all look targeted at the code being built, either so a crate can access its own metadata or build.rs / test purposes.

Also see #8758 where CARGO_PRIMARY_PACKAGE was added "primarily for Clippy".

Thanks for pointing out precedence on this. I am still interested in seeing if there is an answer to my question or if there is interest in creating an answer to my question. Until that time, I am not comfortable merging this though maybe others of the cargo team would be.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with waiting on other team members. Just wanted to bring this up through a PR again, because my question some time ago on Zulip didn't get any replies 😅

Copy link
Member

@joshtriplett joshtriplett May 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* `CARGO_PKG_RUST_VERSION` — The Rust version from the manifest of your package.
* `CARGO_PKG_RUST_VERSION` — The Rust version from the manifest of your package. Note that this is the minimum Rust version supported by the package, not the current Rust version.

(People regularly want Rust compiler version information, so I want to make sure people don't think this is the thing they're looking for for that.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this in the cargo team meeting and we have no guiding principle on what env variables should exist and are intending to be responsive to what different teams needs are. I'm fine merging this after the documentation gets improved.

Copy link
Contributor

@epage epage May 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised we don't link any of these fields to their manifest field's documentation. That would also be a natural way to clarify what this is intending to contain

Note that this is the minimum Rust version supported by the package, not the
current Rust version.
* `CARGO_CRATE_NAME` — The name of the crate that is currently being compiled.
* `CARGO_BIN_NAME` — The name of the binary that is currently being compiled (if it is a binary). This name does not include any file extension, such as `.exe`.
* `OUT_DIR` — If the package has a build script, this is set to the folder where the build
Expand Down
3 changes: 3 additions & 0 deletions tests/testsuite/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1314,6 +1314,7 @@ fn crate_env_vars() {
authors = ["wycats@example.com"]
license = "MIT OR Apache-2.0"
license-file = "license.txt"
rust-version = "1.61.0"

[[bin]]
name = "foo-bar"
Expand All @@ -1338,6 +1339,7 @@ fn crate_env_vars() {
static LICENSE: &'static str = env!("CARGO_PKG_LICENSE");
static LICENSE_FILE: &'static str = env!("CARGO_PKG_LICENSE_FILE");
static DESCRIPTION: &'static str = env!("CARGO_PKG_DESCRIPTION");
static RUST_VERSION: &'static str = env!("CARGO_PKG_RUST_VERSION");
static BIN_NAME: &'static str = env!("CARGO_BIN_NAME");
static CRATE_NAME: &'static str = env!("CARGO_CRATE_NAME");

Expand All @@ -1356,6 +1358,7 @@ fn crate_env_vars() {
assert_eq!("MIT OR Apache-2.0", LICENSE);
assert_eq!("license.txt", LICENSE_FILE);
assert_eq!("This is foo", DESCRIPTION);
assert_eq!("1.61.0", RUST_VERSION);
let s = format!("{}.{}.{}-{}", VERSION_MAJOR,
VERSION_MINOR, VERSION_PATCH, VERSION_PRE);
assert_eq!(s, VERSION);
Expand Down