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

/opt/finch/bin/socket_vmnet --version shows the commit hash of runfinch/finch, not the commit hash of lima-vm/socket_vmnet #21

Closed
AkihiroSuda opened this issue Nov 22, 2022 · 2 comments · Fixed by #188
Assignees
Labels
bug Something isn't working

Comments

@AkihiroSuda
Copy link

Describe the bug
/opt/finch/bin/socket_vmnet --version shows the commit hash of https://github.com/runfinch/finch, not the commit hash of https://github.com/lima-vm/socket_vmnet

Steps to reproduce

$ finch version
Finch version: v0.1.0

$ /opt/finch/bin/socket_vmnet --version
a16a541

This commit hash exists (a16a541) on https://github.com/runfinch/finch , but does not exist on lima-vm/socket_vmnet@a16a541

Expected behavior

It should print a valid commit hash of https://github.com/lima-vm/socket_vmnet

Additional context

/opt/finch/bin/socket_vmnet seems built from https://github.com/runfinch/finch-core/blob/4e5d8285fa98ef0b851aab095c81a8bf30683f27/Makefile#L21 but the version is accidentally set to the version of https://github.com/runfinch/finch ?

@AkihiroSuda AkihiroSuda added the bug Something isn't working label Nov 22, 2022
@pendo324
Copy link
Member

Confirmed that the commit hash is wrong, and from the finch initial commit. Investigating

@pendo324 pendo324 self-assigned this Nov 22, 2022
@pendo324
Copy link
Member

pendo324 commented Nov 22, 2022

Since we are building socket_vment from a tarball instead of a git repo / submodule, it's picking up the commit hash from the wrong repo when this line in the socket_vmnet Makefile is executed:

VERSION ?= $(shell git describe --match 'v[0-9]*' --dirty='.m' --always --tags)

I think the fix is to set a VERSION parameter here, something like:

SOCKET_VMNET_VERSION := $(or $(SOCKET_VMNET_VERSION),"8b16e51")
...
cd $(SOCKET_VMNET_DEPDIR) && PREFIX=$(SOCKET_VMNET_TEMP_PREFIX) VERSION=$(SOCKET_VMNET_VERSION) $(MAKE) install.bin

Will link to the related finch-core PR when its available.

pendo324 added a commit to runfinch/finch-core that referenced this issue Jan 26, 2023
Signed-off-by: Justin Alvarez <alvajus@amazon.com>

Issue #, if available: runfinch/finch#21

*Description of changes:*
Updates the socket_vmnet target to use the submodule which fixes the
version issue.

*Testing done:*
- Tested locally with latest finch main commit:
```shell
$ make lima-socket-vmnet
$ ./_output/dependencies/lima-socket_vmnet/opt/finch/bin/socket_vmnet --version
> v1.1.0-4-g910aaef
```

- [x] I've reviewed the guidance in CONTRIBUTING.md


#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

Signed-off-by: Justin Alvarez <alvajus@amazon.com>
pendo324 added a commit that referenced this issue Jan 26, 2023
)

Signed-off-by: Justin Alvarez <alvajus@amazon.com>

Issue #, if available: Supersedes #185, resolves #21

*Description of changes:*
- make finch-core a submodule instead of downloading the release tarball
- removes the extra step of uploading / downloading tar files from the
internet at build time
- makes it easier/possible to reference the submodules of finch-core
directly

*Testing done:*
- local build and e2e tests

- [x] I've reviewed the guidance in CONTRIBUTING.md

#### License Acceptance

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license.

Signed-off-by: Justin Alvarez <alvajus@amazon.com>
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 a pull request may close this issue.

2 participants