Update github.com/xo/usql v0.9.5 -> v0.19.x
Feature Request
Currently, TiUP depends on a version of usql from 2021:
github.com/xo/usql v0.9.5
See https://github.com/pingcap/tiup/blob/f6aa0ac09b9d6405929c2cea3c0207880e53d199/go.mod#L48
This version is of usql is so old that it itself has outdated and unmaintained dependencies, such as for example in https://github.com/xo/usql/blame/6228ccb8c84c99d34bfbd02da339edbb8a9c3c2e/env/env.go#L23 it uses zaf/temp, which has been deprecated since 2022 and its functionality has been included in the Go standard library since version 1.14 in the os package (os.CreateTemp and os.MkdirTemp).
- I'm not convinced that keeping usql is the right solution for the long run. This is because
tiup client(which uses usql) tries to mimic a Postgres client. This means that it uses a non-standard SQL mode etc. to change the quoting behavior. And this doesn't match with our documentation (which uses the MySQL Client). Alsotiup clientis used only fortiup playground, but not in situations where TiDB is deployed withtiup clusteror the k8s operator. A solution might be to callout tomysqlwith the right options instead of using usql. I do think usql is a good and useful tool, but maybe not in the way that we use it. - Updating usql will probably pull in many new dependencies. I think it might have some ways to build with only a subset of database drivers, but I'm not sure if we can limit the set of dependencies in
go.mod. - We can try to update usql to the latest version (and then try to keep it up to date) or we can update it to a slightly newer version than we have now if that's easier and if there is a plan to eventually get rid of it.
Looks like the API for github.com/xo/usql/rline changed a bit.
@ottok @xhebox I created two PRs for this. One does a upgrade to the latest version and the other upgrades to a slightly newer version, allowing to do this in multiple steps.
I created a third option: to remove usql
Thanks for looking into this. Just to be explicit, this is not a blocker for tiup in Debian and I am not in a position to decide if TiUP really needs this or not. I filed this mainly because I noticed that usql seems kind of old. The version I am in progress of uploading to Debian is usql 0.19.19.
In #2602 I have a tiny commit that makes TiUP compatible with usql 0.19.19:
Subject: [PATCH 4/6] Make TiUP compatible with usql v0.19.19
As the golang-github-xo-usql-dev package v0.19.19 in Debian is
newer than the v0.9.5 in go.mod of TiUP, the Debian build failed on:
src/github.com/pingcap/tiup/components/client/main.go:98:33: not enough arguments in call to rline.New
have (bool, string, string)
want (bool, bool, bool, string, string)
src/github.com/pingcap/tiup/components/client/main.go:102:70: not enough arguments in call to handler.New
have (rline.IO, *user.User, string, bool)
want (rline.IO, *user.User, string, billy.Filesystem, bool)
Apply these changes on TiUP to make it compatible:
- `rline.New` now requires two additional boolean arguments. These are
set to `false` as safe defaults.
- `handler.New` now requires a `billy.Filesystem` argument. This is
provided by importing `osfs` and creating a new `osfs.New` instance
using the instance data directory.
---
components/client/main.go | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/components/client/main.go b/components/client/main.go
index 1e03eb0560..a12d68ac2e 100644
--- a/components/client/main.go
+++ b/components/client/main.go
@@ -23,6 +23,7 @@ import (
"os/user"
"path"
+ "github.com/go-git/go-billy/v5/osfs"
ui "github.com/gizak/termui/v3"
"github.com/gizak/termui/v3/widgets"
"github.com/pingcap/tiup/pkg/environment"
@@ -95,11 +96,12 @@ func connect(target string) error {
if err != nil {
return fmt.Errorf("can't get current user: %s", err.Error())
}
- l, err := rline.New(false, "", env.HistoryFile(u))
+ l, err := rline.New(false, false, false, "", env.HistoryFile(u))
if err != nil {
return fmt.Errorf("can't open history file: %s", err.Error())
}
- h := handler.New(l, u, os.Getenv(localdata.EnvNameInstanceDataDir), true)
+ dataDir := os.Getenv(localdata.EnvNameInstanceDataDir)
+ h := handler.New(l, u, dataDir, osfs.New(dataDir), true)
if err = h.Open(context.TODO(), ep.dsn); err != nil {
return fmt.Errorf("can't open connection to %s: %s", ep.dsn, err.Error())
}
In #2594 I have a similar, but slightly different solution: https://github.com/pingcap/tiup/pull/2594/files#diff-b0901027b8959f66be5eb63095655f04324c7fbc83348dcf9868699d6d258f4a
Yours is for usql v0.19.25 and fine to merge. I will just bump usql in Debian to v0.19.25 and we are good with that.
#2595 merged
Merged commit https://github.com/pingcap/tiup/commit/c57cefb1f1394613e7f3a80ccdcb03668569a43c updated to usql v0.14.0.