Skip to content

Fix Git version detection when embedded - #551

Merged
Sapd merged 2 commits into
Sapd:masterfrom
ChrisLauinger77:codex/fix-embedded-git-version
Jul 30, 2026
Merged

Fix Git version detection when embedded#551
Sapd merged 2 commits into
Sapd:masterfrom
ChrisLauinger77:codex/fix-embedded-git-version

Conversation

@ChrisLauinger77

Copy link
Copy Markdown
Contributor

What changed

  • Run git describe from the HeadsetControl source directory.
  • Run the git rev-parse fallback from the same directory.

Why

When HeadsetControl is included by another project through add_subdirectory(), the Git commands could execute in the parent repository context. This caused the generated HeadsetControl version to contain the parent project's commit instead of the HeadsetControl revision.

Impact

Embedded consumers now receive a version derived from the HeadsetControl repository, while standalone builds keep the existing behavior.

Validation

  • git diff --check
  • Standalone CMake configure
  • CMake configure through a parent project using add_subdirectory()
  • Confirmed embedded version output: 4.0.0-10-gf0e2921

@ChrisLauinger77
ChrisLauinger77 marked this pull request as ready for review July 30, 2026 09:24

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f0e29217d0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread CMakeLists.txt
@ChrisLauinger77

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: 8c6530c65d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@Sapd
Sapd merged commit 2370aa7 into Sapd:master Jul 30, 2026
5 checks passed
@ChrisLauinger77
ChrisLauinger77 deleted the codex/fix-embedded-git-version branch July 30, 2026 11:41
Sapd added a commit that referenced this pull request Jul 30, 2026
The Windows build has been reporting 0.0.0-unknown, for two independent
reasons. Both are verified on a real Windows 11 + MSYS2 MINGW64 box.

1. The MSYS2 shell has no git at all. It is not part of base-devel, and
   the shell does not inherit Git for Windows from the runner PATH
   (confirmed: no "/Program Files/Git" entry in the MINGW64 PATH). So
   every git call in CMakeLists.txt failed. Fixed by installing the git
   package alongside the toolchain.

2. Once git is available, the --show-toplevel guard added in #551 would
   still reject every Windows build, permanently pinning it to
   0.0.0-unknown. MSYS2 git reports POSIX paths while CMake reports
   Windows ones, and get_filename_component(REALPATH) does not reconcile
   them:

     PROJECT_SOURCE_DIR  = C:/Users/Denis/hc-vtest/probe/sub
     git --show-toplevel = /c/Users/Denis/hc-vtest/probe/sub

   so the STREQUAL can never match. Replaced with --show-prefix, which is
   empty exactly when the source dir is the root of a work tree. That is a
   plain string test needing no path normalisation, and it drops both
   REALPATH calls.

Measured on Windows with the same package set as CI: the old guard yields
0.0.0-unknown, the new one yields 4.0.0-11-g00ca695, a vendored copy
correctly falls back to 0.0.0-unknown, and build + ctest pass.

Both reject paths now explain themselves and point at
-DHEADSETCONTROL_VERSION. Also renames the reused GIT_RESULT to
GIT_DESCRIBE_RESULT.
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.

2 participants