From 8efce1ed7453e949ecc0379a4448fe2fbc219086 Mon Sep 17 00:00:00 2001 From: Danyal Prout Date: Wed, 14 Jan 2026 15:12:59 -0600 Subject: [PATCH 1/3] chore: add rc support for dependency updater --- dependency_updater/dependency_updater.go | 101 +++++++------ dependency_updater/go.mod | 5 +- dependency_updater/go.sum | 2 + dependency_updater/version.go | 92 ++++++++++++ dependency_updater/version_test.go | 183 +++++++++++++++++++++++ 5 files changed, 340 insertions(+), 43 deletions(-) create mode 100644 dependency_updater/version.go create mode 100644 dependency_updater/version_test.go diff --git a/dependency_updater/dependency_updater.go b/dependency_updater/dependency_updater.go index 9b0dc736..f269bb3b 100644 --- a/dependency_updater/dependency_updater.go +++ b/dependency_updater/dependency_updater.go @@ -177,75 +177,92 @@ 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 { + log.Printf("Skipping %s for %s: %v", *tag.Name, dependencyType, err) + 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 { + 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 +287,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..f17ff651 --- /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 { + // If current version is unparseable, allow the update + // (could be legacy format or non-semver tag) + return nil + } + + // 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..094fad95 --- /dev/null +++ b/dependency_updater/version_test.go @@ -0,0 +1,183 @@ +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}, + } + + 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) + } + }) + } +} From ab4c42f00bb51925bb2ae02a48bd3e62c525ad72 Mon Sep 17 00:00:00 2001 From: Danyal Prout Date: Wed, 14 Jan 2026 16:03:08 -0600 Subject: [PATCH 2/3] fix: skip unparseable versions in dependency updater When the current tag couldn't be parsed (e.g., prefixed tag without tagPrefix configured), the updater would allow any tag through, potentially selecting tags from different products in the same repo. Now unparseable tags are properly skipped in both validation and max-version selection. --- dependency_updater/dependency_updater.go | 6 ++++++ dependency_updater/version.go | 6 +++--- dependency_updater/version_test.go | 4 ++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/dependency_updater/dependency_updater.go b/dependency_updater/dependency_updater.go index f269bb3b..663c8ac0 100644 --- a/dependency_updater/dependency_updater.go +++ b/dependency_updater/dependency_updater.go @@ -223,6 +223,12 @@ func getVersionAndCommit(ctx context.Context, client *github.Client, dependencie // 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 diff --git a/dependency_updater/version.go b/dependency_updater/version.go index f17ff651..0c1dbbe2 100644 --- a/dependency_updater/version.go +++ b/dependency_updater/version.go @@ -54,9 +54,9 @@ func ValidateVersionUpgrade(currentTag, newTag, tagPrefix string) error { // Parse current version currentVersion, err := ParseVersion(currentTag, tagPrefix) if err != nil { - // If current version is unparseable, allow the update - // (could be legacy format or non-semver tag) - return nil + // Current version unparseable - still validate new version is parseable + _, newErr := ParseVersion(newTag, tagPrefix) + return newErr } // Parse new version diff --git a/dependency_updater/version_test.go b/dependency_updater/version_test.go index 094fad95..bc1944f4 100644 --- a/dependency_updater/version_test.go +++ b/dependency_updater/version_test.go @@ -104,6 +104,10 @@ func TestValidateVersionUpgrade(t *testing.T) { {"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 { From b437436bea103452c1d971f1089fe3be73adab7a Mon Sep 17 00:00:00 2001 From: Danyal Prout Date: Wed, 14 Jan 2026 16:32:47 -0600 Subject: [PATCH 3/3] remove tag --- dependency_updater/dependency_updater.go | 1 - 1 file changed, 1 deletion(-) diff --git a/dependency_updater/dependency_updater.go b/dependency_updater/dependency_updater.go index 663c8ac0..e2c547a3 100644 --- a/dependency_updater/dependency_updater.go +++ b/dependency_updater/dependency_updater.go @@ -208,7 +208,6 @@ func getVersionAndCommit(ctx context.Context, client *github.Client, dependencie // Check if this is a valid upgrade (not a downgrade) if err := ValidateVersionUpgrade(currentTag, *tag.Name, tagPrefix); err != nil { - log.Printf("Skipping %s for %s: %v", *tag.Name, dependencyType, err) continue }