Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add commit hash to version info, if present #957

Merged
merged 3 commits into from
Nov 14, 2021
Merged

Add commit hash to version info, if present #957

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2f2c0d4
Add commit hash to version info, if present
AloeareV Nov 2, 2021
577009a
Rename GIT_HASH to indicate that it includes version, fix linter error
AloeareV Nov 2, 2021
a690aac
Add whitespace after use statement
AloeareV Nov 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions helix-term/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use std::process::Command;
fn main() {
AloeareV marked this conversation as resolved.
Show resolved Hide resolved
let git_hash = Command::new("git")
.args(&["describe", "--dirty"])
Copy link
Member

Choose a reason for hiding this comment

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

I think we also want to use "rev-parse", "--short", "HEAD" so we only get the SHA

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I thought @pickfire 's comment here:

#922 (comment)

made sense. It seems like dirty-ness and distance from release could be really useful info. Is there a constraint somewhere else that makes such a string difficult to work with?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had that for my first draft, and then changed to describe --dirty after reading @pickfire's comment on the issue. Would be no problem to switch back, if we're not concerned about version including knowledge of dirty state.

Copy link
Member

Choose a reason for hiding this comment

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

I guess it's okay, but describe always uses the latest available tag. We'll have to be careful because if the code is built after a new tag is accidentally added then the version info will be wrong.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be worthwhile to use a specific process/tool to enforce that the version can't use any tag except the actual release tag?

Copy link
Member

Choose a reason for hiding this comment

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

So this actually happened in the latest release (it builds as v0.5 on latest master because the release points to a commit before I rebased against master). I'd still rather see we only pull the SHA and use the crate version.

.output()
.map(|x| String::from_utf8(x.stdout).ok())
.ok()
.flatten()
.unwrap_or(String::from(env!("CARGO_PKG_VERSION")));
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.map(|x| String::from_utf8(x.stdout).ok())
.ok()
.flatten()
.unwrap_or(String::from(env!("CARGO_PKG_VERSION")));
.and_then(|output| String::from_utf8((output.stdout))
.unwrap_or_default();

println!("cargo:rustc-env=GIT_HASH={}", git_hash);
}
4 changes: 2 additions & 2 deletions helix-term/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ FLAGS:
-V, --version Prints version information
",
env!("CARGO_PKG_NAME"),
env!("CARGO_PKG_VERSION"),
env!("GIT_HASH"),
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather see us use version-hash in both places, i.e. v0.4.0-abcdef

env!("CARGO_PKG_AUTHORS"),
env!("CARGO_PKG_DESCRIPTION"),
logpath.display(),
Expand All @@ -81,7 +81,7 @@ FLAGS:
}

if args.display_version {
println!("helix {}", env!("CARGO_PKG_VERSION"));
println!("helix {}", env!("GIT_HASH"));
std::process::exit(0);
}

Expand Down