rust-lang
Add `cfg_version` support
@epage I will of course take care of proper error handling, but I just want to get feedback on the initial direction.
~cfg(nightly) can be already parsed as a Name.~
cfg(nightly)can be already parsed as aName.
I'm not sure what this is referring to as I don't see this in the RFC or the rustc version
Be sure to check out rust-lang/rust#141137 for a summary of what has changed from the RFC
A question came up about how older versions of cargo will handle this.
At least for a local package, this causes an error like:
failed to parse `version('1.80')` as a cfg expression: unexpected content `('1.80')` found after cfg expression
That seems fine to me.
However, I have a little more concern about how this impacts the index.
At a minimum, can you add a test that shows using a dependency from the index that itself uses a cfg() dependency? So, essentially, something like:
Package::new("only_newer", "1.0.0").publish();
Package::new("bar", "2.0.0")
.file("src/lib.rs", "extern crate only_newer;")
.target_dep("only_newer", "1.0", "cfg(version('1.89.0'))")
.publish();
and having a regular dependency on bar. In my quick testing, this seems to be broken somehow, but I'm not sure how. Can you look into that?
Another thing to look into is if the crates.io parsing of the manifest will have any issues with this, or how it displays dependencies.
Unfortunately we don't have very good infrastructure for checking that older versions of cargo work with newer versions of the index. I think the best we have right now is #[cargo_test(requires_rustup_stable)], where we can make sure stable is compatible with what is generated by nightly. But that doesn't really help with a fixed version like "does rust 1.87 stay compatible with nightly". We have a whole module for doing that (https://github.com/rust-lang/cargo/blob/HEAD/tests/testsuite/old_cargos.rs), but those tests are disabled due to the cost.
If we are able to figure some way to test older versions, then one approach would be to make sure the index for a crate has a mix of old and new cfgs. So something like:
Package::new("only_newer", "1.0.0").publish();
Package::new("bar", "1.0.0").publish();
Package::new("bar", "2.0.0")
.file("src/lib.rs", "extern crate only_newer;")
.target_dep("only_newer", "1.0", "cfg(version('1.89.0'))")
.publish();
where you have a dependency on bar = "1.0" should still work on older versions of cargo (that is, they should not choke on the 2.0 index entry). AFAIK, that should be fine, since older cargos are supposed to ignore index entries it can't parse. However, I don't remember where cargo parses the cfg strings, and I don't want old versions to choke on that.
r? @ehuss
rustbot has assigned @ehuss. They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.
Use r? to explicitly pick a reviewer
:umbrella: The latest upstream changes (possibly 2d662d919ed8bfca29ac6dd5d6eec3332cf7ec49) made this pull request unmergeable. Please resolve the merge conflicts.
@epage Thanks for rewriting the history. I found it a bit disappointing that I wasn't listed as an author anymore. Therefore, I updated the author information of the commits and hope that you are happy with the current state.
I also fixed the CI by removing dbg!(input).
Finally, I resolved the merge conflicts.
Learning jj today helped a lot :D
If we agree on https://github.com/rust-lang/cargo/pull/15533#discussion_r2098745262, then the PR should be ready for merging.
I found it a bit disappointing that I wasn't listed as an author anymore.
Sorry that I forgot to preserve that!
FYI a concern was raised on the new stabilization issue (https://github.com/rust-lang/rust/pull/141766#issuecomment-2946372974) about what syntax is used for this.
@epage @ehuss Unfortunately, I was not able to find time to finish this PR. I will also be away for about 3 weeks. So feel free to add your changes and then merge.
@mo8it don't worry, there is nothing to do atm while the design is being re-evaluated
:umbrella: The latest upstream changes (possibly #15841) made this pull request unmergeable. Please resolve the merge conflicts.