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

Add version subcomand to wsl-helper #8270

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

bcxpro
Copy link
Contributor

@bcxpro bcxpro commented Feb 21, 2025

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

@jandubois
Copy link
Member

I am not sure which version number to use and where it is more appropiate to store the version constant.

I think the version should be the Rancher Desktop version, and not some independent utility version. We already do the latter for rdctl and never bump it, so it is completely useless.

In a Makefile we would determine the version something like this:

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 scripts/dependencies/go-source.ts.

@bcxpro bcxpro force-pushed the 5808-wsl-helper-version branch from 6a68596 to 9275037 Compare February 24, 2025 16:50
@bcxpro
Copy link
Contributor Author

bcxpro commented Feb 24, 2025

@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.

@bcxpro bcxpro force-pushed the 5808-wsl-helper-version branch from 9275037 to 1275481 Compare February 24, 2025 17:11
jandubois
jandubois previously approved these changes Feb 24, 2025
Copy link
Member

@jandubois jandubois left a 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

@@ -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, '.'], {
Copy link
Contributor

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).

Copy link
Member

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.

@bcxpro bcxpro force-pushed the 5808-wsl-helper-version branch 2 times, most recently from 70cef82 to b02b881 Compare February 25, 2025 14:29
@bcxpro bcxpro force-pushed the 5808-wsl-helper-version branch from b02b881 to 4508ec4 Compare February 25, 2025 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wsl-helper needs either a --version option or a version subcommand
3 participants