-
Notifications
You must be signed in to change notification settings - Fork 931
Rework lighthouse_version to reduce spurious recompilation
#8336
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
Conversation
| args = [ | ||
| "--always", | ||
| "--dirty=+", | ||
| "--abbrev=7", | ||
| // NOTE: using --match instead of --exclude for compatibility with old Git | ||
| "--match=thiswillnevermatchlol" | ||
| ], | ||
| prefix = "Lighthouse/v8.0.0-rc.2-", | ||
| fallback = "Lighthouse/v8.0.0-rc.2" | ||
| ); | ||
| /// `Lighthouse/v8.0.0-67da032` | ||
| pub const VERSION: &str = env!("GIT_VERSION"); | ||
|
|
||
| /// Returns the first eight characters of the latest commit hash for this build. | ||
| /// | ||
| /// No indication is given if the tree is dirty. This is part of the standard | ||
| /// for reporting the client version to the execution engine. | ||
| pub const COMMIT_PREFIX: &str = git_version!( | ||
| args = [ | ||
| "--always", | ||
| "--abbrev=8", | ||
| // NOTE: using --match instead of --exclude for compatibility with old Git | ||
| "--match=thiswillnevermatchlol" | ||
| ], | ||
| prefix = "", | ||
| suffix = "", | ||
| cargo_prefix = "", | ||
| cargo_suffix = "", | ||
| fallback = "00000000" | ||
| ); | ||
| pub const COMMIT_PREFIX: &str = env!("GIT_COMMIT_PREFIX"); |
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.
We could also consider moving to an 8 character commit hash. Currently we provide both a 7 character hash (for --version, etc) and an 8 character hash (for execution engine reporting). If we can use 8 character hashes for everything, that would reduce some complexity
| update_cargo_toml ../account_manager/Cargo.toml | ||
| update_cargo_toml ../beacon_node/Cargo.toml | ||
| update_cargo_toml ../boot_node/Cargo.toml | ||
| update_cargo_toml ../lcli/Cargo.toml | ||
| update_cargo_toml ../lighthouse/Cargo.toml | ||
| update_cargo_toml ../validator_client/Cargo.toml |
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.
I'm using workspace version inheritance for the binaries that were explicitly listed here, but we could also include the other ones (database_manager, validator_manager, slasher) to keep it consistent
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.
This is amazing! I have been annoyed with this behaviour for years. Thanks for fixing it.
rust-analyzer runs so much quicker now that it doesn't recompile lighthouse_version each time.
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.
Looks great! So glad to have this fixed finally!
Issue Addressed
#8311
Proposed Changes
Removes the
git_versioncrate fromlighthouse_versionand implements gitHEADtracking manually.This removes the (mostly) broken dirty tracking but prevents spurious recompilation of the
lighthouse_versioncrate.This also reworks the way crate versions are handled by utilizing workspace version inheritance and Cargo environment variables.
This means the only place where Lighthouse's version is defined is in the top level
Cargo.tomlfor the workspace. All relevant binaries then inherit this version. This largely makes thechange_version.shscript useless so I've removed it, although we could keep a version which just alters the workspace version (if we need to maintain compatibility with certain build/release tooling.When is a Rebuild Triggered?
Note that working/staged changes will not trigger a recompile of
lighthouse_version.Additional Info
There might exist certain edge cases with packed git refs where a recompile might not get triggered after a git garbage collection cycle but I suspect this will be quite rare.