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

Include compiler version in lighthouse --version #6392

Closed
wants to merge 2 commits into from

Conversation

michaelsproul
Copy link
Member

@michaelsproul michaelsproul commented Sep 13, 2024

Proposed Changes

In aid of making builds easier to reproduce, include the compiler's version information in lighthouse --version.

Initial testing for reproducible builds shows that Lighthouse can't (yet) be built bit-identically by make build-x86_64 for the same commit hash & compiler version :(

See:

@michaelsproul michaelsproul added ready-for-review The code is ready for review low-hanging-fruit Easy to resolve, get it before someone else does! v6.0.0 New major release for hierarchical state diffs labels Sep 13, 2024
@michaelsproul
Copy link
Member Author

Related:

@@ -85,6 +87,29 @@ fn build_profile_name() -> String {
.to_string()
}

fn compiler_version() -> String {
let version = rustc_version::version_meta().expect("should be able to fetch compiler version");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will error if the command rustc -vV fails to run.

https://docs.rs/rustc_version/0.4.1/src/rustc_version/lib.rs.html#201
https://docs.rs/rustc_version/0.4.1/src/rustc_version/lib.rs.html#180

Is this run on compilation of on every run of LH?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh shit I think you're right, this is running at runtime 🤦‍♂️

@michaelsproul michaelsproul added waiting-on-author The reviewer has suggested changes and awaits thier implementation. and removed ready-for-review The code is ready for review v6.0.0 New major release for hierarchical state diffs labels Sep 23, 2024
@michaelsproul
Copy link
Member Author

I don't know how to fix this easily, and it's low prio so I'm not going to spend any time on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
low-hanging-fruit Easy to resolve, get it before someone else does! waiting-on-author The reviewer has suggested changes and awaits thier implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants