Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The letter v is now always prepended to output of -v #751

Merged
merged 2 commits into from
Jul 18, 2024

Conversation

olli-holmala
Copy link
Contributor

Proposed changes

We encountered a bug where the output of

nginx-agent -v
nginx-agent version 2.36.0-cb1fc63a

produced a version without the letter 'v' prepended, which does not conform to correct formatting.

This PR ensures that we always prepend the letter 'v' to the output of the '-v' CLI flag.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING document
  • I have run make install-tools and have attached any dependency changes to this pull request
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes
  • If applicable, I have updated any relevant documentation (README.md)
  • If applicable, I have tested my cross-platform changes on Ubuntu 22, Redhat 8, SUSE 15 and FreeBSD 13

@olli-holmala olli-holmala self-assigned this Jul 17, 2024
@github-actions github-actions bot added the bug Something isn't working label Jul 17, 2024
@olli-holmala
Copy link
Contributor Author

Adding some local test output here.

Env var with a 'v' prepended:

➜ agent (fix-version-number) ✔ rm -rf build
➜ agent (fix-version-number) ✔ echo $VERSION
v1.0.0
➜ agent (fix-version-number) ✔ make build
GOWORK=off CGO_ENABLED=0 GOARCH=amd64 go build -pgo=auto -ldflags="-w -X main.version=v1.0.0 -X main.commit=dd0735b7 -X main.date=2024-07-17_11-27-17" -o ./build/nginx-agent
➜ agent (fix-version-number) ✔ ./build/nginx-agent -v
nginx-agent version v1.0.0-dd0735b7

Env var without 'v' prepended:

➜ agent (fix-version-number) ✔ rm -rf build
➜ agent (fix-version-number) ✔ echo $VERSION
1.0.0
➜ agent (fix-version-number) ✔ make build
GOWORK=off CGO_ENABLED=0 GOARCH=amd64 go build -pgo=auto -ldflags="-w -X main.version=v1.0.0 -X main.commit=dd0735b7 -X main.date=2024-07-17_11-26-18" -o ./build/nginx-agent
➜ agent (fix-version-number) ✔ ./build/nginx-agent -v
nginx-agent version v1.0.0-dd0735b7

No VERSION env var at all:

➜ agent (fix-version-number) ✔ rm -rf build
➜ agent (fix-version-number) ✔ echo $VERSION

➜ agent (fix-version-number) ✔ make build
GOWORK=off CGO_ENABLED=0 GOARCH=amd64 go build -pgo=auto -ldflags="-w -X main.version=v2.36.0 -X main.commit=dd0735b7 -X main.date=2024-07-17_11-27-55" -o ./build/nginx-agent
➜ agent (fix-version-number) ✔ ./build/nginx-agent -v
nginx-agent version v2.36.0-dd0735b7

Seems to be working ✅

@oliveromahony
Copy link
Contributor

Thanks for this, https://github.com/nginx/agent/blob/main/scripts/packages/packager/local-entrypoint.sh and https://github.com/nginx/agent/blob/main/scripts/packages/packager/signed-entrypoint.sh also has tr -d 'v' logic in there. Same with the release-branch.yml https://github.com/nginx/agent/blob/main/.github/workflows/release-branch.yml#L49

Is it worth reviewing how these version strings are done and simplify the pipelines here?

@olli-holmala
Copy link
Contributor Author

Thanks for this, https://github.com/nginx/agent/blob/main/scripts/packages/packager/local-entrypoint.sh and https://github.com/nginx/agent/blob/main/scripts/packages/packager/signed-entrypoint.sh also has tr -d 'v' logic in there. Same with the release-branch.yml https://github.com/nginx/agent/blob/main/.github/workflows/release-branch.yml#L49

Is it worth reviewing how these version strings are done and simplify the pipelines here?

Added version number variables to the files you referenced, hopefully it's a bit easier to read now.

@dhurley dhurley changed the title fix: the letter v is now always prepended to output of -v The letter v is now always prepended to output of -v Jul 18, 2024
@olli-holmala olli-holmala merged commit e441f47 into main Jul 18, 2024
28 checks passed
@olli-holmala olli-holmala deleted the fix-version-number branch July 18, 2024 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants