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

[ijackson]

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 <[email protected]>",
         "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]

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

[weihanglo]

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]

See #13868. If there is any documentation improvement can be done, let us know.

[ijackson]

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]

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.

[epage]

Unknown diretives are ignored, so cargo::rustc-check-cfg is completely fine.

This isn't strictly true. There were no "unknown directives" but anything that wasn't a known directive was instead a metadata KEY. Even if this means that the KEY will be :rustc-check-cfg.

With cargo::, especially moving metadata keys under cargo::metadata=, unknown keys are now a hard error. Maybe this can be softened like we do with "unknown manifest field" warnings. However, that won't completely solve your problem, see below.

Normally Rust's stability guarantee would mean that if 1.56 works, so would 1.76. But that's not the case here.

This is actually fairly common. Say we add an unstable Cargo.toml field. Before we add it, it will be an "unused manifest key" warning. When we add it, it is an error saying that the feature is unstable. We then stabilize it and it i\s now valid to use it. Having features a hard error on stable is important for users to identify when a field with major semantic meaning is being unused. If it doesn't have major semantic meaning, we warn instead of error so people can use the feature without an MSRV bump, like with [lints].

If you only test your MSRV and its from before we made a field unstable, then you can run into this. It would be great if we provided diagnostics to let you know when use of a feature would violate your MSRV. It looks like we are in this case, so I'm a bit confused at how this broke you. @ijackson is there something I'm missing as to how you accidentally broke your MSRV?

One thing thats a little different with this build script situation compared to most Cargo.toml changes is that we took existing, valid cargo:KEY=VALUE when KEY starts with :, and changed its meaning. The error was there for a period of time to raise visibility of this changed meaning so we didn't start re-interpreting people's program without notification.

In the case of check-cfg, it doesn't have meaning in old versions.

As the hard error aligns with other uses hard error in build scripts and other places, I propose we close this.