-
Notifications
You must be signed in to change notification settings - Fork 309
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
Add version subcomand to wsl-helper #8270
base: main
Are you sure you want to change the base?
Conversation
I think the version should be the Rancher Desktop version, and not some independent utility version. We already do the latter for In a VERSION := $(shell git describe --match 'v[0-9]*' --dirty='.m' --always --tags)
VERSION_TRIMMED := $(VERSION:v%=%)
GO_BUILD_LDFLAGS := -ldflags="-s -w -X $(PACKAGE)/pkg/version.Version=$(VERSION)" For the bundled utilities this should be done inside |
6a68596
to
9275037
Compare
@jandubois I just pushed new code updating rdctl and wsl-helper with the version stamped as you suggeested. I created a version package in each module with the Version variable. Them updated go-source.ts to stamp the version to the binary. I did not touch the utilities that are build with makefiles yet. Please tell me if this is in line with what you expected. |
9275037
to
1275481
Compare
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, LGTM
On Windows:
C:\Users\SUSE>"\Program Files\Rancher Desktop\resources\resources\win32\internal\wsl-helper" version
wsl-helper version: v1.18.0-57-g65a7241f2
C:\Users\SUSE>rdctl version
rdctl client version: v1.18.0-57-g65a7241f2, targeting server version: v1
On macOS:
❯ rdctl version
rdctl client version: v1.18.0-57-g65a7241f2, targeting server version: v1
scripts/dependencies/go-source.ts
Outdated
@@ -50,7 +63,7 @@ export class GoDependency implements Dependency { | |||
const outFile = this.outFile(context); | |||
|
|||
console.log(`Building go utility \x1B[1;33;40m${ this.name }\x1B[0m from ${ sourceDir } to ${ outFile }...`); | |||
await simpleSpawn('go', ['build', '-ldflags', '-s -w', '-o', outFile, '.'], { | |||
await simpleSpawn('go', ['build', '-ldflags', `-s -w -X ${ this.options.modulePath }/pkg/version.Version=${ this.options.version }`, '-o', outFile, '.'], { |
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.
Both of the new parameters are optional, so for things like nerdctl-stub
we get -X undefined/pkg/version.Version=undefined
, but I guess that ends up being okay? There is no package by such name, but the go compiler doesn't complain (at least CI passes everything).
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.
That's a good point! @bcxpro can you please change the code so that -X
is only passed when the parameters are set? I prefer that the code is "correct" and not just works by accident because the Go compiler does not care.
70cef82
to
b02b881
Compare
Closes rancher-sandbox#5808 Signed-off-by: Carlos Barcenilla <[email protected]>
Closes rancher-sandbox#5808 Signed-off-by: Carlos Barcenilla <[email protected]>
Closes rancher-sandbox#5808 Signed-off-by: Carlos Barcenilla <[email protected]>
Closes rancher-sandbox#5808 Signed-off-by: Carlos Barcenilla <[email protected]>
Signed-off-by: Carlos Barcenilla <[email protected]>
b02b881
to
4508ec4
Compare
Added a version subcommand in a similar way to
rdctl version
. This task is mostly trivial, however I am not sure which version number to use and where it is more appropiate to store the version constant.Closes #5808