Sunday, April 07, 2024

Re: [patch (kind of)] sysutils/gitlab-cli: terminal input doesn't work properly on OpenBSD

On Fri, Apr 05, 2024 at 09:18:12PM +0300, Mikhail Pchelin wrote:
> [cc'ing maintainer]
>
> Currently glab port doesn't properly handle terminal input, for example,
> after 'glab auth login' (setup wizard) you won't be able to type
> properly or choose the options, you will get ^[[B^[[A etc stuff.
>
> This happens because survey library on which whole interface is based
> still uses syscall.* interface, which was removed in OpenBSD.

gitlab-cli and github-cli have been broken in this regard for some time
already, but I never investigated it, thanks for tracking it down.

>
> The library (https://github.com/AlecAivazis/survey/) was abandoned by
> the author:
>
> > This project is no longer maintained. For an alternative, please check
> > out: https://github.com/charmbracelet/bubbletea
>
> Inlined patch for this library fixes everything for me, but I must admit
> that I've spent only couple hours with Go, and even if the patch looks
> simple it can contain stupid mistakes, I'd be appreciated if someone
> more proficient in Go can take a look at it (especially this 'unsafe'
> stuff).
>
> What would be the best way to bring the fix into the ports tree?

Best would to have it fixed upstream, either via patched survey or by
switching to whatever can replace it.

Otherwise patching MODULES=lang/go ports isn't possible as modules are
extracted during build, i.e. not before the patch target.

One way is to build from a vendor tarball, such that the lang/go module
doesn't do module-wise extraction and we can patch is like any other port.

> To understand what I'm talking about here is the steps for reproduction:
>
> # build original gitlab-cli
> cd /tmp
> export GOPATH=/tmp/go
> git clone https://gitlab.com/gitlab-org/cli
> cd cli
> gmake
> ./bin/glab auth login
> <type/use arrows>
>
> reset
>
> # clone the library with the patch
> cd /tmp
> git clone https://github.com/bsdmp/survey
>
> # make go use patched library
> cd /tmp/cli
> go mod edit -replace github.com/AlecAivazis/survey/v2=/tmp/survey
> gmake
> ./bin/glab auth login
> <should work fine now>
>
> diff --git a/go.mod b/go.mod
> index 4837ce7..cc04509 100644
> --- a/go.mod
> +++ b/go.mod
> @@ -10,6 +10,7 @@ require (
> github.com/mattn/go-isatty v0.0.8
> github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b
> github.com/stretchr/testify v1.6.1
> + golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f
> golang.org/x/term v0.0.0-20210927222741-03fcf44c2211
> golang.org/x/text v0.4.0
> )
> diff --git a/terminal/runereader_posix.go b/terminal/runereader_posix.go
> index 563a081..4c1e88a 100644
> --- a/terminal/runereader_posix.go
> +++ b/terminal/runereader_posix.go
> @@ -14,7 +14,7 @@ import (
> "bytes"
> "fmt"
> "syscall"
> - "unsafe"
> + "golang.org/x/sys/unix"
> )
>
> const (
> @@ -23,7 +23,7 @@ const (
> )
>
> type runeReaderState struct {
> - term syscall.Termios
> + term unix.Termios
> reader *bufio.Reader
> buf *bytes.Buffer
> }
> @@ -45,19 +45,20 @@ func (rr *RuneReader) Buffer() *bytes.Buffer {
>
> // For reading runes we just want to disable echo.
> func (rr *RuneReader) SetTermMode() error {
> - if _, _, err := syscall.Syscall6(syscall.SYS_IOCTL, uintptr(rr.stdio.In.Fd()), ioctlReadTermios, uintptr(unsafe.Pointer(&rr.state.term)), 0, 0, 0); err != 0 {
> + termios, err := unix.IoctlGetTermios((int)(rr.stdio.In.Fd()), ioctlReadTermios)
> + if err != nil {
> return err
> }
>
> - newState := rr.state.term
> - newState.Lflag &^= syscall.ECHO | syscall.ECHONL | syscall.ICANON | syscall.ISIG
> + rr.state.term = *termios
> + termios.Lflag &^= syscall.ECHO | syscall.ECHONL | syscall.ICANON | syscall.ISIG
> // Because we are clearing canonical mode, we need to ensure VMIN & VTIME are
> // set to the values we expect. This combination puts things in standard
> // "blocking read" mode (see termios(3)).
> - newState.Cc[syscall.VMIN] = 1
> - newState.Cc[syscall.VTIME] = 0
> + termios.Cc[syscall.VMIN] = 1
> + termios.Cc[syscall.VTIME] = 0
>
> - if _, _, err := syscall.Syscall6(syscall.SYS_IOCTL, uintptr(rr.stdio.In.Fd()), ioctlWriteTermios, uintptr(unsafe.Pointer(&newState)), 0, 0, 0); err != 0 {
> + if err = unix.IoctlSetTermios((int)(rr.stdio.In.Fd()), ioctlWriteTermios, termios); err != nil {
> return err
> }
>
> @@ -65,7 +66,7 @@ func (rr *RuneReader) SetTermMode() error {
> }
>
> func (rr *RuneReader) RestoreTermMode() error {
> - if _, _, err := syscall.Syscall6(syscall.SYS_IOCTL, uintptr(rr.stdio.In.Fd()), ioctlWriteTermios, uintptr(unsafe.Pointer(&rr.state.term)), 0, 0, 0); err != 0 {
> + if err := unix.IoctlSetTermios((int)(rr.stdio.In.Fd()), ioctlWriteTermios, &rr.state.term); err != nil {
> return err
> }
> return nil
>

No comments:

Post a Comment