search

hofstadter-io / hof

Framework that joins data models, schemas, code generation, and a task engine. Language and technology agnostic.
https://docs.hofstadter.io
Apache License 2.0
523 stars 37 forks source link

hof/containers: init fails when no container runtime present, panics in several commands #257

Closed chenrui333 closed 1 year ago

chenrui333 commented 1 year ago

While upgrading the formula to latest release, 0.6.8, I ran into the test failure while version check depends on [ docker podman nerdctl] check.

==> /usr/local/Cellar/hof/0.6.8/bin/hof version
  panic: runtime error: invalid memory address or nil pointer dereference
  [signal SIGSEGV: segmentation violation code=0x1 addr=0x40 pc=0x18f3241]

  goroutine 1 [running]:
  github.com/hofstadter-io/hof/lib/container.GetVersion()
    github.com/hofstadter-io/hof/lib/container/version.go:26 +0xc1
  github.com/hofstadter-io/hof/cmd/hof/cmd.glob..func20(0x2c51740?, {0x20892a2?, 0x0?, 0x0?})
    github.com/hofstadter-io/hof/cmd/hof/cmd/version.go:[57](https://github.com/Homebrew/homebrew-core/actions/runs/5822589822/job/15797103569?pr=139219#step:3:58) +0xc5
  github.com/spf13/cobra.(*Command).execute(0x2c51740, {0x2c9b348, 0x0, 0x0})
    github.com/spf13/cobra@v1.7.0/command.go:944 +0x847
  github.com/spf13/cobra.(*Command).ExecuteC(0x2c50320)
    github.com/spf13/cobra@v1.7.0/command.go:1068 +0x3bd
  github.com/spf13/cobra.(*Command).Execute(...)
    github.com/spf13/cobra@v1.7.0/command.go:992
  github.com/hofstadter-io/hof/cmd/hof/cmd.RunErr()
    github.com/hofstadter-io/hof/cmd/hof/cmd/root.go:173 +0x1[65](https://github.com/Homebrew/homebrew-core/actions/runs/5822589822/job/15797103569?pr=139219#step:3:66)
  github.com/hofstadter-io/hof/cmd/hof/cmd.RunExit()
    github.com/hofstadter-io/hof/cmd/hof/cmd/root.go:141 +0x19
  main.main()
    github.com/hofstadter-io/hof/cmd/hof/main.go:8 +0x17
  failed to find any of [ docker podman nerdctl] in PATH

Also, it would be good to have better error handling rather than displaying the error stack trace.

relates to https://github.com/Homebrew/homebrew-core/pull/139219

verdverm commented 1 year ago

Thanks for the report, yes this should definitely not panic, and should not even really error

Guess we need a test for this, and a new version, there is another bug I wouldn't mind getting in either

verdverm commented 1 year ago

@chenrui333 is it possible to add the docker dependency (or ideally make sure they have one of the 3 listed), and then this test would pass? While not absolutely required, it is very recommended to have a container runtime tool available

I'm happy to make the updates, just need some guidance

chenrui333 commented 1 year ago

yeah, we can add docker dependency for CI, but it actually does not work.

verdverm commented 1 year ago

I've fixed that issue on my end already, warning message and continues with normal operation

In it's broken form, it doesn't actually need containers to run, just the cli to be in the path. It now reports "none"

verdverm commented 1 year ago

This should be fixed, but we should also add at least another test for hof gen that would invoke a formatter, something like js or py code. Then we need to decide what happens in this case. I think we should finish gracefully, writing unformatted code and warning the user?

verdverm commented 1 year ago

should we also add a brew install test or build the formula, will that catch any errors we wouldn't/couldn't otherwise?

(thinking more about our own tap, but also homebrew's?)

verdverm commented 1 year ago

fixed in #261 and #268