Add optional zerocopy trait derives #3407
Conversation
|
r? @JohnTitor (rustbot has picked a reviewer for you, use r? to override) |
|
I'm uploading this as a draft early so I can run stuff through CI. Definitely not ready for review yet! |
joshlf
force-pushed
the
zerocopy
branch
2 times, most recently
from
e72740e
to
9dd0f1b
Compare
|
@JohnTitor I do a have a question if you happen to have any insight: This PR adds a new Cargo feature which doesn't work on the MSRV of 1.13. I'm looking through the CI infrastructure but don't see an obvious place to put logic to test features only on more recent MSRVs. The README advertises various MSRVs for various features, so I assume such machinery must exist somewhere? |
joshlf
force-pushed
the
zerocopy
branch
6 times, most recently
from
73a4cca
to
e1425e2
Compare
|
You can tweak https://github.com/rust-lang/libc/blob/main/build.rs :) |
It looks like the current logic in that file automatically enables features unconditionally if the compiler version is high enough (ie, it doesn't require the user to enable the corresponding Cargo feature). Is that the behavior we want here? I assumed y'all would want this to remain optional, but if that's not the case, I'd be happy to make this unconditional following the logic used with the other features. |
|
Ah, hmm, fair enough. |
|
Sounds good. Do you have a sense of how I could enable this feature in CI, but only when the toolchain version is 1.61 (zerocopy's MSRV) or greater? |
|
It'd be hacky but something like this should be possible: Lines 69 to 79 in 5e32df0 |
|
Sounds good! Does this look good to you? |
joshlf
force-pushed
the
zerocopy
branch
4 times, most recently
from
c038816
to
c137795
Compare
|
Also, do these CI tests test on the MSRV (1.13)? I searched for code that enabled it, and found none: |
joshlf
force-pushed
the
zerocopy
branch
2 times, most recently
from
fc8980d
to
333bf03
Compare
| @@ -130,6 +130,7 @@ cargo-args = ["-Zbuild-std=core"] | |||
|
|
|||
| [dependencies] | |||
| rustc-std-workspace-core = { version = "1.0.0", optional = true } | |||
| zerocopy = { version = "0.7.16", optional = true, features = ["derive"] } | |||
There was a problem hiding this comment.
Leaving a TODO for myself:
- Rename to
zerocopy-0-7 - Add
zerocopy-0-6(to be removed before merge) to test that multiple version trains work
Since libc doesn't specify an edition, we have to use the legacy approach of importing macros via #[macro_use] extern crate zerocopy;, which may not play nicely with having macros from multiple versions of the same crate. Instead, we may need to update build.rs to detect when the Rust version is early enough that editions are supported, and instruct rustc to use a recent-enough version that we can explicitly import different versions under different names. This would be necessary in case a build requires multiple zerocopy versions simultaneously.
There was a problem hiding this comment.
See for more discussion: google/zerocopy#557
There was a problem hiding this comment.
Once rust-lang/libs-team#72 gets a conclusion, we can specify the edition.
No, I remember that I removed it at some point as something on the test pipeline had been broken. |
|
☔ The latest upstream changes (presumably #3393) made this pull request unmergeable. Please resolve the merge conflicts. |
This permits `Cargo.toml` to include optional dependencies. On toolchains earlier than 1.31, the optional dependency syntax is not supported. With the MSRV at 1.31, any optional dependency may be supported even if *its* MSRV is higher than *our* MSRV since only users using a more recent toolchain will enable that dependency. In particular, this paves the way for adding an optional `zerocopy` dependency (prototyped in rust-lang#3407 and rust-lang#3914; see also google/zerocopy#557), which permits many uses of libc to no longer require `unsafe` code.
This permits `Cargo.toml` to include optional dependencies. On toolchains earlier than 1.31, the optional dependency syntax is not supported. With the MSRV at 1.31, any optional dependency may be supported even if *its* MSRV is higher than *our* MSRV since only users using a more recent toolchain will enable that dependency. In particular, this paves the way for adding an optional `zerocopy` dependency (prototyped in rust-lang#3407 and rust-lang#3914; see also google/zerocopy#557), which permits many uses of libc to no longer require `unsafe` code.
This permits `Cargo.toml` to include optional dependencies. On toolchains earlier than 1.31, the optional dependency syntax is not supported. With the MSRV at 1.31, any optional dependency may be supported even if *its* MSRV is higher than *our* MSRV since only users using a more recent toolchain will enable that dependency. In particular, this paves the way for adding an optional `zerocopy` dependency (prototyped in rust-lang#3407 and rust-lang#3914; see also google/zerocopy#557), which permits many uses of libc to no longer require `unsafe` code.
This permits `Cargo.toml` to include optional dependencies. On toolchains earlier than 1.31, the optional dependency syntax is not supported. With the MSRV at 1.31, any optional dependency may be supported even if *its* MSRV is higher than *our* MSRV since only users using a more recent toolchain will enable that dependency. In particular, this paves the way for adding an optional `zerocopy` dependency (prototyped in rust-lang#3407 and rust-lang#3914; see also google/zerocopy#557), which permits many uses of libc to no longer require `unsafe` code.
|
Closing per #3317 (comment) |
No description provided.