Skip to content

fix: report the real version and commit in the binary - #436

Merged
shiva-istari merged 1 commit into
mainfrom
fix/binary-version-reporting
Oct 8, 2026
Merged

shiva-istari merged 1 commit into
mainfrom
fix/binary-version-reporting

Conversation

@shiva-istari

@shiva-istari shiva-istari commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

dgraph/ratel:v25.2.0 says "Ratel Version: 21.03.0" with an empty commit ID, and so does every image before it. The Docker tag has always been right; only the binary's own answer was wrong, which makes it useless for working out what someone is running.

Three separate causes.

The Dockerfile ran build.prod.sh without --version, so the build fell back to client/package.json, which has read 21.03.0 since 2021. The Makefile already derives BUILD_VERSION from git describe for the image tag, so it now passes the same value through --build-arg. A bare docker build with no argument reports "dev" rather than a six-year-old number.

buildServer declared commitID and commitINFO locally. The comment said they were "assigned in build script", and they are — as globals in build.prod.sh — but declare inside a function creates an empty local that shadows the global for the rest of the call, so both were compiled in blank. Removing the two declarations lets the caller's values through. go_bindata stays: it is only read by an error message in the same function.

With commitINFO no longer empty, a third fault became visible: the build substitutes a sentinel for every space, because -ldflags splits its value on whitespace, and nothing reversed it. The commit info printed as "eb93c19¨•¨¨•¨Thu¨•¨Oct". It is now undone at print time.

Verified by building the image through make, as the CD workflow does:

before Ratel Version: 21.03.0
Commit ID:
after Ratel Version: v25.3.0
Commit ID: eb93c19
Commit Info: eb93c19 Thu Oct 1 16:01:35 2026 +0530 (tag: v25.3.0)

The image still serves HTTP 200, and the Help sidebar — which gets its values from webpack at client build time and was already correct — is unaffected.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Build & Versioning
    • Server builds now include the configured build version, defaulting to dev when none is provided.
  • Bug Fixes
    • Commit information in the server’s version output now displays with readable spacing instead of separator characters.

dgraph/ratel:v25.2.0 says "Ratel Version: 21.03.0" with an empty commit
ID, and so does every image before it. The Docker tag has always been
right; only the binary's own answer was wrong, which makes it useless for
working out what someone is running.

Three separate causes.

The Dockerfile ran build.prod.sh without --version, so the build fell back
to client/package.json, which has read 21.03.0 since 2021. The Makefile
already derives BUILD_VERSION from git describe for the image tag, so it
now passes the same value through --build-arg. A bare docker build with no
argument reports "dev" rather than a six-year-old number.

buildServer declared commitID and commitINFO locally. The comment said
they were "assigned in build script", and they are — as globals in
build.prod.sh — but `declare` inside a function creates an empty local that
shadows the global for the rest of the call, so both were compiled in
blank. Removing the two declarations lets the caller's values through.
go_bindata stays: it is only read by an error message in the same function.

With commitINFO no longer empty, a third fault became visible: the build
substitutes a sentinel for every space, because -ldflags splits its value
on whitespace, and nothing reversed it. The commit info printed as
"eb93c19¨•¨¨•¨Thu¨•¨Oct". It is now undone at print time.

Verified by building the image through make, as the CD workflow does:

  before   Ratel Version: 21.03.0
           Commit ID:
  after    Ratel Version: v25.3.0
           Commit ID: eb93c19
           Commit Info: eb93c19  Thu Oct 1 16:01:35 2026 +0530  (tag: v25.3.0)

The image still serves HTTP 200, and the Help sidebar — which gets its
values from webpack at client build time and was already correct — is
unaffected.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@shiva-istari
shiva-istari requested a review from a team as a code owner October 8, 2026 06:38
@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d0cf8d38-7676-4ad1-b2ef-14f1111bfce2
📥 Commits

Reviewing files that changed from the base of the PR and between eb93c19 and 391203f.

📒 Files selected for processing (5)
  • Dockerfile
  • Makefile
  • scripts/functions.sh
  • server/commitinfo_test.go
  • server/server.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

The build now passes BUILD_VERSION to the server build script. The server replaces a sentinel in commit information with spaces before printing it. Tests cover sentinel replacement and unchanged inputs.

Changes

Server version information

Layer / File(s) Summary
Pass version information to the server build
Dockerfile, Makefile, scripts/functions.sh
The Docker build accepts BUILD_VERSION, defaulting to dev, and passes it to the server build script. buildServer leaves caller-provided commitID and commitINFO available for linker-flag construction.
Format commit information in version output
server/server.go, server/commitinfo_test.go
The version output replaces each ldflagsSpace sentinel in commitINFO with a space. Tests cover replacement and inputs that remain unchanged.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: matthewmcneely

Merge Risk: ⚪ Minimal · up to 39120

The build uses its Git-derived version, and commit information is rendered readably. No actionable merge-blocking risk remains in the reviewed changes.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: correcting the version and commit information reported by the binary.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 3 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@shiva-istari
shiva-istari merged commit 9728662 into main Oct 8, 2026
11 checks passed
@shiva-istari
shiva-istari deleted the fix/binary-version-reporting branch October 8, 2026 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant