trishume / syntect

Rust library for syntax highlighting using Sublime Text syntax definitions.
https://docs.rs/syntect
MIT License
1.85k stars 130 forks source link

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

Closed dtolnay closed 6 months ago

dtolnay commented 6 months ago

2021 edition is supported since Rust 1.56 (October 2021). Based on https://github.com/trishume/syntect/pull/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 commented 6 months ago

Thanks!

(A note regarding your PR https://github.com/sharkdp/bat/pull/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.)