rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.39k stars 2.35k forks source link

`build.rs` `cargo::rustc-check-cfg` spurious error with MSRV 1.56 #14147

Open ijackson opened 1 month ago

ijackson commented 1 month ago

Problem

The check for the cargo:: (double colon) syntax should apply only to directives that the MSRV would understand.

Steps

Cargo.toml:

[package]
name = "derive-deftly-macros"
version = "0.13.0"
edition = "2021"
readme = "README.md"
authors=["Ian Jackson <ijackson@chiark.greenend.org.uk>",
         "and the contributors to Rust derive-deftly"]
license = "MIT"
description="Macros that implement the derive_deftly crate"
homepage = "https://gitlab.torproject.org/Diziet/rust-derive-deftly"
repository = "https://gitlab.torproject.org/Diziet/rust-derive-deftly"
rust-version = "1.56"

# After editing this file, you will probably need to run
#   maint/update-bizarre
# to update the "bizarre" testing versions in tests/pub-export/bizarre-*

[features]
case = ["heck"]
expect = ["sha3", "syn/full"]
meta-as-expr = ["syn/full"]
meta-as-items = ["syn/full"]

[dependencies]
indexmap = ">=1.8, <3"
itertools = ">=0.10.1, <0.14"
proc-macro-crate = ">=1.1.3, <4"
proc-macro2 = "1.0.53"
quote = "1"
sha3 = { version = "0.10", optional = true }
strum = { version = ">=0.24, <0.27", features = ["derive"] }
syn = { version = "2.0.53", features = ["extra-traits"] }
void = "1"

heck = { version = ">=0.4, <0.6", optional = true }

[lib]
path = "macros.rs"
proc-macro = true

build.rs:

fn main() {
    println!(
        r#"cargo::rustc-check-cfg=cfg(derive_deftly_dprint)
cargo::rustc-check-cfg=cfg(feature, values("bizarre"))"#
    );
}

Expected output: it compiles.

Actual output:

error: the `cargo::` syntax for build script output instructions was added in Rust 1.77.0, but the minimum supported Rust version of `derive-deftly-macros v0.13.0 (/volatile/rustcargo/Rustup/Derive-Deftly/derive-deftly/macros)` is 1.56.
Switch to the old `cargo:rustc-check-cfg=cfg(derive_deftly_dprint)` syntax (note the single colon).
See https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script for more information about build script outputs.

Possible Solution(s)

Accept cargo:: syntax unless the directive that follows :: is one that a compiler which only supports : would understand.

Notes

Unknown diretives are ignored, so cargo::rustc-check-cfg is completely fine. I htink no version of rust that would understand rustc-check-cfg at all, doesn't understand ::.

Version

cargo 1.80.0-nightly (34a6a87d8 2024-06-04)
release: 1.80.0-nightly
commit-hash: 34a6a87d8a2330d8c9d578f927489689328a652d
commit-date: 2024-06-04
host: x86_64-unknown-linux-gnu
libgit2: 1.7.2 (sys:0.18.3 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 10.0.0 (buster) [64-bit]
ijackson commented 1 month ago

It turns out that while 1.56 ignores this, it was made deprecated somewhere between that and 1.76.

weihanglo commented 1 month ago

I think it makes sense being a hard error since it might be a typo hard to catch. As suggested from the error message, developers are able to just switch to the single colon version. There is also a feature release schedule mix between rustc-check-cfg and :: syntax, so the behavior. We can't travel back to patch 1.76, so I feel like it would be a wontfix.

Anything blocking you from switching to the single colon cargo:rustc-check-cfg?

weihanglo commented 1 month ago

See https://github.com/rust-lang/cargo/issues/13868. If there is any documentation improvement can be done, let us know.

ijackson commented 1 month ago

Anything blocking you from switching to the single colon cargo:rustc-check-cfg?

Not if it's going to be supported indefinitely.

One problem I have is that I accidentally broke the MSRV of my crate. I have CI tests that test with 1.56, which didn't detect this. Normally Rust's stability guarantee would mean that if 1.56 works, so would 1.76. But that's not the case here.

How many different Rust versions do I need to test with in CI. if I want to know that all versions since my MSRV will work? Which versions should I choose?

ETA: my ticket forpreventing this happening again: https://gitlab.torproject.org/Diziet/rust-derive-deftly/-/issues/104

weihanglo commented 1 month ago

One problem I have is that I accidentally broke the MSRV of my crate. I have CI tests that test with 1.56, which didn't detect this. Normally Rust's stability guarantee would mean that if 1.56 works, so would 1.76. But that's not the case here.

That is unfortunate. For Cargo.toml we intentionally allow unknown fields to reduce the risk of breaking things in future releases. We missed the scenario that people will opt-in a new syntax from 1.77 in a package with an older MSRV. It's always hard if we allowed a wildcard like syntax from the beginning then we made some change breaking people.

This is a useful reminder that emitting errors should follow pretty much the same compatibility practice as changes to Cargo.toml and config.toml when developing. Thank you for this bug report. I've opened #14150 for such a principle.