Skip to content

Commit

Permalink
fix(tool): some bugfix for kitex tool (cloudwego#1692)
Browse files Browse the repository at this point in the history
  • Loading branch information
@HeyJavaBean
HeyJavaBean authored Feb 11, 2025
1 parent 0fed92c commit 56a1e36
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 158 deletions.
118 changes: 11 additions & 107 deletions tool/cmd/kitex/args/args.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package args

import (
"context"
"errors"
"flag"
"fmt"
Expand All @@ -24,10 +23,7 @@ import (
"os/exec"
"path"
"path/filepath"
"regexp"
"strconv"
"strings"
"time"

"github.com/cloudwego/kitex/tool/internal_pkg/generator"
"github.com/cloudwego/kitex/tool/internal_pkg/log"
Expand Down Expand Up @@ -128,10 +124,8 @@ func (a *Arguments) buildFlags(version string) *flag.FlagSet {
"Specify a protocol for codec")
f.BoolVar(&a.NoDependencyCheck, "no-dependency-check", false,
"Skip dependency checking.")
f.BoolVar(&a.Rapid, "rapid", false,
"Try some experimental features to generate code faster.")
f.BoolVar(&a.LocalThriftgo, "local_thriftgo", false,
"Use local thriftgo exec instead of kitex embedded thriftgo.")
f.BoolVar(&a.LocalThriftgo, "local-thriftgo", false,
"Use local thriftgo exec instead of kitex embedded thriftgo. This is mainly used for debugging, you need to ensure that thriftgo is installed correctly.")
f.Var(&a.BuiltinTpl, "tpl", "Specify kitex built-in template.")
f.BoolVar(&a.StreamX, "streamx", false,
"Generate streaming code with streamx interface",
Expand Down Expand Up @@ -335,7 +329,7 @@ func (a *Arguments) checkPath(curpath string) error {
}
a.PackagePrefix = path.Join(a.ModuleName, filepath.ToSlash(refPath), genPath)
} else {
if err := initGoMod(a.ModuleName); err != nil {
if err := initGoMod(curpath, a.ModuleName); err != nil {
return fmt.Errorf("init go mod failed: %w", err)
}
a.PackagePrefix = path.Join(a.ModuleName, genPath)
Expand Down Expand Up @@ -458,93 +452,20 @@ func (a *Arguments) BuildCmd(out io.Writer) (*exec.Cmd, error) {
return cmd, nil
}

// ValidateCMD check if the path exists and if the version is satisfied
// ValidateCMD check if the path exists and if the version is satisfied. Thriftgo is embedded, only validate protoc.
func ValidateCMD(path, idlType string) error {
// check if the path exists
if _, err := os.Stat(path); err != nil {
tool := "thriftgo"
if idlType == "protobuf" {
tool = "protoc"
}

if idlType == "thrift" {
_, err = runCommand("go install github.com/cloudwego/thriftgo@latest")
if err != nil {
return fmt.Errorf("[ERROR] %s is also unavailable, please install %s first.\n"+
"Refer to https://github.com/cloudwego/thriftgo, or simple run:\n"+
" go install -v github.com/cloudwego/thriftgo@latest", path, tool)
}
} else {
return fmt.Errorf("[ERROR] %s is also unavailable, please install %s first.\n"+
"Refer to https://github.com/protocolbuffers/protobuf", path, tool)
if idlType == "protobuf" {
if _, err := os.Stat(path); err != nil {
return fmt.Errorf("[ERROR] %s is also unavailable, please install protoc first.\n"+
"Refer to https://github.com/protocolbuffers/protobuf", path)
}
}

// check if the version is satisfied
if idlType == "thrift" {
// run `thriftgo -versions and get the output
cmd := exec.Command(path, "-version")
out, err := cmd.CombinedOutput()
if err != nil {
return fmt.Errorf("failed to get thriftgo version: %s", err.Error())
}
if !strings.HasPrefix(string(out), "thriftgo ") {
return fmt.Errorf("thriftgo -version returns '%s', please reinstall thriftgo first", string(out))
}
version := strings.Replace(strings.TrimSuffix(string(out), "\n"), "thriftgo ", "", 1)
if !versionSatisfied(version, requiredThriftGoVersion) {
_, err = runCommand("go install github.com/cloudwego/thriftgo@latest")
if err != nil {
return fmt.Errorf("[ERROR] thriftgo version(%s) not satisfied, please install version >= %s",
version, requiredThriftGoVersion)
}
}
return nil
log.Warnf("You are using local thriftgo: %s. Please make sure the version is matched with kitex tool by yourself.", path)
}
return nil
}

var versionSuffixPattern = regexp.MustCompile(`-.*$`)

func removeVersionPrefixAndSuffix(version string) string {
version = strings.TrimPrefix(version, "v")
version = strings.TrimSuffix(version, "\n")
version = versionSuffixPattern.ReplaceAllString(version, "")
return version
}

func versionSatisfied(current, required string) bool {
currentSegments := strings.SplitN(removeVersionPrefixAndSuffix(current), ".", 3)
requiredSegments := strings.SplitN(removeVersionPrefixAndSuffix(required), ".", 3)

requiredHasSuffix := versionSuffixPattern.MatchString(required)
if requiredHasSuffix {
return false // required version should be a formal version
}

for i := 0; i < 3; i++ {
var currentSeg, minimalSeg int
var err error
if currentSeg, err = strconv.Atoi(currentSegments[i]); err != nil {
log.Warnf("invalid current version: %s, seg=%v, err=%v", current, currentSegments[i], err)
return false
}
if minimalSeg, err = strconv.Atoi(requiredSegments[i]); err != nil {
log.Warnf("invalid required version: %s, seg=%v, err=%v", required, requiredSegments[i], err)
return false
}
if currentSeg > minimalSeg {
return true
} else if currentSeg < minimalSeg {
return false
}
}
if currentHasSuffix := versionSuffixPattern.MatchString(current); currentHasSuffix {
return false
}
return true
}

// LookupTool returns the compiler path found in $PATH; if not found, returns $GOPATH/bin/$tool
func LookupTool(idlType, compilerPath string) string {
tool := "thriftgo"
Expand All @@ -568,7 +489,7 @@ func LookupTool(idlType, compilerPath string) string {
return path
}

func initGoMod(module string) error {
func initGoMod(curpath, module string) error {
if util.Exists("go.mod") {
return nil
}
Expand All @@ -577,6 +498,7 @@ func initGoMod(module string) error {
return err
}
cmd := &exec.Cmd{
Dir: curpath,
Path: pathToGo,
Args: []string{"go", "mod", "init", module},
Stdin: os.Stdin,
Expand All @@ -585,21 +507,3 @@ func initGoMod(module string) error {
}
return cmd.Run()
}

func runCommand(input string) (string, error) {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
arr := strings.Split(input, " ")
var cmd *exec.Cmd
if len(arr) > 1 {
cmd = exec.CommandContext(ctx, arr[0], arr[1:]...)
} else {
cmd = exec.CommandContext(ctx, arr[0])
}
output, err := cmd.CombinedOutput()
if err != nil {
return "", err
}
result := strings.TrimSpace(string(output))
return result, nil
}
31 changes: 0 additions & 31 deletions tool/cmd/kitex/args/args_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,37 +24,6 @@ import (
"github.com/cloudwego/kitex/internal/test"
)

func Test_versionSatisfied(t *testing.T) {
t.Run("lower", func(t *testing.T) {
test.Assert(t, !versionSatisfied("v1.0.0", "v1.0.1"))
test.Assert(t, !versionSatisfied("v1.0.1", "v1.1.0"))
test.Assert(t, !versionSatisfied("v1.1.1", "v2.0.0"))
})

t.Run("equal", func(t *testing.T) {
test.Assert(t, versionSatisfied("v1.2.3", "v1.2.3"))
})

t.Run("higher", func(t *testing.T) {
test.Assert(t, versionSatisfied("v1.2.4", "v1.2.3"))
test.Assert(t, versionSatisfied("v1.2.3", "v1.1.4"))
test.Assert(t, versionSatisfied("v2.2.3", "v1.3.4"))
})

t.Run("suffix", func(t *testing.T) {
test.Assert(t, !versionSatisfied("v1.2.3", "v1.2.3-rc1")) // required shouldn't have suffix
test.Assert(t, !versionSatisfied("v1.2.3-rc1", "v1.2.3")) // suffix means lower
})
t.Run("no prefix", func(t *testing.T) {
test.Assert(t, versionSatisfied("v1.2.3", "1.2.3"))
test.Assert(t, versionSatisfied("1.2.3", "v1.2.3"))
test.Assert(t, versionSatisfied("1.2.3", "1.2.3"))
test.Assert(t, !versionSatisfied("v1.2.3", "1.2.4"))
test.Assert(t, !versionSatisfied("1.2.3", "v1.2.4"))
test.Assert(t, !versionSatisfied("1.2.3", "1.2.4"))
})
}

func Test_refGoSrcPath(t *testing.T) {
gopath, _ := os.Getwd()
t.Setenv("GOPATH", gopath)
Expand Down
17 changes: 0 additions & 17 deletions tool/cmd/kitex/args/version_requirements.go
View file

This file was deleted.

3 changes: 0 additions & 3 deletions tool/cmd/kitex/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,6 @@ func main() {
if args.IDLType == "thrift" && !args.LocalThriftgo {
err = sdk.InvokeThriftgoBySDK(curpath, cmd)
} else {
if args.IDLType == "thrift" {
log.Warn("You are using local thriftgo. Please make sure the version is matched with kitex tool.")
}
err = kargs.ValidateCMD(cmd.Path, args.IDLType)
if err != nil {
log.Warn(err)
Expand Down

0 comments on commit 56a1e36

Please sign in to comment.