Skip to content

Add GIT_COMMIT cmake variable#488

Merged
Anilm3 merged 2 commits intomasterfrom
glopes/git-commit
Jan 27, 2026
Merged

Add GIT_COMMIT cmake variable#488
Anilm3 merged 2 commits intomasterfrom
glopes/git-commit

Conversation

@cataphract
Copy link
Contributor

Running git is not always possible, either because there's no git checkout, there's no git available, or we're in a git subtree without access to reference git files.

Allow passing GIT_COMMIT to cmake to bypass the git invocation.

Running git is not always possible, either because there's no git
checkout, there's no git available, or we're in a git subtree without
access to reference git files.

Allow passing GIT_COMMIT to cmake to bypass the git invocation.
@cataphract cataphract requested a review from a team as a code owner January 27, 2026 11:46
Copy link

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

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: 51e5c95792

ℹ️ 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 on lines +25 to +28
if(GIT_COMMIT)
set(DDWAF_VERSION ${CMAKE_PROJECT_VERSION})
execute_process(COMMAND git rev-parse --short HEAD
string(SUBSTRING "${GIT_COMMIT}" 0 7 SHORT_BUILD_ID)
else()

Choose a reason for hiding this comment

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

P2 Badge Preserve tag-based package naming when GIT_COMMIT set

When GIT_COMMIT is provided, the code skips the tag check and always derives DDWAF_VERSION from CMAKE_PROJECT_VERSION while also unconditionally setting SHORT_BUILD_ID, which means CPACK_PACKAGE_FILE_NAME will always get a -<short> suffix even for tagged releases. In environments without a git checkout (the stated use case), this changes the filename format compared to the tagged build path (which previously produced no suffix), so release pipelines or consumers expecting tag-named artifacts can break when they pass GIT_COMMIT (e.g., building from a release tarball). Consider mirroring the tag behavior when GIT_COMMIT is set or making the suffix conditional.

Useful? React with 👍 / 👎.

@codecov-commenter
Copy link

codecov-commenter commented Jan 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.92%. Comparing base (cf49c49) to head (e7a28f1).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #488   +/-   ##
=======================================
  Coverage   84.92%   84.92%           
=======================================
  Files         190      190           
  Lines        9203     9203           
  Branches     4143     4143           
=======================================
  Hits         7816     7816           
  Misses        509      509           
  Partials      878      878           
Flag Coverage Δ
waf_test 84.92% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link

github-actions bot commented Jan 27, 2026

Dynamic Artifact Size Comparison 📦

Artifact Previous Release This PR Difference
darwin-arm64::libddwaf.dylib 2004032 2004032 0.00%
darwin-universal::libddwaf.dylib 4215872 4215872 0.00%
darwin-x86_64::libddwaf.dylib 2195712 2195712 0.00%
linux-aarch64::libddwaf.so 2378224 2378224 0.00%
linux-armv7::libddwaf.so 2046280 2046280 0.00%
linux-i386::libddwaf.so 2300828 2300828 0.00%
linux-x86_64::libddwaf.so 2571120 2571120 0.00%
windows-arm64::ddwaf.dll 6686208 6686208 0.00%
windows-win32::ddwaf.dll 3315712 3315712 0.00%
windows-x64::ddwaf.dll 4045824 4045824 0.00%

Static Artifact Size Comparison 📦

Artifact Previous Release This PR Difference
darwin-arm64::libddwaf.a 92215128 92215128 0.00%
darwin-arm64::libddwaf.a.stripped 4707464 4707464 0.00%
darwin-universal::libddwaf.a 185945888 185945888 0.00%
darwin-universal::libddwaf.a.stripped 10023864 10023864 0.00%
darwin-x86_64::libddwaf.a 93730712 93730712 0.00%
darwin-x86_64::libddwaf.a.stripped 5316352 5316352 0.00%
linux-aarch64::libddwaf.a 75161592 75161592 0.00%
linux-aarch64::libddwaf.a.stripped 12162860 12162860 0.00%
linux-armv7::libddwaf.a 66399138 66399138 0.00%
linux-armv7::libddwaf.a.stripped 11168358 11168358 0.00%
linux-i386::libddwaf.a 64591428 64591428 0.00%
linux-i386::libddwaf.a.stripped 9661240 9661240 0.00%
linux-x86_64::libddwaf.a 75652672 75652672 0.00%
linux-x86_64::libddwaf.a.stripped 11966124 11966124 0.00%
windows-arm64::ddwaf.lib 16410 16410 0.00%
windows-arm64::ddwaf_static.lib 134528158 134528158 0.00%
windows-win32::ddwaf.lib 16726 16726 0.00%
windows-win32::ddwaf_static.lib 47564214 47564214 0.00%
windows-x64::ddwaf.lib 16410 16410 0.00%
windows-x64::ddwaf_static.lib 55369970 55369970 0.00%

@pr-commenter
Copy link

pr-commenter bot commented Jan 27, 2026

Benchmarks clang-pgo

Benchmark execution time: 2026-01-27 14:29:38

Comparing candidate commit e7a28f1 in PR branch glopes/git-commit with baseline commit cf49c49 in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics.

@Anilm3 Anilm3 merged commit 18eae43 into master Jan 27, 2026
57 checks passed
@Anilm3 Anilm3 deleted the glopes/git-commit branch January 27, 2026 15:28
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

Comments