-
Notifications
You must be signed in to change notification settings - Fork 148
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
Remove frozen-abi build script #2911
Remove frozen-abi build script #2911
Conversation
The Firedancer team maintains a line-for-line reimplementation of the |
30978c3
to
4c0f5a0
Compare
I appreciate these changes and think this is in good direction. just curious. how much is build time reduced? |
also, could you fix conflicts? (sorry for the delayed code-review btw...) |
0f759a3
to
ea66840
Compare
Seems to be 10-100 ms when the frozen-abi feature is deactivated. That's more than I would have guessed since the build script is empty when the feature is deactivated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(it seems that "rustc_version 0.4.1"
itself can't still get rid of because some trnasitive deps (namely ark-ff
, cast
and curve25519-dalek
) depends on that..)
[build-dependencies] | ||
rustc_version = { workspace = true, optional = true } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems this isn't needed to begin with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm; come to think of it, feature-based gating makes sense then custom build.rs and custom cfg... really thanks for all the efforts of fixing this one by one.
Problem
Since all frozen-abi stuff is feature gated these days, we don't need
#[cfg(RUSTC_WITH_SPECIALIZATION)]
any more. We can just rely on thefrozen-abi
feature that already exists for all the crates that need it.Summary of Changes
RUSTC_WITH_SPECIALIZATION
with checks forfeature = "frozen-abi"
RUSTC_WITH_SPECIALIZATION
). In a few cases, there is still a build script doing other tasks, and I've only removed the frozen-abi parts.rustc_version
dependencies, as these are only used by frozen-abi build scriptsfrozen-abi
feature of those crates is activated. The point of this is to make sure the workspace compiles on stable Rust - thefrozen-abi
feature is only for nightly Rust