make crates.io version not depend on the git tool
#259
Conversation
also error if git version is used but `git` is not installed prompt the user to install `git` in that case fixes #256
| String::from_utf8(output.stdout).unwrap() | ||
| let hash = Command::new("git") | ||
| .args(&["rev-parse", "HEAD"]) | ||
| .output() |
There was a problem hiding this comment.
The output() call itself will fail if the program cannot be found, so I don't think the fix can use .map
There was a problem hiding this comment.
the original was not checking the .git path if git was not installed but the new version should
| } else { | ||
| assert!(!Path::new(".git").exists(), "you need to install the `git` command line tool to install the git version of `probe-run`"); |
There was a problem hiding this comment.
Did you test that this is the right path? I'm not sure in which working directory build scripts are executed. If it's the Cargo workspace root this seems correct, but it seems like it might be the package root, which means that this would be wrong for the decoder.
There was a problem hiding this comment.
yes, there's an old commit that removes a panic!("{:?}, env::current_dir()) statement from the build script. I used to inspect the directory from which the build script was invoked. It was the package root and not the workspace root.
I guess I could also use CARGO_MANIFEST_DIR (was that the path to the directory that contains the Cargo.toml file?) instead of relying on current_dir. Perhaps that'd be more reliable?
There was a problem hiding this comment.
Either way seems fine to me
also error if git version is used but
gitis not installedprompt the user to install
gitin that casefixes #256
we need
gitwhen the git version is used because we use the commit hash to check if probe-run is compatible with the firmware-side defmtinstead of panicking when git is not installed we could:
git rev-parse HEADusing just the std library. It seems reading the.git/HEADfile and then reading the file specified there (e.g..git/refs/heads/main, which contains the full hash) should do the trick but it does feel a bit brittle