makefile: extract git commands by Tema · Pull Request #10514 · tikv/pd

Tema · 2026-03-31T00:45:05Z

What problem does this PR solve?

Issue Number: Close #10525

What is changed and how does it work?

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Code changes

Has the configuration change
Has HTTP APIs changed (Don't forget to add the declarative for the new API)
Has persistent data change

Side effects

Possible performance regression
Increased code complexity
Breaking backward compatibility

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
PR to update pingcap/tiup:
Need to cherry-pick to the release branch

Release note

None.

Summary by CodeRabbit

Chores
- Optimized the build process by precomputing VCS commit and branch once and reusing them in build metadata.
- Streamlined dashboard versioning to rely on the configured release version (left unset in CI), improving consistency and build determinism.

ti-chi-bot · 2026-03-31T00:45:16Z

Hi @Tema. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

coderabbitai · 2026-03-31T00:45:25Z

📝 Walkthrough

Walkthrough

The Makefile now computes GITHASH and GITBRANCH once via git rev-parse and references them in LDFLAGS; when DASHBOARD != 0, the dashboard PDVersion linker value uses $(RELEASE_VERSION) instead of an inline git describe invocation.

Changes

Cohort / File(s)	Summary
Build Variable Optimization `Makefile`	Adds `GITHASH := $(shell git rev-parse HEAD)` and `GITBRANCH := $(shell git rev-parse --abbrev-ref HEAD)` and updates `LDFLAGS` to reference these variables. Replaces inline `git describe --tags --dirty --always` with `$(RELEASE_VERSION)` for dashboard `PDVersion` when building the dashboard.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 I hopped through lines of make and cheer,
Cached my hashes, held them near.
Branches tucked in neat and snug,
Builds now hum with lighter mug.
🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description is incomplete. The issue number placeholder 'Close `#xxx`' was not replaced with an actual issue reference, and the commit message body is empty.	Replace 'Close `#xxx`' with the actual issue number (e.g., 'Close `#10525`') and provide a detailed commit message explaining the changes made to the Makefile.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'makefile: extract git commands' is concise and clearly summarizes the main change: extracting git command substitutions into Makefile variables.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Makefile`:
- Around line 70-71: GITHASH and GITBRANCH are being eagerly evaluated with :=
causing git commands to run at Makefile parse time; change their assignment to
conditional/simple assignment (use ?= like RELEASE_VERSION) and provide safe
fallback defaults (e.g., "unknown" or empty) so that git rev-parse is only
executed when requested and not during every make invocation; update the lines
that define GITHASH and GITBRANCH to mirror the RELEASE_VERSION pattern so they
don't fail in non-git or shallow-clone environments.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a5c873d8-fe25-49b4-b25b-406fc26997e9

📥 Commits

Reviewing files that changed from the base of the PR and between 0a0dcfd and 267af44.

📒 Files selected for processing (1)

Makefile

coderabbitai · 2026-03-31T00:48:53Z

Makefile

+GITHASH := $(shell git rev-parse HEAD)
+GITBRANCH := $(shell git rev-parse --abbrev-ref HEAD)


⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash # Verify current assignment flavor and linker-flag usage (read-only). rg -n '^(GITHASH|GITBRANCH)\s*[:?]?=' Makefile -C2 rg -n 'PDGitHash|PDGitBranch' Makefile -C2

Repository: tikv/pd

Length of output: 630

🏁 Script executed:

#!/bin/bash # Check for other assignments to GITHASH or GITBRANCH rg -n 'GITHASH|GITBRANCH' Makefile # Check how other shell-based assignments are handled in Makefile rg -n 'shell.*git|:=.*\$\(shell' Makefile | head -20

Repository: tikv/pd

Length of output: 899

🏁 Script executed:

#!/bin/bash # Check for CI guards or checks for git context rg -n '\.git|GIT_DIR|git.*error|non-git' Makefile -C1 # Check if any targets skip git operations rg -n 'ifeq.*git|ifdef.*GIT' Makefile

Repository: tikv/pd

Length of output: 511

Avoid eager git command execution during Makefile parse.

On Lines 70-71, := runs git rev-parse for every make invocation (including non-build targets), which can fail in non-git contexts (shallow clones, docker builds) and inject empty values. Use ?= with fallbacks to match the pattern already established by RELEASE_VERSION on Line 65.

🔧 Suggested change

-GITHASH := $(shell git rev-parse HEAD) -GITBRANCH := $(shell git rev-parse --abbrev-ref HEAD) +GITHASH ?= $(shell git rev-parse HEAD 2>/dev/null || echo None) +GITBRANCH ?= $(shell git rev-parse --abbrev-ref HEAD 2>/dev/null || echo None)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

GITHASH := $(shell git rev-parse HEAD)

GITBRANCH := $(shell git rev-parse --abbrev-ref HEAD)

GITHASH ?= $(shell git rev-parse HEAD 2>/dev/null || echo None)

GITBRANCH ?= $(shell git rev-parse --abbrev-ref HEAD 2>/dev/null || echo None)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 70 - 71, GITHASH and GITBRANCH are being eagerly evaluated with := causing git commands to run at Makefile parse time; change their assignment to conditional/simple assignment (use ?= like RELEASE_VERSION) and provide safe fallback defaults (e.g., "unknown" or empty) so that git rev-parse is only executed when requested and not during every make invocation; update the lines that define GITHASH and GITBRANCH to mirror the RELEASE_VERSION pattern so they don't fail in non-git or shallow-clone environments.

codecov · 2026-03-31T00:56:02Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.92%. Comparing base (3eb99ae) to head (7e4316e).
⚠️ Report is 9 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10514      +/-   ##
==========================================
+ Coverage   78.88%   78.92%   +0.03%     
==========================================
  Files         530      531       +1     
  Lines       71548    71632      +84     
==========================================
+ Hits        56439    56533      +94     
+ Misses      11092    11071      -21     
- Partials     4017     4028      +11

Flag	Coverage Δ
unittests	`78.92% <ø> (+0.03%)`	⬆️

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: artem_danilov <artem.danilov@airbnb.com>

bufferflies · 2026-03-31T01:55:11Z

What's the difference?

~/Documents/soft/golang/pd/bin/pd-server -V
Release Version: v9.0.0-beta.2.pre-328-g5ef55c918
Edition: Community
Kernel Type: Classic
Git Commit Hash: 5ef55c918795292e83aea368b1c0febdc7d2d965
Git Branch: bugfix/create_keyspace_atomic
UTC Build Time:  2026-03-26 02:12:06

Tema · 2026-03-31T15:32:49Z

What's the difference?

@bufferflies no-op for you, but our internal infra now can pass GITHASH and GITBRANCH same way as RELEASE_VERSION. This allows our CI to work with shallow git layers. Please treat it as an enhancement for periphery infra

I have the same for TiDB: pingcap/tidb#67437

Tema · 2026-03-31T15:33:02Z

/retest

ti-chi-bot · 2026-03-31T15:33:19Z

@Tema: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

Details

In response to this:

/retest

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

rleungx · 2026-04-01T06:08:56Z

/ok-to-test

rleungx · 2026-04-01T06:09:11Z

/retest

ti-chi-bot · 2026-04-01T07:10:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: okJiang

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [okJiang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2026-04-01T07:10:59Z

[LGTM Timeline notifier]

Timeline:

2026-04-01 07:10:58.856635554 +0000 UTC m=+335464.061995611: ☑️ agreed by okJiang.

Tema · 2026-04-01T17:17:58Z

/retest-required

Tema · 2026-04-01T17:31:10Z

/retest-required

Tema · 2026-04-01T21:53:36Z

@rleungx tests are passed. PTAL

rleungx · 2026-04-02T05:20:52Z

Makefile

 	LDFLAGS += -X "github.com/pingcap/tidb-dashboard/pkg/utils/version.InternalVersion=$(shell scripts/describe-dashboard.sh internal-version)"
 	LDFLAGS += -X "github.com/pingcap/tidb-dashboard/pkg/utils/version.Standalone=No"
-	LDFLAGS += -X "github.com/pingcap/tidb-dashboard/pkg/utils/version.PDVersion=$(shell git describe --tags --dirty --always)"
+	LDFLAGS += -X "github.com/pingcap/tidb-dashboard/pkg/utils/version.PDVersion=$(RELEASE_VERSION)"


If RUN_CI is equal to 1, will the PDVersion turn into None?

ti-chi-bot bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Mar 31, 2026

coderabbitai bot reviewed Mar 31, 2026

View reviewed changes

Tema mentioned this pull request Mar 31, 2026

makefile: extract git commands pingcap/tidb-dashboard#1880

Open

makefile: extract git commands

7e4316e

Signed-off-by: artem_danilov <artem.danilov@airbnb.com>

Tema force-pushed the patch-1 branch from 267af44 to 7e4316e Compare March 31, 2026 00:59

ti-chi-bot bot added dco-signoff: yes Indicates the PR's author has signed the dco. and removed dco-signoff: no Indicates the PR's author has not signed dco. labels Mar 31, 2026

ti-chi-bot bot removed the do-not-merge/needs-linked-issue label Mar 31, 2026

ti-chi-bot bot added ok-to-test Indicates a PR is ready to be tested. and removed needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Apr 1, 2026

okJiang approved these changes Apr 1, 2026

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Apr 1, 2026

ti-chi-bot bot added the approved label Apr 1, 2026

rleungx reviewed Apr 2, 2026

View reviewed changes

		GITHASH := $(shell git rev-parse HEAD)
		GITBRANCH := $(shell git rev-parse --abbrev-ref HEAD)

Conversation

Tema commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

Summary by CodeRabbit

Uh oh!

ti-chi-bot bot commented Mar 31, 2026

Uh oh!

coderabbitai bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bufferflies commented Mar 31, 2026

Uh oh!

Tema commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Tema commented Mar 31, 2026

Uh oh!

ti-chi-bot bot commented Mar 31, 2026

Uh oh!

rleungx commented Apr 1, 2026

Uh oh!

rleungx commented Apr 1, 2026

Uh oh!

ti-chi-bot bot commented Apr 1, 2026

Uh oh!

ti-chi-bot bot commented Apr 1, 2026

[LGTM Timeline notifier]

Uh oh!

Tema commented Apr 1, 2026

Uh oh!

Tema commented Apr 1, 2026

Uh oh!

Tema commented Apr 1, 2026

Uh oh!

rleungx Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Tema commented Mar 31, 2026 •

edited

Loading

coderabbitai bot commented Mar 31, 2026 •

edited

Loading

codecov bot commented Mar 31, 2026 •

edited

Loading

Tema commented Mar 31, 2026 •

edited

Loading