feat(cli): add commit hash and target to long_version output #8133
Conversation
cli/version.rs
Outdated
| @@ -1,6 +1,7 @@ | |||
| // Copyright 2018-2020 the Deno authors. All rights reserved. MIT license. | |||
|
|
|||
| pub const DENO: &str = env!("CARGO_PKG_VERSION"); | |||
| pub const BUILD: &str = env!("GIT_COMMIT_HASH"); | |||
There was a problem hiding this comment.
Just "GIT_COMMIT_HASH" instead of "BUILD" would be more descriptive.
cli/build.rs
Outdated
| fn git_commit_hash() -> String { | ||
| let output = std::process::Command::new("git") | ||
| .arg("rev-list") | ||
| .arg("-1") | ||
| .arg("HEAD") | ||
| .output() | ||
| .expect("failed to execute process"); | ||
| std::str::from_utf8(&output.stdout[..7]) | ||
| .unwrap() | ||
| .to_string() | ||
| } | ||
|
|
There was a problem hiding this comment.
Is this going to work when doing cargo install deno?
There was a problem hiding this comment.
It's a good point. I think it will break "cargo install deno".
There was a problem hiding this comment.
I see. It won't work with cargo install...
We need a different strategy here.
There was a problem hiding this comment.
It seems sha1 is stored in the file called .cargo_vcs_info.json in crates.io. (ref: rust-lang/cargo#5629 ) Maybe we can use it.
There was a problem hiding this comment.
Maybe just return "crates.io" if anything in git_commit_hash fails?
|
Closing for the moment. I'll reopen it when I find a way to handle |
|
@kt3k I think this is a valid approach. As @lucacasonato suggested we can fallback to some constant value eg. empty string. For reference |
|
OK. I'll update the change! |
|
I updated the change. Now the commit hash becomes $ ./target/debug/deno --version
deno 1.5.0 (UNKNOWN, debug, x86_64-apple-darwin)
v8 8.7.220.3
typescript 4.0.3Checked on windows as well just in case. |
|
The solution looks satisfactory to me. Waiting on @ry to double check before landing. |
This PR adds the git commit hash and build target to long version output (
--version) as suggested in #6037. The output now looks like the below:fixes #6037