Use gvisor-tap-vsock version from go.mod file #25688
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: evidolob The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
cc @Luap99 |
Makefile
Outdated
| @@ -219,7 +219,7 @@ endif | |||
|
|
|||
| # gvisor-tap-vsock version for gvproxy.exe and win-sshproxy.exe downloads | |||
| # the upstream project ships pre-built binaries since version 0.7.1 | |||
| GV_VERSION=v0.8.4 | |||
| GV_VERSION=$(shell grep github.com/containers/gvisor-tap-vsock go.mod | cut -d" " -f2) | |||
There was a problem hiding this comment.
I think it would make sense to rename this to GVPROXY_VERSION for some better consistency
Makefile
Outdated
| @@ -865,6 +865,7 @@ podman-remote-release-%.zip: test/version/version ## Build podman-remote for %=$ | |||
| # Downloads pre-built gvproxy and win-sshproxy helpers. See comment on GV_VERSION declaration | |||
| .PHONY: win-gvproxy | |||
| win-gvproxy: test/version/version | |||
| echo $(GV_VERSION) | |||
There was a problem hiding this comment.
do we need this? I don't mind but then it should likely say GVPROXY_VERSION $(GV_VERSION) so that the output is clear on which version it is.
There was a problem hiding this comment.
No, that was a debug echo
Luap99
added
the
No New Tests
evidolob
force-pushed
the
use-gvisor-from-gomod
branch
from
1b18013 to
1e55a06
Compare
winmake.ps1
Outdated
| @@ -72,7 +72,8 @@ function Win-SSHProxy { | |||
|
|
|||
| New-Item -ItemType Directory -Force -Path "./bin/windows" | |||
| if (-Not $Version) { | |||
| $Version = "v0.8.4" | |||
| $match = Select-String -Path ".\go.mod" -Pattern "github.com/containers/gvisor-tap-vsock\s+(v[\d\.]+)" | |||
There was a problem hiding this comment.
| $match = Select-String -Path ".\go.mod" -Pattern "github.com/containers/gvisor-tap-vsock\s+(v[\d\.]+)" | |
| $match = Select-String -Path "$PSScriptRoot\go.mod" -Pattern "github.com/containers/gvisor-tap-vsock\s+(v[\d\.]+)" |
In a lot of places we (incorrectly) assume that winmake.ps1 is executed from the root directory, but for new code we should avoid that.
There was a problem hiding this comment.
I have add you suggestion
Discussion there containers/gvisor-tap-vsock#475 Signed-off-by: Yevhen Vydolob <[email protected]>
evidolob
force-pushed
the
use-gvisor-from-gomod
branch
from
1e55a06 to
fafba60
Compare
|
yeah, LGTM ... @cfergeau this look ok to you and no planned changes with gvproxy that would break this ? |
| @@ -219,7 +219,7 @@ endif | |||
|
|
|||
| # gvisor-tap-vsock version for gvproxy.exe and win-sshproxy.exe downloads | |||
| # the upstream project ships pre-built binaries since version 0.7.1 | |||
| GV_VERSION=v0.8.4 | |||
| GVPROXY_VERSION=$(shell grep github.com/containers/gvisor-tap-vsock go.mod | cut -d" " -f2) | |||
There was a problem hiding this comment.
Perhaps something like this instead (untested):
GVPROXY_VERSION=$(shell $(GO) list -m -f '{{.Version}}' github.com/containers/gvisor-tap-vsock)
Looks fine to me, and I don’t expect changes in gvproxy which would break this. |
openshift-merge-robot
added
the
needs-rebase
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@evidolob Still working on this? It would be great to have this ready by next Tuesday so we have the right versions in the 5.5-rc1 installer. |
Discussion there containers/gvisor-tap-vsock#475
Does this PR introduce a user-facing change?