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.

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

Switch to 2021 edition and Cargo's v2 resolver #510

Merged
merged 1 commit into from
Dec 28, 2023

Conversation

dtolnay
Copy link
Contributor

@dtolnay dtolnay commented Dec 28, 2023

2021 edition is supported since Rust 1.56 (October 2021). Based on #462, syntect's current toolchain support is 1.73+, so there should be no issue with version support.

Cargo uses a different feature resolver by default when the project root uses 2021 edition. https://doc.rust-lang.org/edition-guide/rust-2021/default-cargo-resolver.html. Example behavior difference relevant to syntect:

[package]
name = "repro"
edition = "..."

[dependencies]
serde = "1"

[dev-dependencies]
serde = { version = "1", features = ["derive"] }

With 2018 edition, cargo check will build serde with "derive" feature turned on, despite not having run cargo check --tests i.e. the feature is definitely not needed for this build. With 2021 edition, cargo check will build serde with "derive" feature off, while cargo check --tests builds serde with "derive" feature on.

We can see the impact this has on syntect by running cargo tree --format '{p} [{f}]' > 2018 with 2018 edition and cargo tree --format '{p} [{f}]' > 2021 with 2021 edition, followed by diff 2018 2021. Here are the packages that this PR immediately affects:

9c9
< │           └── syn v2.0.43 [clone-impls,default,derive,full,parsing,printing,proc-macro,quote,visit]
---
> │           └── syn v2.0.43 [clone-impls,default,derive,parsing,printing,proc-macro,quote]
28c28
< │       │   └── libc v0.2.151 [default,std]
---
> │       │   └── libc v0.2.151 []
56c56
< │       └── syn v2.0.43 [clone-impls,default,derive,full,parsing,printing,proc-macro,quote,visit] (*)
---
> │       └── syn v2.0.43 [clone-impls,default,derive,parsing,printing,proc-macro,quote] (*)
64c64
< │   │   └── libc v0.2.151 [default,std]
---
> │   │   └── libc v0.2.151 []

I am sending this PR to prepare for a different PR I intend to send, which will make syntect no longer depend on serde's "derive" feature, allowing better pipelining of downstream builds.

Syntect switching to 2021 edition or v2 resolver is not a hard prerequisite for that other PR, because only the project root is relevant to what resolver Cargo chooses to use. A single resolver is used per build, not per crate. So, the downstream projects would get the benefit of pipelining whether or not syntect uses the v2 resolver in its own repo.

However, I am sending this PR first because it enables the next PR to be profiled (cargo build --timings) against syntect in isolation. Without this PR, profiling would need to be done against some other repo instead, because profiling in this repo would be misleading as syntect's dev-dependencies would cause features to be turned on which are not necessarily turned on when some other crate depends on syntect.

@Enselic
Copy link
Collaborator

Enselic commented Dec 28, 2023

Thanks!

(A note regarding your PR sharkdp/bat#2812: I merged a fix for the unrelated CI failure, so if you rebase and add a line to CHANGELOG.md the CI should become green for that PR.)

@Enselic Enselic merged commit 77b548b into trishume:master Dec 28, 2023
4 checks passed
@dtolnay dtolnay deleted the edition branch December 28, 2023 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants