-
Notifications
You must be signed in to change notification settings - Fork 58
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
🐛 fix catalogd version variable paths in Makefile #1705
base: main
Are you sure you want to change the base?
🐛 fix catalogd version variable paths in Makefile #1705
Conversation
Signed-off-by: Ankita Thomas <[email protected]>
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1705 +/- ##
=======================================
Coverage 67.49% 67.49%
=======================================
Files 59 59
Lines 5003 5003
=======================================
Hits 3377 3377
Misses 1379 1379
Partials 247 247
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -121,7 +121,7 @@ VERSION := $(shell git describe --tags --always --dirty) | |||
endif | |||
export VERSION | |||
|
|||
export VERSION_PKG := $(shell go list -m)/internal/version | |||
export VERSION_PKG := $(shell go list -mod=mod ./internal/version) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix, but I am not sure why we need to use -mod
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested this in the Makefile by printing the VERSION_PKG. Without this fix it resolves to github.com/operator-framework/operator-controller/internal/version
and with the fix it resolves to github.com/operator-framework/operator-controller/catalogd/internal/version
.PHONY: print-version
+print-version:
+ echo "$(VERSION_PKG)"
$ git diff
diff --git a/catalogd/Makefile b/catalogd/Makefile
index ce56ed5..447a6a0 100644
--- a/catalogd/Makefile
+++ b/catalogd/Makefile
@@ -122,6 +122,9 @@ endif
export VERSION
export VERSION_PKG := $(shell go list -m)/internal/version
+.PHONY: print-version
+print-version:
+ echo "$(VERSION_PKG)"
export GIT_COMMIT := $(shell git rev-parse HEAD)
export GIT_VERSION := $(shell git describe --tags --always --dirty)
lmohanty@LenovoP16v:~/code/github.com/OLM-Upstream/operator-controller/catalogd$ make print-version
echo "github.com/operator-framework/operator-controller/internal/version"
github.com/operator-framework/operator-controller/internal/version
$ git diff
diff --git a/catalogd/Makefile b/catalogd/Makefile
index ce56ed5..c2cc8c6 100644
--- a/catalogd/Makefile
+++ b/catalogd/Makefile
@@ -121,7 +121,10 @@ VERSION := $(shell git describe --tags --always --dirty)
endif
export VERSION
-export VERSION_PKG := $(shell go list -m)/internal/version
+export VERSION_PKG := $(shell go list -mod=mod ./internal/version)
+.PHONY: print-version
+print-version:
+ echo "$(VERSION_PKG)"
export GIT_COMMIT := $(shell git rev-parse HEAD)
export GIT_VERSION := $(shell git describe --tags --always --dirty)
$ make print-version
echo "github.com/operator-framework/operator-controller/catalogd/internal/version"
github.com/operator-framework/operator-controller/catalogd/internal/version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-mod=mod
Forces the command to use module mode to resolve dependencies instead of old GOPATH mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Description
Fix for the catalogd version package path in the Makefile to ensure catalogd binary contains the correct version information.
See https://issues.redhat.com/browse/OCPBUGS-49866