cargo icon indicating copy to clipboard operation
cargo copied to clipboard

`cargo::` syntax should be permitted despite MSRV with the right configuration

Open joshlf opened this issue 1 year ago • 8 comments

Currently, zerocopy's build.rs script contains the following:

    for version_cfg in version_cfgs {
        if rustc_version >= version_cfg.version {
            println!("cargo:rustc-cfg={}", version_cfg.cfg_name);
        }
    }

We have a lot of cfgs that are now triggering the recently-added unexpected_cfgs lint. In order to fix this, I added the following to our build script:

    if rustc_version >= (Version { major: 1, minor: 77, patch: 0 }) {
        for version_cfg in &version_cfgs {
            println!("cargo::rustc-check-cfg=cfg({})", version_cfg.cfg_name);
        }
    }

Note that this is gated to only run on Rust version 1.77 or later, since that's when the cargo:: syntax was introduced.

Unfortunately, this causes the following error to be emitted:

error: the `cargo::` syntax for build script output instructions was added in Rust 1.77.0, but the minimum supported Rust version of `zerocopy v0.8.0-alpha.9 (.../zerocopy)` is 1.56.0.
See https://doc.rust-lang.org/cargo/reference/build-scripts.html#outputs-of-the-build-script for more information about build script outputs.

This is spurious since our version detection logic will ensure that we don't emit these annotations prior to 1.77, but since it's a hard error, there's no way to work around it.

It would be great if there were a way to ask Cargo to disable this error for cases like these. The alternative for us is to just add a blanket #![allow(unexpected_cfgs)], which would be a shame since that lint is genuinely useful.

(I will also note that this error seems especially odd given that pre-1.77 toolchains simply ignore cargo::; it doesn't cause a compilation error or even a warning)

joshlf avatar May 06 '24 17:05 joshlf

Strictly Cargo-related.

@rustbot transfer cargo

fmease avatar May 06 '24 18:05 fmease

Does the single colon version work for you? (cargo:rustc-check-cfg specifically).

weihanglo avatar May 06 '24 18:05 weihanglo

Huh yeah that works, thanks! Maybe the lint could suggest the single-colon version instead? Or maybe the MSRV error could note the single-colon version as an alternative in its help text?

joshlf avatar May 06 '24 18:05 joshlf

They are all good suggestions!

@Urgau any opinion on this? Should we mention only single-colon syntax everywhere to avoid compatibility hazard?

weihanglo avatar May 06 '24 18:05 weihanglo

I don't know if we should mention it in the diagnostic output it's already quite heavy; also mentioning the single column wouldn't be useful for the majority of users.

However I think like in the blog post we could mention it in the documention of rustc-check-cfg in the Cargo book.

Urgau avatar May 06 '24 19:05 Urgau

Maybe it could be mentioned in the MSRV error, since that's a case where you know for a fact that it's relevant to the user? Ie, an error that says something like "note that you can get this to work by using cargo: instead of cargo::"?

joshlf avatar May 06 '24 19:05 joshlf

Maybe it could be mentioned in the MSRV error, since that's a case where you know for a fact that it's relevant to the user? Ie, an error that says something like "note that you can get this to work by using cargo: instead of cargo::"?

I am open to this. We could emit such a suggestion if the given instruction is one of the prefix in this list:

https://github.com/rust-lang/cargo/blob/85a4d7041f3898a86078ba19c057a2c07b84b93d/src/cargo/core/compiler/custom_build.rs#L703-L721

weihanglo avatar May 06 '24 22:05 weihanglo

I'll look into this.

torhovland avatar May 07 '24 07:05 torhovland

Just want to point out that the docs are misleading, as they say

The old invocation prefix cargo: (one colon only) is deprecated and won’t get any new features.

implying that this switchover happened with 1.77 when support for the cargo:: prefix shipped. As mentioned in this issue, this is a problem since specifying an msrv in Cargo.toml causes hard compilation errors if you use a newer cargo with a pre-1.77 msrv but this error with rustc-check-cfg only happens under 1.80+ so it makes no sense to complain about it if you're using a rust version prior to that.

But the good news is that (at least with 1.80 nightly), cargo:rustc-check-cfg works even though the docs imply that it requires cargo::.

mqudsi avatar May 08 '24 20:05 mqudsi

That is an oddity from the order of things being implemented compared to the order they were stabilized.

epage avatar May 08 '24 20:05 epage

I assumed as much (it’s why I thought to try it to begin with), but I’m grateful for the workaround all the same!

mqudsi avatar May 08 '24 20:05 mqudsi

We just ran into this same problem in PyO3. Will there ever be a point where the deprecated cargo: syntax will be removed?

We typically have quite low MSRV (currently 1.63) and are worried that we might reach a state where we cannot use cargo:: syntax due to the hard error and cannot use cargo: syntax because it has been removed.

davidhewitt avatar May 15 '24 09:05 davidhewitt

Will there ever be a point where the deprecated cargo: syntax will be removed?

No it hasn't been and will not be removed. (at least won't be removed in a incompatible way)

check-cfg is in a weird state because of the stabilization order, though it should be generally fine with a single colon. Anything didn't work as expected?

weihanglo avatar May 15 '24 11:05 weihanglo

All working ok, for now we're just sticking with cargo: single colon and will update in the future. Thanks for confirming!

davidhewitt avatar May 15 '24 11:05 davidhewitt