diff --git a/cmd/opencodereview/main.go b/cmd/opencodereview/main.go index cf3e4ddc..fd47e9e1 100644 --- a/cmd/opencodereview/main.go +++ b/cmd/opencodereview/main.go @@ -5,10 +5,12 @@ package main import ( "context" + "errors" "fmt" "os" "time" + "github.com/alibaba/open-code-review/internal/gitcmd" "github.com/alibaba/open-code-review/internal/llm" "github.com/alibaba/open-code-review/internal/telemetry" ) @@ -22,6 +24,12 @@ func main() { defer telemetry.ShutdownWithTimeout(ctx, 5*time.Second) } + if _, err := gitcmd.CheckGitVersion(); err != nil { + if !errors.Is(err, gitcmd.ErrGitVersionTooOld) { + fmt.Fprintf(os.Stderr, "warning: %v\n", err) + } + } + if err := rootCmd.Execute(); err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) diff --git a/internal/gitcmd/version.go b/internal/gitcmd/version.go new file mode 100644 index 00000000..f6555cab --- /dev/null +++ b/internal/gitcmd/version.go @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 alibaba/open-code-review Contributors + +package gitcmd + +import ( + "errors" + "fmt" + "io" + "os" + "os/exec" + "strings" +) + +type GitVersion struct { + Major int + Minor int + Patch int +} + +var gitVersionMin = GitVersion{Major: 2, Minor: 41, Patch: 0} + +var ErrGitVersionTooOld = errors.New("git version below minimum supported") + +func (v GitVersion) String() string { + return fmt.Sprintf("%d.%d.%d", v.Major, v.Minor, v.Patch) +} + +func (v GitVersion) AtLeast(o GitVersion) bool { + switch { + case v.Major != o.Major: + return v.Major > o.Major + case v.Minor != o.Minor: + return v.Minor > o.Minor + default: + return v.Patch >= o.Patch + } +} + +// ParseGitVersion extracts a MAJOR.MINOR.PATCH version from a git --version +func ParseGitVersion(s string) (GitVersion, error) { + i := 0 + for i < len(s) && (s[i] < '0' || s[i] > '9') { + i++ + } + if i == len(s) { + return GitVersion{}, fmt.Errorf("unrecognized git version %q", strings.TrimSpace(s)) + } + var v GitVersion + n, err := fmt.Sscanf(s[i:], "%d.%d.%d", &v.Major, &v.Minor, &v.Patch) + if err != nil || n < 3 { + return GitVersion{}, fmt.Errorf("unrecognized git version %q", strings.TrimSpace(s)) + } + return v, nil +} + +func versionWarning(current GitVersion) (string, bool) { + if current.AtLeast(gitVersionMin) { + return "", false + } + return fmt.Sprintf( + "warning: git %s is older than the minimum supported version %s. "+ + "Some commands may not work correctly; consider upgrading git.\n", + current, gitVersionMin, + ), true +} + +func CheckGitVersion() (string, error) { + return checkGitVersion(os.Stderr, func() ([]byte, error) { + return exec.Command("git", "--version").Output() + }) +} + +func checkGitVersion(w io.Writer, getVersion func() ([]byte, error)) (string, error) { + out, err := getVersion() + if err != nil { + return "", fmt.Errorf("running git --version: %w", err) + } + v, err := ParseGitVersion(string(out)) + if err != nil { + return "", err + } + if msg, ok := versionWarning(v); ok { + fmt.Fprint(w, msg) + return v.String(), fmt.Errorf("%w: %s", ErrGitVersionTooOld, v.String()) + } + return v.String(), nil +} diff --git a/internal/gitcmd/version_test.go b/internal/gitcmd/version_test.go new file mode 100644 index 00000000..6cd7ca55 --- /dev/null +++ b/internal/gitcmd/version_test.go @@ -0,0 +1,200 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 alibaba/open-code-review Contributors + +package gitcmd + +import ( + "bytes" + "errors" + "fmt" + "os/exec" + "strings" + "testing" +) + +func TestParseGitVersion(t *testing.T) { + tests := []struct { + name string + input string + want GitVersion + wantErr bool + }{ + {name: "plain", input: "git version 2.41.0\n", want: GitVersion{2, 41, 0}}, + {name: "apple suffix", input: "git version 2.39.2 (Apple Git-145)\n", want: GitVersion{2, 39, 2}}, + {name: "windows suffix", input: "git version 2.41.0.windows.1\n", want: GitVersion{2, 41, 0}}, + {name: "no trailing newline", input: "git version 2.45.2", want: GitVersion{2, 45, 2}}, + {name: "no prefix", input: "2.41.0", want: GitVersion{2, 41, 0}}, + {name: "minor 10", input: "git version 2.10.0\n", want: GitVersion{2, 10, 0}}, + {name: "newer major", input: "git version 3.0.0\n", want: GitVersion{3, 0, 0}}, + {name: "empty", input: "", wantErr: true}, + {name: "garbage", input: "banana", wantErr: true}, + {name: "two component", input: "git version 2.41\n", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ParseGitVersion(tt.input) + if tt.wantErr { + if err == nil { + t.Errorf("expected error for input %q, got %v", tt.input, got) + } + return + } + if err != nil { + t.Fatalf("unexpected error for %q: %v", tt.input, err) + } + if got != tt.want { + t.Errorf("got %v, want %v", got, tt.want) + } + }) + } +} + +func TestGitVersion_AtLeast(t *testing.T) { + min := GitVersion{2, 41, 0} + tests := []struct { + v GitVersion + want bool + }{ + {GitVersion{2, 41, 0}, true}, + {GitVersion{2, 41, 1}, true}, + {GitVersion{2, 42, 0}, true}, + {GitVersion{3, 0, 0}, true}, + {GitVersion{2, 40, 0}, false}, + {GitVersion{2, 39, 99}, false}, + {GitVersion{1, 99, 99}, false}, + {GitVersion{2, 40, 1}, false}, + {GitVersion{2, 41, 0}, true}, // duplicate to be explicit + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("%s>=%s=%v", tt.v, min, tt.want), func(t *testing.T) { + if got := tt.v.AtLeast(min); got != tt.want { + t.Errorf("%s.AtLeast(%s) = %v, want %v", tt.v, min, got, tt.want) + } + }) + } +} + +func TestVersionWarning(t *testing.T) { + tests := []struct { + v GitVersion + wantOk bool + contain []string + }{ + {GitVersion{2, 30, 0}, true, []string{"warning:", "2.30.0", "2.41.0", "upgrading git"}}, + {GitVersion{2, 41, 0}, false, nil}, + {GitVersion{2, 41, 1}, false, nil}, + {GitVersion{3, 0, 0}, false, nil}, + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("v=%s", tt.v), func(t *testing.T) { + msg, ok := versionWarning(tt.v) + if ok != tt.wantOk { + t.Errorf("versionWarning(%s) ok=%v, want %v", tt.v, ok, tt.wantOk) + } + if tt.wantOk { + for _, want := range tt.contain { + if !strings.Contains(msg, want) { + t.Errorf("message %q does not contain %q", msg, want) + } + } + } else if msg != "" { + t.Errorf("expected empty message, got %q", msg) + } + }) + } +} + +func TestCheckGitVersion(t *testing.T) { + t.Run("happy path", func(t *testing.T) { + var buf bytes.Buffer + getVersion := func() ([]byte, error) { + return []byte("git version 2.45.2\n"), nil + } + ver, err := checkGitVersion(&buf, getVersion) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if ver != "2.45.2" { + t.Errorf("version = %q, want %q", ver, "2.45.2") + } + if buf.Len() != 0 { + t.Errorf("expected no warning, got %q", buf.String()) + } + }) + + t.Run("old git warns", func(t *testing.T) { + var buf bytes.Buffer + getVersion := func() ([]byte, error) { + return []byte("git version 2.30.0\n"), nil + } + ver, err := checkGitVersion(&buf, getVersion) + if err == nil { + t.Fatal("expected ErrGitVersionTooOld, got nil") + } + if !errors.Is(err, ErrGitVersionTooOld) { + t.Errorf("expected ErrGitVersionTooOld, got %v", err) + } + if ver != "2.30.0" { + t.Errorf("version = %q, want %q", ver, "2.30.0") + } + msg := buf.String() + if !strings.Contains(msg, "warning:") || !strings.Contains(msg, "2.41.0") { + t.Errorf("expected warning with version, got %q", msg) + } + }) + + t.Run("getVersion error", func(t *testing.T) { + var buf bytes.Buffer + getVersion := func() ([]byte, error) { + return nil, errors.New("git not found") + } + _, err := checkGitVersion(&buf, getVersion) + if err == nil { + t.Fatal("expected error, got nil") + } + if buf.Len() != 0 { + t.Errorf("expected no warning, got %q", buf.String()) + } + }) + + t.Run("garbage output", func(t *testing.T) { + var buf bytes.Buffer + getVersion := func() ([]byte, error) { + return []byte("banana\n"), nil + } + _, err := checkGitVersion(&buf, getVersion) + if err == nil { + t.Fatal("expected parse error, got nil") + } + if buf.Len() != 0 { + t.Errorf("expected no warning, got %q", buf.String()) + } + }) +} + +func TestCheckGitVersion_Real(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not found in PATH") + } + ver, err := CheckGitVersion() + if errors.Is(err, ErrGitVersionTooOld) { + // Allowed: installed git is older than minimum. Version string must + // still be populated so the caller can inspect it. + if ver == "" { + t.Error("expected non-empty version string even when too old") + } + return + } + if err != nil { + t.Fatalf("CheckGitVersion() error: %v", err) + } + if ver == "" { + t.Error("expected non-empty version string") + } + if !strings.Contains(ver, ".") { + t.Errorf("version %q does not look like a semver", ver) + } +}