Skip to content

Add Git information to cargo-gpu's --version CLI subcommand#623

Merged
Firestar99 merged 3 commits into
Rust-GPU:mainfrom
Steveplays28:feat/add-git-info-to-cargo-gpu-cli-version-subcommand
Jul 16, 2026
Merged

Add Git information to cargo-gpu's --version CLI subcommand#623
Firestar99 merged 3 commits into
Rust-GPU:mainfrom
Steveplays28:feat/add-git-info-to-cargo-gpu-cli-version-subcommand

Conversation

@Steveplays28

@Steveplays28 Steveplays28 commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Old:

$ cargo-gpu --version
cargo-gpu 0.10.0-alpha.1

New:

$ cargo-gpu --version
cargo-gpu 0.10.0-alpha.1 installed from crates.io (rev d95ec972)

$ cargo-gpu --version
cargo-gpu 0.10.0-alpha.1 installed from repo at rev d95ec972 (2026-07-16)

$ cargo-gpu --version
cargo-gpu 0.10.0-alpha.1 installed from unknown source

@nazar-pc nazar-pc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is just my opinion, but it seems really wasteful to add such a massive dependency tree for such a tiny feature.

You essentially added more than 1000 lines of dependencies just to obtain VERGEN_GIT_COMMIT_DATE environment variable, which doesn't make sense to me. Everything else was already there.

Adding commit hash is a single line change instead of current +1346 -80, adding date is a bit more than that, but still less than what this PR does.

@Steveplays28

Steveplays28 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

@nazar-pc well okay, fair.
I'll keep the Git CLI invocation that was there before

@Steveplays28 Steveplays28 marked this pull request as draft June 29, 2026 06:36
@Steveplays28 Steveplays28 force-pushed the feat/add-git-info-to-cargo-gpu-cli-version-subcommand branch 2 times, most recently from 85f9e81 to 9ffe19b Compare June 29, 2026 07:05
@Steveplays28 Steveplays28 marked this pull request as ready for review June 29, 2026 07:06
@Steveplays28

Steveplays28 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor Author

I added a helper function for invoking Git, added cargo:rerun-if-changed lines and got the date through the Git CLI. Thanks for checking the PR, Nazar.

My bad for not doing it in this simple way in the first place, I agree that this is better. Less time needed to compile and less code to maintain

@nazar-pc nazar-pc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks much better, thanks 🙂

Comment thread crates/cargo-gpu/build.rs Outdated
@Firestar99 Firestar99 force-pushed the feat/add-git-info-to-cargo-gpu-cli-version-subcommand branch from 9ffe19b to 959c711 Compare July 2, 2026 12:37

@Firestar99 Firestar99 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not yet sure if this is a net positive, would love input from others.

Some unordered thoughts:

  • I didn't notice cargo gpu on main already stored the commit rev, see cargo gpu show commitsh. Not very discoverable, definitely prefer putting it into the version.
  • a cargo package installed from crates.io won't have a git repo, so the git rev will be "unknown"
    • cargo nextest --version also prints the git rev, but also suffers from the same issue when installed via cargo install
    • there doesn't seem to be a nice universal way to handle this
  • setting a GIT_HASH env var will force a larger recompile on each rev change
  • cargo-deny ci is broken, see #624

@Firestar99 Firestar99 force-pushed the feat/add-git-info-to-cargo-gpu-cli-version-subcommand branch from 959c711 to ae41cbe Compare July 2, 2026 12:42
@nazar-pc

nazar-pc commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

My assumption is that this is primarily useful for identifying what version is used when reporting bugs. Versions published on crates.io are well known, while those installed from git may have the same version, but correspond to any revision.

So it is fine to have unknown version for crates.io (I'd probably not set revision at all rather than setting to unknown though) and actual hash when installed from git. And for the same reason it is fine to not have anything when installed from local path since it is something developer will hopefully specify explicitly (they're probably testing some local patches in that case).

@Firestar99 Firestar99 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'll merge it once the comment is addressed.

I wouldn't go to the length of writing the git rev to a file on release for cargo install from crates.io to include it, seems like a lot of effort without much gain. nextest actually implements such a system, yet I don't see the git rev so I assume it broke at some point and noone noticed or cared to fix it.

Comment thread crates/cargo-gpu/build.rs Outdated
@Steveplays28 Steveplays28 force-pushed the feat/add-git-info-to-cargo-gpu-cli-version-subcommand branch from 350bddf to 4b06627 Compare July 9, 2026 14:33
@Steveplays28

Steveplays28 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

My assumption is that this is primarily useful for identifying what version is used when reporting bugs. Versions published on crates.io are well known, while those installed from git may have the same version, but correspond to any revision.

yeah, I recently brought up an issue on the Discord server that was just due to an out-of-date cargo-gpu. I thought I'd updated it, but I hadn't

So it is fine to have unknown version for crates.io (I'd probably not set revision at all rather than setting to unknown though) and actual hash when installed from git. And for the same reason it is fine to not have anything when installed from local path since it is something developer will hopefully specify explicitly (they're probably testing some local patches in that case).

I think I agree, maybe the unknown commit hash and date shouldn't be set/shown when cargo-gpu is installed from crates.io
I could change that if you want?

Added the commit hash and date to the `--version` CLI subcommand.
@Steveplays28 Steveplays28 force-pushed the feat/add-git-info-to-cargo-gpu-cli-version-subcommand branch from 4b06627 to 44e63a3 Compare July 9, 2026 15:04
@Firestar99

Firestar99 commented Jul 13, 2026

Copy link
Copy Markdown
Member

I gave it one more pass made it print this:

$ cargo install --path .
$ cargo gpu -V
cargo-gpu 0.10.0-alpha.1 (42a53815 2026-07-13)
$ cargo install cargo-gpu@...
$ cargo gpu -V
cargo-gpu 0.10.0-alpha.1 (installed from crates.io)

Changes:

  • always print rev, don't require long --version
  • short rev
  • explicitly state when installed from crates.io

Thoughts? (a 👍 if you're happy is fine)

I would also like to make a MSRV-like system but for cargo-gpu, to notify you that you need to upgrade cargo-gpu before proceeding with the build and failing. (In another PR, not here)

@Firestar99 Firestar99 force-pushed the feat/add-git-info-to-cargo-gpu-cli-version-subcommand branch from 42a5381 to dc947ff Compare July 13, 2026 15:01
@Steveplays28

Copy link
Copy Markdown
Contributor Author

looks good @Firestar99, thanks

also that cargo-gpu minimum version system would be great

@Firestar99

Copy link
Copy Markdown
Member

Small detail I just noticed: When cargo package creates a package, it adds a file at ./.cargo_vcs_info.json with the following contents:

{
  "git": {
    "sha1": "2dd42e970faff3e6f8b1c8c4e8d2daf930c5dd04"
  },
  "path_in_vcs": "crates/cargo-gpu"
}

We could read the rev from there, without needing some special release script...

@Firestar99 Firestar99 force-pushed the feat/add-git-info-to-cargo-gpu-cli-version-subcommand branch from dc947ff to d95ec97 Compare July 16, 2026 14:34
@Firestar99

Copy link
Copy Markdown
Member

Made one more iteration:

cargo-gpu 0.10.0-alpha.1 installed from crates.io (rev d95ec972)
cargo-gpu 0.10.0-alpha.1 installed from repo at rev d95ec972 (2026-07-16)
cargo-gpu 0.10.0-alpha.1 installed from unknown source

@Firestar99 Firestar99 force-pushed the feat/add-git-info-to-cargo-gpu-cli-version-subcommand branch from d95ec97 to 748b4ab Compare July 16, 2026 14:39
@Firestar99 Firestar99 enabled auto-merge July 16, 2026 14:41
@Firestar99 Firestar99 added this pull request to the merge queue Jul 16, 2026
Merged via the queue into Rust-GPU:main with commit c2de013 Jul 16, 2026
23 checks passed
@Steveplays28

Copy link
Copy Markdown
Contributor Author

thanks @Firestar99!

@Steveplays28 Steveplays28 deleted the feat/add-git-info-to-cargo-gpu-cli-version-subcommand branch July 16, 2026 15:46
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.

4 participants