Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions cmd/protobuild/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ import (
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/pluginpb"
yaml "gopkg.in/yaml.v3"

_ "github.com/samber/lo"
_ "golang.org/x/mod/module"
)

var (
Expand Down Expand Up @@ -334,8 +331,8 @@ func Main() *cli.Command {

var changed bool

// 解析go.mod并获取所有pkg版本
versions := modutil.LoadVersions()
// 通过 go mod graph 获取每个 pkg 最大版本
versions := modutil.LoadVersionGraph()
for i, dep := range globalCfg.Depends {
pathVersion := strings.SplitN(dep.Url, "@", 2)
if len(pathVersion) == 2 {
Expand All @@ -357,7 +354,7 @@ func Main() *cli.Command {
v := strutil.FirstFnNotEmpty(func() string {
return versions[url]
}, func() string {
return generic.DePtr(dep.Version)
return generic.FromPtr(dep.Version)
}, func() string {
// go.mod中version不存在, 并且protobuf.yaml也没有指定
// go pkg缓存
Expand Down
5 changes: 5 additions & 0 deletions docs/docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,8 @@ package docs
// https://github.com/bufbuild/protocompile
// https://github.com/mitchellh/protoc-gen-go-json
// https://github.com/foxygoat/protog/tree/master/httprule

import (
_ "github.com/samber/lo"
_ "golang.org/x/mod/module"
)
39 changes: 39 additions & 0 deletions internal/modutil/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,15 @@ import (
"io/ioutil"
"os"
"path/filepath"
"strings"

mapset "github.com/deckarep/golang-set/v2"
ver "github.com/hashicorp/go-version"
"github.com/pubgo/funk/assert"
"github.com/pubgo/funk/pathutil"
"github.com/pubgo/funk/v2/result"
"github.com/pubgo/protobuild/internal/shutil"
"github.com/samber/lo"
"golang.org/x/mod/modfile"
)

Expand All @@ -28,6 +34,35 @@ func GoModPath() string {
return getFileByRecursion("go.mod", pwd)
}

func LoadVersionGraph() map[string]string {
modList := strings.Split(result.Wrap(shutil.GoModGraph()).Must(), "\n")
modSet := mapset.NewSet[string]()
for _, m := range modList {
for _, v := range strings.Split(m, " ") {
modSet.Add(strings.TrimSpace(v))
}
}

var modMap = make(map[string][]*ver.Version)
modSet.Each(func(s string) bool {
ver2 := strings.Split(s, "@")
if len(ver2) != 2 {
return false
}

if !strings.HasPrefix(ver2[1], "v") {
return false
}

modMap[ver2[0]] = append(modMap[ver2[0]], ver.Must(ver.NewSemver(ver2[1])))
return false
})
Comment on lines +47 to +59
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The use of ver.Must(...) can cause a panic if go mod graph provides a version string that cannot be parsed as a semantic version. Consider handling the error from ver.NewSemver gracefully, for example, by logging the error and skipping the invalid entry.

modSet.Each(func(s string) bool {
		ver2 := strings.Split(s, "@")
		if len(ver2) != 2 {
			return false
		}

		if !strings.HasPrefix(ver2[1], "v") {
			return false
		}

		v, err := ver.NewSemver(ver2[1])
		if err != nil {
			// Consider logging this error, e.g., slog.Warn("failed to parse version, skipping", "entry", s, "error", err)
			return false
		}

		modMap[ver2[0]] = append(modMap[ver2[0]], v)
		return false
	})


return lo.MapValues(modMap, func(versions []*ver.Version, path string) string {
return "v" + maxVersion(versions).String()
})
}

func LoadVersions() map[string]string {
path := GoModPath()
assert.Assert(path == "", "go.mod not exists")
Expand All @@ -51,3 +86,7 @@ func LoadVersions() map[string]string {

return versions
}

func maxVersion(versions []*ver.Version) *ver.Version {
return lo.MaxBy(versions, func(a *ver.Version, b *ver.Version) bool { return a.GreaterThan(b) })
}
47 changes: 47 additions & 0 deletions internal/modutil/util_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package modutil

import (
mapset "github.com/deckarep/golang-set/v2"
ver "github.com/hashicorp/go-version"
"github.com/pubgo/funk/pretty"
"github.com/pubgo/funk/v2/result"
"github.com/pubgo/protobuild/internal/shutil"
"github.com/samber/lo"
"strings"
"testing"
)

func TestName(t *testing.T) {
pretty.Println(LoadVersions())

modList := strings.Split(result.Wrap(shutil.GoModGraph()).Must(), "\n")
modSet := mapset.NewSet[string]()
for _, m := range modList {
for _, v := range strings.Split(m, " ") {
modSet.Add(strings.TrimSpace(v))
}
}

var modMap = make(map[string][]*ver.Version)
modSet.Each(func(s string) bool {
ver2 := strings.Split(s, "@")
if len(ver2) != 2 {
return false
}

if !strings.HasPrefix(ver2[1], "v") {
return false
}

modMap[ver2[0]] = append(modMap[ver2[0]], ver.Must(ver.NewSemver(ver2[1])))
return false
})

for k, v := range modMap {
pretty.Println(k, maxVersion(v).String(), minVersion(v).String())
}
}
Comment on lines +14 to +43
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The test function TestName has critical issues:

  1. It does not contain any assertions (e.g., t.Errorf, require.Equal) to verify behavior. It only prints values, so it can't detect regressions.
  2. It duplicates the implementation of LoadVersionGraph instead of calling it. Tests should treat the function under test as a black box.

Please refactor this to be a proper unit test. It should call LoadVersionGraph and assert its results against expected values. To make the test deterministic, consider mocking shutil.GoModGraph to return a fixed go mod graph output.


func minVersion(versions []*ver.Version) *ver.Version {
return lo.MaxBy(versions, func(a *ver.Version, b *ver.Version) bool { return !a.GreaterThan(b) })
}
Comment on lines +45 to +47
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The minVersion function is implemented using lo.MaxBy. While technically correct that using a <= comparison with MaxBy finds the minimum, this is confusing and unidiomatic. The lo library provides lo.MinBy, which should be used here for better readability and maintainability.

Suggested change
func minVersion(versions []*ver.Version) *ver.Version {
return lo.MaxBy(versions, func(a *ver.Version, b *ver.Version) bool { return !a.GreaterThan(b) })
}
func minVersion(versions []*ver.Version) *ver.Version {
return lo.MinBy(versions, func(a *ver.Version, b *ver.Version) bool { return a.LessThan(b) })
}