diff --git a/dependency_updater/dependency_updater.go b/dependency_updater/dependency_updater.go index 9b0dc736..e2c547a3 100644 --- a/dependency_updater/dependency_updater.go +++ b/dependency_updater/dependency_updater.go @@ -177,75 +177,97 @@ func getAndUpdateDependency(ctx context.Context, client *github.Client, dependen } func getVersionAndCommit(ctx context.Context, client *github.Client, dependencies Dependencies, dependencyType string) (string, string, VersionUpdateInfo, error) { - var version *github.RepositoryRelease + var selectedTag *github.RepositoryTag var commit string var diffUrl string var updatedDependency VersionUpdateInfo - foundPrefixVersion := false options := &github.ListOptions{Page: 1} + currentTag := dependencies[dependencyType].Tag + tagPrefix := dependencies[dependencyType].TagPrefix + if dependencies[dependencyType].Tracking == "tag" { + // Collect all valid tags across all pages, then find the max version + var validTags []*github.RepositoryTag + for { - releases, resp, err := client.Repositories.ListReleases( + tags, resp, err := client.Repositories.ListTags( ctx, dependencies[dependencyType].Owner, dependencies[dependencyType].Repo, options) if err != nil { - return "", "", VersionUpdateInfo{}, fmt.Errorf("error getting releases: %s", err) + return "", "", VersionUpdateInfo{}, fmt.Errorf("error getting tags: %s", err) } - if dependencies[dependencyType].TagPrefix == "" { - version = releases[0] - if *version.TagName != dependencies[dependencyType].Tag { - diffUrl = generateGithubRepoUrl(dependencies, dependencyType) + "/compare/" + - dependencies[dependencyType].Tag + "..." + *version.TagName - } - break - } else if dependencies[dependencyType].TagPrefix != "" { - for release := range releases { - if strings.HasPrefix(*releases[release].TagName, dependencies[dependencyType].TagPrefix) { - version = releases[release] - foundPrefixVersion = true - if *version.TagName != dependencies[dependencyType].Tag { - diffUrl = generateGithubRepoUrl(dependencies, dependencyType) + "/compare/" + - dependencies[dependencyType].Tag + "..." + *version.TagName - } - break - } + for _, tag := range tags { + // Skip if tagPrefix is set and doesn't match + if tagPrefix != "" && !strings.HasPrefix(*tag.Name, tagPrefix) { + continue } - if foundPrefixVersion { - break + + // Check if this is a valid upgrade (not a downgrade) + if err := ValidateVersionUpgrade(currentTag, *tag.Name, tagPrefix); err != nil { + continue } - options.Page = resp.NextPage - } else if resp.NextPage == 0 { + + validTags = append(validTags, tag) + } + + if resp.NextPage == 0 { break } + options.Page = resp.NextPage } + + // Find the maximum version among valid tags + for _, tag := range validTags { + // Skip if this tag can't be parsed + if _, err := ParseVersion(*tag.Name, tagPrefix); err != nil { + log.Printf("Skipping unparseable tag %s: %v", *tag.Name, err) + continue + } + + if selectedTag == nil { + selectedTag = tag + continue + } + + cmp, err := CompareVersions(*tag.Name, *selectedTag.Name, tagPrefix) + if err != nil { + log.Printf("Error comparing versions %s and %s: %v", *tag.Name, *selectedTag.Name, err) + continue + } + if cmp > 0 { + selectedTag = tag + } + } + + // If no valid version found, keep current version + if selectedTag == nil { + log.Printf("No valid upgrade found for %s, keeping %s", dependencyType, currentTag) + return currentTag, dependencies[dependencyType].Commit, VersionUpdateInfo{}, nil + } + + if *selectedTag.Name != currentTag { + diffUrl = generateGithubRepoUrl(dependencies, dependencyType) + "/compare/" + + currentTag + "..." + *selectedTag.Name + } + + // Get commit SHA from the tag + commit = *selectedTag.Commit.SHA } if diffUrl != "" { updatedDependency = VersionUpdateInfo{ dependencies[dependencyType].Repo, dependencies[dependencyType].Tag, - *version.TagName, + *selectedTag.Name, diffUrl, } } - if dependencies[dependencyType].Tracking == "tag" { - versionCommit, _, err := client.Repositories.GetCommit( - ctx, - dependencies[dependencyType].Owner, - dependencies[dependencyType].Repo, - "refs/tags/"+*version.TagName, - &github.ListOptions{}) - if err != nil { - return "", "", VersionUpdateInfo{}, fmt.Errorf("error getting commit for "+dependencyType+": %s", err) - } - commit = *versionCommit.SHA - - } else if dependencies[dependencyType].Tracking == "branch" { + if dependencies[dependencyType].Tracking == "branch" { branchCommit, _, err := client.Repositories.ListCommits( ctx, dependencies[dependencyType].Owner, @@ -270,8 +292,8 @@ func getVersionAndCommit(ctx context.Context, client *github.Client, dependencie } } - if version != nil { - return *version.TagName, commit, updatedDependency, nil + if selectedTag != nil { + return *selectedTag.Name, commit, updatedDependency, nil } return "", commit, updatedDependency, nil diff --git a/dependency_updater/go.mod b/dependency_updater/go.mod index d467a3e5..85193c9c 100644 --- a/dependency_updater/go.mod +++ b/dependency_updater/go.mod @@ -8,4 +8,7 @@ require ( github.com/urfave/cli/v3 v3.3.8 ) -require github.com/google/go-querystring v1.1.0 // indirect +require ( + github.com/Masterminds/semver/v3 v3.4.0 // indirect + github.com/google/go-querystring v1.1.0 // indirect +) diff --git a/dependency_updater/go.sum b/dependency_updater/go.sum index 53aa8069..72b6f03e 100644 --- a/dependency_updater/go.sum +++ b/dependency_updater/go.sum @@ -1,3 +1,5 @@ +github.com/Masterminds/semver/v3 v3.4.0 h1:Zog+i5UMtVoCU8oKka5P7i9q9HgrJeGzI9SA1Xbatp0= +github.com/Masterminds/semver/v3 v3.4.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/ethereum-optimism/optimism v1.13.3 h1:rfPx7OembMnoEASU1ozA/Foa7Am7UA+h0SB+OUrxn7s= diff --git a/dependency_updater/version.go b/dependency_updater/version.go new file mode 100644 index 00000000..0c1dbbe2 --- /dev/null +++ b/dependency_updater/version.go @@ -0,0 +1,92 @@ +package main + +import ( + "fmt" + "regexp" + "strings" + + "github.com/Masterminds/semver/v3" +) + +// rcPattern matches various RC formats: -rc1, -rc.1, -rc-1, -RC1, etc. +var rcPattern = regexp.MustCompile(`(?i)-rc[.-]?(\d+)`) + +// ParseVersion extracts and normalizes a semantic version from a tag string. +// It handles tagPrefix stripping, v-prefix normalization, and RC format normalization. +func ParseVersion(tag string, tagPrefix string) (*semver.Version, error) { + versionStr := tag + + // Step 1: Strip tagPrefix if present (e.g., "op-node/v1.16.2" -> "v1.16.2") + if tagPrefix != "" && strings.HasPrefix(tag, tagPrefix) { + versionStr = strings.TrimPrefix(tag, tagPrefix) + versionStr = strings.TrimPrefix(versionStr, "/") + } + + // Step 2: Normalize RC formats to semver-compatible format + // "-rc1" -> "-rc.1", "-rc-1" -> "-rc.1" + versionStr = normalizeRCFormat(versionStr) + + // Step 3: Parse using Masterminds/semver (handles v prefix automatically) + v, err := semver.NewVersion(versionStr) + if err != nil { + return nil, fmt.Errorf("invalid version format %q: %w", tag, err) + } + + return v, nil +} + +// normalizeRCFormat converts various RC formats to semver-compatible format. +// Examples: "-rc1" -> "-rc.1", "-rc-2" -> "-rc.2" +func normalizeRCFormat(version string) string { + return rcPattern.ReplaceAllString(version, "-rc.$1") +} + +// ValidateVersionUpgrade checks if transitioning from currentTag to newTag +// is a valid upgrade (not a downgrade). +// Returns nil if valid, error explaining why if invalid. +func ValidateVersionUpgrade(currentTag, newTag, tagPrefix string) error { + // First-time setup: no current version, any valid version is acceptable + if currentTag == "" { + _, err := ParseVersion(newTag, tagPrefix) + return err + } + + // Parse current version + currentVersion, err := ParseVersion(currentTag, tagPrefix) + if err != nil { + // Current version unparseable - still validate new version is parseable + _, newErr := ParseVersion(newTag, tagPrefix) + return newErr + } + + // Parse new version + newVersion, err := ParseVersion(newTag, tagPrefix) + if err != nil { + return fmt.Errorf("new version %q is not a valid semver: %w", newTag, err) + } + + // Check for downgrade + if newVersion.LessThan(currentVersion) { + return fmt.Errorf( + "version downgrade detected: %s -> %s", + currentTag, newTag, + ) + } + + return nil +} + +// CompareVersions compares two version tags and returns: +// -1 if v1 < v2, 0 if v1 == v2, 1 if v1 > v2 +// Returns 0 and error if either version cannot be parsed. +func CompareVersions(v1Tag, v2Tag, tagPrefix string) (int, error) { + v1, err := ParseVersion(v1Tag, tagPrefix) + if err != nil { + return 0, err + } + v2, err := ParseVersion(v2Tag, tagPrefix) + if err != nil { + return 0, err + } + return v1.Compare(v2), nil +} diff --git a/dependency_updater/version_test.go b/dependency_updater/version_test.go new file mode 100644 index 00000000..bc1944f4 --- /dev/null +++ b/dependency_updater/version_test.go @@ -0,0 +1,187 @@ +package main + +import ( + "testing" +) + +func TestNormalizeRCFormat(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"v0.3.0-rc1", "v0.3.0-rc.1"}, + {"v0.3.0-rc.1", "v0.3.0-rc.1"}, + {"v0.3.0-rc-1", "v0.3.0-rc.1"}, + {"v0.3.0-RC1", "v0.3.0-rc.1"}, + {"v0.3.0-rc12", "v0.3.0-rc.12"}, + {"v0.3.0", "v0.3.0"}, + {"v0.3.0-alpha", "v0.3.0-alpha"}, + {"v0.3.0-beta.1", "v0.3.0-beta.1"}, + } + + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + result := normalizeRCFormat(tt.input) + if result != tt.expected { + t.Errorf("normalizeRCFormat(%q) = %q, want %q", tt.input, result, tt.expected) + } + }) + } +} + +func TestParseVersion(t *testing.T) { + tests := []struct { + tag string + tagPrefix string + wantErr bool + }{ + // Standard versions + {"v0.2.2", "", false}, + {"v0.3.0", "", false}, + {"1.35.3", "", false}, // nethermind style - no v prefix + + // RC versions + {"v0.3.0-rc1", "", false}, + {"v0.3.0-rc.1", "", false}, + {"v0.3.0-rc-1", "", false}, + {"v0.3.0-rc.2", "", false}, + + // With tagPrefix + {"op-node/v1.16.2", "op-node", false}, + {"op-node/v1.16.3-rc1", "op-node", false}, + + // Non-standard but parseable + {"v1.101603.5", "", false}, // op-geth style + + // Invalid + {"not-a-version", "", true}, + {"", "", true}, + } + + for _, tt := range tests { + t.Run(tt.tag, func(t *testing.T) { + _, err := ParseVersion(tt.tag, tt.tagPrefix) + if (err != nil) != tt.wantErr { + t.Errorf("ParseVersion(%q, %q) error = %v, wantErr %v", tt.tag, tt.tagPrefix, err, tt.wantErr) + } + }) + } +} + +func TestValidateVersionUpgrade(t *testing.T) { + tests := []struct { + name string + currentTag string + newTag string + tagPrefix string + wantErr bool + }{ + // Valid upgrades + {"stable to rc", "v0.2.2", "v0.3.0-rc1", "", false}, + {"rc to rc", "v0.3.0-rc1", "v0.3.0-rc2", "", false}, + {"rc to stable", "v0.3.0-rc2", "v0.3.0", "", false}, + {"stable to stable", "v0.2.2", "v0.3.0", "", false}, + {"patch upgrade", "v0.2.2", "v0.2.3", "", false}, + {"minor upgrade", "v0.2.2", "v0.3.0", "", false}, + {"major upgrade", "v0.2.2", "v1.0.0", "", false}, + {"same version", "v0.2.2", "v0.2.2", "", false}, + + // With tagPrefix + {"prefix upgrade", "op-node/v1.16.2", "op-node/v1.16.3", "op-node", false}, + {"prefix rc upgrade", "op-node/v1.16.2", "op-node/v1.17.0-rc1", "op-node", false}, + + // No v prefix (nethermind style) + {"no v prefix upgrade", "1.35.3", "1.35.4", "", false}, + + // Invalid downgrades + {"downgrade major", "v0.3.0", "v0.2.2", "", true}, + {"downgrade minor", "v0.3.0", "v0.2.9", "", true}, + {"downgrade patch", "v0.3.1", "v0.3.0", "", true}, + {"stable to rc same version", "v0.3.0", "v0.3.0-rc2", "", true}, + {"stable to rc older version", "v0.3.0", "v0.2.0-rc1", "", true}, + + // Edge cases + {"empty current - valid new", "", "v0.3.0", "", false}, + {"empty current - invalid new", "", "not-a-version", "", true}, + {"unparseable current allows update", "not-semver", "v0.3.0", "", false}, + + // Unparseable current with unparseable new should fail + {"unparseable current - unparseable new", "rollup-boost/v0.7.11", "websocket-proxy/v0.0.2", "", true}, + {"unparseable current - valid new", "rollup-boost/v0.7.11", "v0.8.0", "", false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := ValidateVersionUpgrade(tt.currentTag, tt.newTag, tt.tagPrefix) + if (err != nil) != tt.wantErr { + t.Errorf("ValidateVersionUpgrade(%q, %q, %q) error = %v, wantErr %v", + tt.currentTag, tt.newTag, tt.tagPrefix, err, tt.wantErr) + } + }) + } +} + +func TestCompareVersions(t *testing.T) { + tests := []struct { + name string + v1 string + v2 string + tagPrefix string + want int + }{ + {"v1 less than v2", "v0.2.2", "v0.3.0", "", -1}, + {"v1 greater than v2", "v0.3.0", "v0.2.2", "", 1}, + {"equal versions", "v0.3.0", "v0.3.0", "", 0}, + {"rc less than stable", "v0.3.0-rc1", "v0.3.0", "", -1}, + {"rc1 less than rc2", "v0.3.0-rc1", "v0.3.0-rc2", "", -1}, + {"stable greater than rc", "v0.3.0", "v0.3.0-rc2", "", 1}, + {"with prefix", "op-node/v1.16.2", "op-node/v1.16.3", "op-node", -1}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := CompareVersions(tt.v1, tt.v2, tt.tagPrefix) + if err != nil { + t.Errorf("CompareVersions(%q, %q, %q) unexpected error: %v", tt.v1, tt.v2, tt.tagPrefix, err) + return + } + if got != tt.want { + t.Errorf("CompareVersions(%q, %q, %q) = %d, want %d", tt.v1, tt.v2, tt.tagPrefix, got, tt.want) + } + }) + } +} + +func TestRCVersionOrdering(t *testing.T) { + // Verify that RC versions are ordered correctly + versions := []string{ + "v0.2.2", + "v0.3.0-rc.1", + "v0.3.0-rc.2", + "v0.3.0", + "v0.3.1", + } + + for i := 0; i < len(versions)-1; i++ { + current := versions[i] + next := versions[i+1] + t.Run(current+" -> "+next, func(t *testing.T) { + err := ValidateVersionUpgrade(current, next, "") + if err != nil { + t.Errorf("Expected %s -> %s to be valid upgrade, got error: %v", current, next, err) + } + }) + } + + // Verify reverse order is invalid + for i := len(versions) - 1; i > 0; i-- { + current := versions[i] + previous := versions[i-1] + t.Run(current+" -> "+previous+" (downgrade)", func(t *testing.T) { + err := ValidateVersionUpgrade(current, previous, "") + if err == nil { + t.Errorf("Expected %s -> %s to be invalid downgrade", current, previous) + } + }) + } +}