Skip to content

Add git branch and commit hash to version string#312

Open
nileshnegi wants to merge 7 commits into
candidatefrom
users/nileshnegi/add-version
Open

Add git branch and commit hash to version string#312
nileshnegi wants to merge 7 commits into
candidatefrom
users/nileshnegi/add-version

Conversation

@nileshnegi
Copy link
Copy Markdown
Collaborator

Motivation

"TransferBench v1.67.00" doesn't tell you which commit you're running. When triaging issues or comparing benchmark results across builds, the branch and hash are essential.

Technical Details

Every invocation now prints: TransferBench v1.67.00 (users/foo/my-branch:6f5ea52) ...
Both build paths (Makefile and CMake) are covered.

  • Makefile => $(shell git rev-parse ...) captured into -D macros at build time
  • CMake => execute_process at configure time, passed via target_compile_definitions
  • Source tarball fallback => a committed GIT_VERSION file (line 1: branch, line 2: commit) is read when .git is absent (i.e. release tar/zip). Stamped automatically:
    • CMake ≥ 3.20: via CPACK_PRE_BUILD_SCRIPTS just before CPack collects files
    • CMake < 3.20: at configure time (only when git is present, so a tarball build never overwrites a correctly-stamped file)
  • #ifndef guards in DisplayVersion() provide a last-resort "unknown" fallback if macros are absent

Test Plan

Test Result

Submission Checklist

nileshnegi and others added 4 commits May 22, 2026 23:09
Print git branch and short commit hash alongside the existing version
number whenever any TransferBench command is run, e.g.:
  TransferBench v1.67.00 (users/nileshnegi/add-version:6f5ea52) ...

Both build paths are covered:
- Makefile: shell-captures branch/commit via `git rev-parse` and passes
  them as -D string macros; uses -C <repo-dir> so it works from any
  invocation directory; falls back to "unknown" when git is unavailable.
- CMake: uses execute_process (same pattern as CollectBench) to capture
  branch/commit at configure time, wires them via target_compile_definitions,
  and emits a STATUS message during configure.

Source: #ifndef guards in DisplayVersion() provide a safe fallback if
the macros are somehow absent.

Co-authored-by: Claude <claude@anthropic.com>
When TransferBench is distributed as a source tarball without the .git
directory, git rev-parse fails and the version string would show
"unknown:unknown". This adds a GIT_VERSION file (line 1: branch, line 2:
commit) that both build paths read as a middle fallback:

  git rev-parse  >  GIT_VERSION  >  "unknown"

The file is committed with placeholder "unknown" values. Packaging scripts
should overwrite it with the real branch/hash before creating a tarball:

  printf '%s\n%s\n' "$(git rev-parse --abbrev-ref HEAD)" \
                    "$(git rev-parse --short HEAD)" > GIT_VERSION

CMake: uses file(STRINGS ...) after the git block fails.
Makefile: uses sed -n '1p'/'2p' in the same shell || chain.

Co-authored-by: Claude <claude@anthropic.com>
Previously the packaging workflow doc said to manually overwrite
GIT_VERSION before creating a tarball. This automates it via CMake.

At configure time, configure_file() stamps branch and commit into
cmake/WriteGitVersion.cmake.in -> build/WriteGitVersion.cmake (values
are baked in as string literals, no git dependency at package time).

CPack then runs that script via CPACK_PRE_BUILD_SCRIPTS before collecting
source files, so GIT_VERSION in the tarball always carries the real values
captured at configure time. No manual steps required of the packager.

Requires CMake >= 3.20 for CPACK_PRE_BUILD_SCRIPTS; silently skipped on
older CMake (GIT_VERSION fallback still works, will just show "unknown").

Co-authored-by: Claude <claude@anthropic.com>
CPACK_PRE_BUILD_SCRIPTS requires CMake 3.20+. On older CMake the
packaging block silently skipped stamping, leaving GIT_VERSION as
"unknown" in any distributed source tarball.

Fix: when CMake < 3.20 and git is available, write GIT_VERSION directly
during configure while the branch/hash values are already in scope.
The guard on NOT "unknown" ensures a tarball build (no git) never
overwrites a correctly-stamped file with "unknown" values.

For CMake >= 3.20 the CPACK_PRE_BUILD_SCRIPTS path is unchanged and
stamps the file right before CPack collects source files.

Co-authored-by: Claude <claude@anthropic.com>
Copilot AI review requested due to automatic review settings May 23, 2026 04:35
@nileshnegi nileshnegi requested review from a team as code owners May 23, 2026 04:35
@nileshnegi nileshnegi changed the base branch from develop to candidate May 23, 2026 04:36
@nileshnegi nileshnegi self-assigned this May 23, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances TransferBench’s runtime version banner to include git branch and short commit hash, improving build traceability for benchmarking and triage. It implements this across both Makefile and CMake build paths, with a GIT_VERSION fallback for source archives that do not include a .git directory.

Changes:

  • Update DisplayVersion() to print (<branch>:<hash>) and provide "unknown" macro fallbacks.
  • Add Makefile and CMake logic to capture git branch/hash at build/configure time and compile them into the binary.
  • Add a GIT_VERSION fallback file and a CPack pre-build script to stamp it during packaging.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/client/Client.cpp Prints git branch/hash in the version banner with #ifndef fallbacks.
Makefile Computes git metadata and injects it via -D macros.
GIT_VERSION Adds a committed fallback file for archive builds without .git.
CMakeLists.txt Captures git metadata, falls back to GIT_VERSION, and wires definitions + packaging stamping.
cmake/WriteGitVersion.cmake.in CPack pre-build script to write GIT_VERSION into the packaged source tree.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Makefile Outdated
Comment thread CMakeLists.txt Outdated
Comment thread CMakeLists.txt Outdated
Comment thread CMakeLists.txt Outdated
Comment thread CMakeLists.txt Outdated
Comment thread CMakeLists.txt
Comment thread cmake/WriteGitVersion.cmake.in Outdated
nileshnegi and others added 2 commits May 22, 2026 23:46
Three real issues fixed:

Makefile: git shell calls outside clean guard
  Move the git metadata block inside ifeq($(filter clean,...)) so
  $(shell git rev-parse ...) is not invoked on every `make clean`.

CMakeLists: duplicate find_package and inconsistent variable name
  The second find_package(Git QUIET) was redundant — git was already
  found at line 113 with GIT_FOUND/GIT_EXECUTABLE in scope. Remove the
  duplicate call and change if(Git_FOUND) to if(GIT_FOUND) for
  consistency with the rest of the file.

CMakeLists: list(GET) crash on malformed GIT_VERSION
  list(GET) with an out-of-range index is a hard CMake error. Add a
  list(LENGTH) guard so a truncated or empty GIT_VERSION falls back to
  "unknown" rather than aborting configure. Also extend the fallback
  trigger to check TB_GIT_COMMIT as well as TB_GIT_BRANCH, so a commit
  hash that fails independently does not get baked in as an empty string.

Co-authored-by: Claude <claude@anthropic.com>
The CMake < 3.20 fallback previously wrote GIT_VERSION into the source
tree on every cmake invocation, dirtying the worktree even for normal
non-packaging builds.

Move the file(WRITE) into both packaging branches as an elseif on the
existing >= 3.20 CPACK_PRE_BUILD_SCRIPTS block. GIT_VERSION is now only
stamped when cmake is explicitly configured for packaging
(BUILD_RELOCATABLE_PACKAGE or rocm_create_package paths), matching the
intent of the >= 3.20 path.

Co-authored-by: Claude <claude@anthropic.com>
Copilot AI review requested due to automatic review settings May 23, 2026 04:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Comment thread CMakeLists.txt
Comment thread CMakeLists.txt
Comment thread CMakeLists.txt
Comment thread cmake/WriteGitVersion.cmake.in Outdated
…lock

The version-patch block above already uses CMAKE_CURRENT_SOURCE_DIR for
git operations; align the git metadata block and packaging writes to match.
No behavior change for standalone builds; correct hygiene.

Co-authored-by: Claude <claude@anthropic.com>
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