feat: version string from git describe + fork banner#21
Open
feat: version string from git describe + fork banner#21
Conversation
- Use OLR_VERSION from `git describe --tags --always` for both the CLI version string and Docker image tag - Replace MAJOR.MINOR.PATCH assembly with single OLR_VERSION string - Add fork attribution banner in welcome message - Add build-release Makefile target - Fix missing ca-certificates in Dockerfile.release builder
📝 WalkthroughWalkthroughDerive OLR_VERSION from Git tags at configure time and propagate it through CMake, Docker builds, Make targets, and runtime banners; replace numeric major/minor/patch macros with a single string Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as "Developer / CI (git)"
participant Make as "Make / docker buildx"
participant Docker as "Docker build (Dockerfile)"
participant CMake as "CMake configure"
participant Build as "Compiler / Build"
participant App as "Runtime (binary)"
Dev->>Make: run build / build-release (OLR_VERSION=$(git describe))
Note right of Make: --build-arg OLR_VERSION
Make->>Docker: docker buildx build (with OLR_VERSION)
Docker->>CMake: cmake ... -DOLR_VERSION=<value> (conditional)
CMake->>Build: configure files (populate config.h with OLR_VERSION)
Build->>App: binary contains OpenLogReplicator_OLR_VERSION
App->>Developer: startup banner prints OLR_VERSION
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Makefile (1)
4-4: Consider avoiding hardcodedunknownforOLR_VERSIONpassed to CMake.At Line 4, fallbacking to
unknownmeans Docker builds always pass a non-empty version (Line 32), which bypasses CMake’s fallback toPROJECT_VERSION. Prefer keepingOLR_VERSIONempty on git failure and using a separate tag fallback for image naming (Line 39).Suggested refactor
-OLR_VERSION ?= $(shell git describe --tags --always --dirty 2>/dev/null || echo unknown) +OLR_VERSION ?= $(shell git describe --tags --always --dirty 2>/dev/null) +OLR_IMAGE_TAG ?= $(if $(OLR_VERSION),$(OLR_VERSION),unknown) @@ - -t rophy/openlogreplicator:$(OLR_VERSION) \ + -t rophy/openlogreplicator:$(OLR_IMAGE_TAG) \Also applies to: 32-32, 39-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 4, Change the OLR_VERSION assignment so it does not default to the literal "unknown" when git describe fails (e.g., make OLR_VERSION empty on failure) so CMake can fall back to PROJECT_VERSION; then introduce or use a separate Docker image tag fallback for image naming (the tag used on the docker build/push step referenced around line 39) so image naming still gets a non-empty tag when needed. Specifically, update the OLR_VERSION variable assignment (the OLR_VERSION ?= ... shell invocation) to return an empty string on git failure, and update the docker tag logic (the variable used at the docker build/push step) to explicitly fallback to a tag value when OLR_VERSION is empty.
🤖 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 33-34: The build argument for GIDOLR is incorrectly sourced from
the user ID; update the Makefile so GIDOLR uses the group ID command instead of
the user ID. Specifically, in the build-release invocation where --build-arg
GIDOLR and --build-arg UIDOLR are set, change the GIDOLR value to use id -g
(leaving UIDOLR using id -u) so GIDOLR and UIDOLR correctly reflect group and
user IDs respectively.
---
Nitpick comments:
In `@Makefile`:
- Line 4: Change the OLR_VERSION assignment so it does not default to the
literal "unknown" when git describe fails (e.g., make OLR_VERSION empty on
failure) so CMake can fall back to PROJECT_VERSION; then introduce or use a
separate Docker image tag fallback for image naming (the tag used on the docker
build/push step referenced around line 39) so image naming still gets a
non-empty tag when needed. Specifically, update the OLR_VERSION variable
assignment (the OLR_VERSION ?= ... shell invocation) to return an empty string
on git failure, and update the docker tag logic (the variable used at the docker
build/push step) to explicitly fallback to a tag value when OLR_VERSION is
empty.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec1d36c0-279d-4d73-b305-24e2beb239af
📒 Files selected for processing (7)
CMakeLists.txtDockerfile.devDockerfile.releaseMakefileconfig.h.insrc/StreamClient.cppsrc/main.cpp
Fix GIDOLR using id -g instead of id -u in build-release target.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
Makefile (2)
30-30: Parameterize the release image repository.Hardcoding
rophy/openlogreplicatormakes reuse in other forks/CI less flexible. Prefer a variable (for exampleOLR_RELEASE_IMAGE) and keep the target generic.Also applies to: 39-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 30, Replace the hardcoded release image repository in the Makefile targets that reference "rophy/openlogreplicator" by introducing a variable (e.g., OLR_RELEASE_IMAGE) and use that variable inside the build-release target (and the other target at line ~39); add a sensible default assignment like OLR_RELEASE_IMAGE ?= rophy/openlogreplicator at the top of the Makefile so callers/CI can override it, and update any docker build/tag or push commands in the build-release and the related target to reference $(OLR_RELEASE_IMAGE) instead of the fixed string.
4-4: Add defensive sanitization for Docker tag format safety.While the repository currently uses safe version strings,
OLR_VERSIONshould be sanitized before use as a Docker tag to handle potential edge cases wheregit describeoutput (e.g., from future annotated tags) contains characters invalid in Docker tags.Suggested fix
OLR_IMAGE ?= olr-dev:latest +OLR_RELEASE_IMAGE ?= rophy/openlogreplicator CACHE_IMAGE ?= ghcr.io/bersler/openlogreplicator:ci BUILD_TYPE ?= Debug OLR_VERSION ?= $(shell git describe --tags --always --dirty 2>/dev/null || echo unknown) +OLR_VERSION_TAG ?= $(shell printf '%s' '$(OLR_VERSION)' | sed 's/[^A-Za-z0-9_.-]/-/g') @@ build-release: ## Build OLR release image (minimal, multi-stage) docker buildx build \ --build-arg OLR_VERSION=$(OLR_VERSION) \ @@ - -t rophy/openlogreplicator:$(OLR_VERSION) \ + -t $(OLR_RELEASE_IMAGE):$(OLR_VERSION_TAG) \ --load \ -f Dockerfile.release .Also applies to: 39-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` at line 4, Sanitize OLR_VERSION before using it as a Docker tag: add a sanitized variable (e.g., SAFE_OLR_VERSION) derived from OLR_VERSION that lowercases the string and replaces any character not allowed in Docker tags (allow only [a-z0-9_.-]) with a safe substitute like '-' (or strip them), and then use SAFE_OLR_VERSION wherever OLR_VERSION is used for Docker tagging (refer to the OLR_VERSION variable and the Docker tag usage at the second occurrence around line 39) so all docker tag operations use the cleaned value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Makefile`:
- Line 30: Replace the hardcoded release image repository in the Makefile
targets that reference "rophy/openlogreplicator" by introducing a variable
(e.g., OLR_RELEASE_IMAGE) and use that variable inside the build-release target
(and the other target at line ~39); add a sensible default assignment like
OLR_RELEASE_IMAGE ?= rophy/openlogreplicator at the top of the Makefile so
callers/CI can override it, and update any docker build/tag or push commands in
the build-release and the related target to reference $(OLR_RELEASE_IMAGE)
instead of the fixed string.
- Line 4: Sanitize OLR_VERSION before using it as a Docker tag: add a sanitized
variable (e.g., SAFE_OLR_VERSION) derived from OLR_VERSION that lowercases the
string and replaces any character not allowed in Docker tags (allow only
[a-z0-9_.-]) with a safe substitute like '-' (or strip them), and then use
SAFE_OLR_VERSION wherever OLR_VERSION is used for Docker tagging (refer to the
OLR_VERSION variable and the Docker tag usage at the second occurrence around
line 39) so all docker tag operations use the cleaned value.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
git describe --tags --alwaysfor both CLI version string and Docker image tagOLR_VERSIONstringmake build-releasetarget for production imagesca-certificatesinDockerfile.releasebuilder (--no-install-recommendsbroke HTTPS)Version output
Test plan
make build) verified version outputmake build-release) verified version outputSummary by CodeRabbit
New Features
Documentation
Chores