knative / client-pkg Goto Github PK

SIGSEGV is thrown quite often from:

Here's goroutines dump:

$ go run github.com/cardil/repro-kn-cli-bad-license@latest
Loading... Done
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x569e90]

goroutine 6 [running]:
github.com/charmbracelet/bubbletea.(*Program).ReleaseTerminal(0xc000189450)
	/home/ksuszyns/.gvm/pkgsets/go1.21.4/global/pkg/mod/github.com/charmbracelet/[email protected]/tea.go:651 +0x30
knative.dev/client-pkg/pkg/output/tui.(*BubbleSpinner).start.func1()
	/home/ksuszyns/.gvm/pkgsets/go1.21.4/global/pkg/mod/knative.dev/[email protected]/pkg/output/tui/spinner.go:94 +0xa5
created by knative.dev/client-pkg/pkg/output/tui.(*BubbleSpinner).start in goroutine 1
	/home/ksuszyns/.gvm/pkgsets/go1.21.4/global/pkg/mod/knative.dev/[email protected]/pkg/output/tui/spinner.go:87 +0x396
exit status 2

/kind bug

Bug description

Bubbletea is using the github.com/mattn/go-localereader as a dependency. It is used only for Windows, and maybe that's why it isn't reported here, by our go-licenses tooling. This lib is not very well maintained, as the only available version (v0.0.1), doesn't contain a license file. The license has been added to the main branch, however.

On downstream projects, it is reported by go-licenses as having missing license. Maybe this is because of it, being vendored here, IDK?

Unknown license type  found for library github.com/mattn/go-localereader

/kind bug

Steps to reproduce

1. Checkout https://github.com/cardil/repro-kn-cli-bad-license
2. Run test: go run github.com/google/go-licenses@latest check ./...
3. You should license check failing, with error:

   Unknown license type  found for library github.com/mattn/go-localereader
   

The following block prevents from using regular stdin with tui.Spinner and tui.Progress:

The following situation happens:

$ go run github.com/cardil/repro-kn-cli-bad-license@latest
2024/03/27 11:37:43 WARN: non-regular file given as input:  stdin  (mode:  Dcrw--w---- ). Using `nil` as input.

/kind bug

Fixing #168 with #170 we have, once again, uncovered the charmbracelet/bubbletea#964 bug.

See the error, here: https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative-extensions_kn-plugin-event/339/integration-tests_kn-plugin-event_main/1773009898955608064#1:build-log.txt%3A644

To confirm that, you can checkout the knative-extensions/kn-plugin-event#339, and run (passing a file as stdin):

$ ./mage clean build < go.mod
[MAGE] 🚿 Cleaning... done.
[MAGE] 🔧 Installing tools
[MAGE] 🔧 › Go install "gotest.tools/gotestsum@latest"
[MAGE] 🔧 › Go install "gotest.tools/gotestsum@latest" › installed
[MAGE] 🔧 › Pre-built binary "golangci/[email protected]"
⛳️ Getting information about v1.55.2 release ▱▱⛳️ Getting information about v1.55.2 release Done
[MAGE] 🔧 Pre-built binary "golangci/[email protected]" have failed: error creating cancelreader: add reader to epoll interest list
Error: error creating cancelreader: add reader to epoll interest list
exit status 1

It looks like the /dev/stdin doesn't support the epoll thing.

/kind bug

Hmm, the package knative.dev/client-pkg/pkg/config already exists, as well. I believe, that's why I put it in here...

We should reconcile those two packages somehow. IMHO, we shouldn't be using the kn sub-package, as all of those packages are kn related, obviously.

IDK, maybe we should dump the kn sub-package, probably in a separate PR, to avoid confusion. But, that's a breaking change, so we should prepare migration as well.

WDYT on those two, already existing, config packages?

Originally posted by @cardil in #139 (comment)

Per the conversation on the linked PR. Let's follow-up on the package name changes.

    I'd remove the `lib/` top-level dir completely. Maybe move to `pkg/util/test` package or just `pkg/util.`

Originally posted by @dsimansk in #67 (comment)

On some fast rendering setups like:

• unit tests,
• background processes,
• pipe'ing output/input

the spinner could hang indefinitely on Quit(), and Wait() method:

See the goroutines dump: https://gist.github.com/cardil/039c0691d94b9537126b50361bbf9d3e

/kind bug

As seen in this build, in some CI scenarios, the tui.Spinner and tui.Progress code could panic with SIGSEGV (here an extracted SIGSEGV goroutines dump).

The tui package shouldn't panic with SIGSEGV, in case of initialization errors, due to lack of proper environment.

/kind bug

The additional explicit wait "hack" in the progress bar should be removed. If your UI has a lot of progress bars, this wait will be visible.

But, I don't see a way, with the current API of bubbles' progress, to achieve it otherwise...

Without the wait, we get UI with progress bars that are never 100% completed. See example: #162 (comment)

Originally posted by @cardil in #162 (comment)

As per recent discussion and previous, move code from https://github.com/maximilien/kn-source-pkg to here under pkg so it can be used by kn-source-kafka and other plugins

cc: @rhuss @dsimansk

On some fast rendering setups like:

• unit tests,
• background processes,
• pipe'ing output/input

the spinner could hang indefinitely on Wait(), and <-b.ended method:

See the goroutines dump: https://gist.github.com/cardil/7fa34571b0dc0770113b5ff707f84841

/kind bug

The knative.dev/client-pkg/pkg/kn-source-pkg sub-package was introduced in #74, but contains some weird stuff, that shouldn't be present in a library:

• a main() file
• some hack scripts
• e2e tests for kn-source-plugin, but (what looks like) name-replaced

Let's clean up this as well, and leave just the actual library, just renamed to knative.dev/client-pkg/plugin/source/....

Originally posted by @cardil in #155 (comment)