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

Check exit status of git commands spawned by build script #6266

Merged
merged 2 commits into from
Aug 4, 2024

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Aug 4, 2024

The git commands git rev-parse HEAD and git log -1 --date=short --pretty=format:%cd in rustfmt's build script might fail with "fatal: not a git repository (or any of the parent directories): .git" if rustfmt is being built from a source tarball rather than a git repository. That message is written by git to stderr, and nothing is written to stdout.

Previously, rustfmt's build script would take that empty stdout and treat it like valid commit info, unlike what it does when git cannot be spawned in the first place, and contradicting this comment about the build script's intended behavior in the case of there not being a git repository:

rustfmt/build.rs

Lines 24 to 25 in 17c5869

// Try to get hash and date of the last commit on a best effort basis. If anything goes wrong
// (git not installed or if this is not a git repository) just return an empty string.

$ curl -L https:/rust-lang/rustfmt/archive/refs/heads/master.tar.gz | tar xz
$ cd rustfmt-master/
$ cargo run --bin rustfmt -- --version

Before this PR: rustfmt 1.7.1-nightly ( )
After first commit: rustfmt 1.7.1-
After second commit: rustfmt 1.7.1

Copy link
Contributor

@ytmimi ytmimi left a comment

Choose a reason for hiding this comment

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

Thanks! This definitely feels like the right output in case we can't grab additional info from git.

@ytmimi ytmimi merged commit 8894466 into rust-lang:master Aug 4, 2024
26 checks passed
@dtolnay dtolnay deleted the commitinfo branch August 4, 2024 18:52
@ytmimi ytmimi added release-notes Needs an associated changelog entry and removed pr-not-reviewed labels Aug 4, 2024
@ytmimi ytmimi removed the release-notes Needs an associated changelog entry label Sep 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants