search

firecracker-microvm / firectl

firectl is a command-line tool to run Firecracker microVMs
Apache License 2.0
479 stars 72 forks source link

Building firectl requires at least Go 1.14 #61

Closed simonis closed 2 years ago

simonis commented 3 years ago

It looks like building firectl requiers at least Go 1.14. Othwise I get the following build error:

$ make build-in-docker
docker run --rm -v /priv/simonisv/OpenJDK/Git/firectl:/firectl --workdir /firectl golang:1.12 make
Unable to find image 'golang:1.12' locally
1.12: Pulling from library/golang
...
# github.com/hashicorp/go-multierror
/go/pkg/mod/github.com/hashicorp/go-multierror@v1.1.0/multierror.go:112:9: undefined: errors.As
/go/pkg/mod/github.com/hashicorp/go-multierror@v1.1.0/multierror.go:117:9: undefined: errors.Is
note: module requires Go 1.14
make: *** [Makefile:30: firectl] Error 2

The fix is trivial. I've updated the Makefile and Readme.md to use the new version.

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

kzys commented 3 years ago

Thanks! That is surprising since we haven't upgraded multierror on Firecracker Go SDK because of that (https://github.com/firecracker-microvm/firecracker-go-sdk/pull/223).

Do you know who is using multierror v1.1? I'm slightly inclined to downgrade multierror instead.

simonis commented 3 years ago

Thanks! That is surprising since we haven't upgraded multierror on Firecracker Go SDK because of that (firecracker-microvm/firecracker-go-sdk#223).

Do you know who is using multierror v1.1? I'm slightly inclined to downgrade multierror instead.

Not sure. I'm not a Go expert at all. I just realized the build error and found that building with 1.14 fixes it. I tried to remove the dependencies on multierror v1.1 from go.sum but when building they get re-added. I have no idea why, who requires that version and how I could find out.

The only thing I found is that the dependency was introduced by #57 (Jailer support) What do you suggest?

Kern-- commented 2 years ago

We recently decided to take a dependency that requires Go 1.14 in firecracker-go-sdk, so I think this PR is OK to move forward (https://github.com/firecracker-microvm/firecracker-go-sdk/pull/363).

@simonis If you're still available to work on this PR can you please do the following? 1) Address the small comment and 2) Add a signed off line to your commit (e.g. by commiting using git commit -s)

simonis commented 2 years ago

Hi @Kern-- ,

yes, I'm still around, I just didn't saw your comment :)

I've pulled and merged with upstream, changed the comment in Readme.md as requested and signed off the commit.

Please let me know if there's anything else I can do?

Best regards, Volker

austinvazquez commented 2 years ago

@simonis, this looks good to me. To pass the DCO validation, you'll need to squash the commits locally and force push to your branch.

simonis commented 2 years ago

Hi @austinvazquez, I've done as requested. Please let me know if there's anything else I have to do. Best regards, Volker