Skip to content

migrate: align shared dependencies #183

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ReneWerner87 wants to merge 4 commits into from
Closed

migrate: align shared dependencies #183

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d2e7236
test: stub fiber module with sanitized exec
ReneWerner87 Aug 23, 2025
745f3ae
test: escape fiber go.mod paths
ReneWerner87 Aug 23, 2025
6413e83
test: cross-platform exec stub
ReneWerner87 Aug 23, 2025
f6dec7c
Merge branch 'master' into codex/2025-08-23-22-40-32
ReneWerner87 Aug 24, 2025
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
111 changes: 111 additions & 0 deletions cmd/internal/migrations/dependencies.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
package migrations

import (
"bytes"
"encoding/json"
"fmt"
"os"
"os/exec"
"path/filepath"
"strings"

semver "github.com/Masterminds/semver/v3"
"github.com/spf13/cobra"
"golang.org/x/mod/modfile"
)

// ExecCommand is used to run external commands. It can be replaced in tests.
var ExecCommand = exec.Command

// MigrateDependencies ensures that dependencies shared with Fiber are at least
// the versions required by the target Fiber release.
//
// It updates go.mod files that already require a dependency also required by
// Fiber, bumping the version when it is lower than Fiber's requirement. No
// changes are made if the existing version is equal or higher.
func MigrateDependencies(cmd *cobra.Command, cwd string, _, target *semver.Version) error {
fiberModule := fmt.Sprintf("github.com/gofiber/fiber/v%d@v%s", target.Major(), target.String())

c := ExecCommand("go", "mod", "download", "-json", fiberModule)
var out bytes.Buffer
c.Stdout = &out
c.Stderr = &out
if err := c.Run(); err != nil {
return fmt.Errorf("download fiber module: %w", err)
}
Comment on lines +30 to +35
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Aug 23, 2025

Choose a reason for hiding this comment

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

medium

Capturing stdout and stderr into the same buffer can lead to issues if the command writes to both streams. For instance, if go mod download writes an error or warning to stderr while also writing JSON to stdout, the combined output would be invalid JSON, causing json.Unmarshal to fail. It's safer to capture stderr in a separate buffer to provide clearer error messages.

Suggested change
var out bytes.Buffer
c.Stdout = &out
c.Stderr = &out
if err := c.Run(); err != nil {
return fmt.Errorf("download fiber module: %w", err)
}
var out, stderr bytes.Buffer
c.Stdout = &out
c.Stderr = &stderr
if err := c.Run(); err != nil {
return fmt.Errorf("download fiber module: %w\n%s", err, stderr.String())
}

var info struct {
GoMod string `json:"GoMod"` //nolint:tagliatelle // field name defined by go tool output
}
if err := json.Unmarshal(out.Bytes(), &info); err != nil {
return fmt.Errorf("parse download info: %w", err)
}

b, err := os.ReadFile(info.GoMod) // #nosec G304
if err != nil {
return fmt.Errorf("read fiber go.mod: %w", err)
}
mf, err := modfile.Parse(info.GoMod, b, nil)
if err != nil {
return fmt.Errorf("parse fiber go.mod: %w", err)
}

deps := make(map[string]*semver.Version, len(mf.Require))
for _, r := range mf.Require {
v, err := semver.NewVersion(strings.TrimPrefix(r.Mod.Version, "v"))
if err != nil {
return fmt.Errorf("parse fiber dependency %s version %s: %w", r.Mod.Path, r.Mod.Version, err)
}
deps[r.Mod.Path] = v
}

dirs, err := fiberModuleDirs(cwd)
if err != nil {
return fmt.Errorf("find modules: %w", err)
}

anyChanged := false
for _, dir := range dirs {
modFile := filepath.Join(dir, "go.mod")
b, err := os.ReadFile(modFile) // #nosec G304
if err != nil {
return fmt.Errorf("read %s: %w", modFile, err)
}
mf, err := modfile.Parse(modFile, b, nil)
if err != nil {
return fmt.Errorf("parse %s: %w", modFile, err)
}

changed := false
for _, r := range mf.Require {
targetVer, ok := deps[r.Mod.Path]
if !ok {
continue
}
currVer, err := semver.NewVersion(strings.TrimPrefix(r.Mod.Version, "v"))
if err != nil {
return fmt.Errorf("parse %s version in %s: %w", r.Mod.Path, modFile, err)
}
if currVer.LessThan(targetVer) {
r.Mod.Version = "v" + targetVer.String()
changed = true
}
}

if changed {
mf.SetRequire(mf.Require)
formatted, err := mf.Format()
if err != nil {
return fmt.Errorf("format %s: %w", modFile, err)
}
if err := os.WriteFile(modFile, formatted, 0o600); err != nil {
return fmt.Errorf("write %s: %w", modFile, err)
}
anyChanged = true
}
}

if anyChanged {
cmd.Println("Updating dependency versions")
}
return nil
}
96 changes: 96 additions & 0 deletions cmd/internal/migrations/dependencies_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package migrations_test

import (
"bytes"
"os"
"path/filepath"
"testing"

semver "github.com/Masterminds/semver/v3"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/gofiber/cli/cmd/internal/migrations"
)

func Test_MigrateDependencies(t *testing.T) {
dir, err := os.MkdirTemp("", "mdeps")
require.NoError(t, err)
defer func() { require.NoError(t, os.RemoveAll(dir)) }()

mod := `module example

go 1.22

require (
github.com/gofiber/fiber/v3 v3.0.0
github.com/valyala/fasthttp v1.0.0
github.com/andybalholm/brotli v1.2.0
)`
require.NoError(t, os.WriteFile(filepath.Join(dir, "go.mod"), []byte(mod), 0o600))

fiberMod := `module github.com/gofiber/fiber/v3

go 1.22

require (
github.com/valyala/fasthttp v1.10.0
github.com/andybalholm/brotli v1.0.0
)`
fiberGoMod := filepath.Join(dir, "fiber.mod")
require.NoError(t, os.WriteFile(fiberGoMod, []byte(fiberMod), 0o600))

restore := stubFiberDownload(t, fiberGoMod)
defer restore()

var buf bytes.Buffer
cmd := newCmd(&buf)
target := semver.MustParse("3.0.0")
require.NoError(t, migrations.MigrateDependencies(cmd, dir, nil, target))

content := readFile(t, filepath.Join(dir, "go.mod"))
assert.Contains(t, content, "github.com/valyala/fasthttp v1.10.0")
assert.Contains(t, content, "github.com/andybalholm/brotli v1.2.0")
assert.Contains(t, buf.String(), "Updating dependency versions")
}

func Test_MigrateDependencies_NoChange(t *testing.T) {
dir, err := os.MkdirTemp("", "mdeps_nc")
require.NoError(t, err)
defer func() { require.NoError(t, os.RemoveAll(dir)) }()

mod := `module example

go 1.22

require (
github.com/gofiber/fiber/v3 v3.0.0
github.com/valyala/fasthttp v1.10.0
github.com/andybalholm/brotli v1.2.0
)`
require.NoError(t, os.WriteFile(filepath.Join(dir, "go.mod"), []byte(mod), 0o600))

fiberMod := `module github.com/gofiber/fiber/v3

go 1.22

require (
github.com/valyala/fasthttp v1.10.0
github.com/andybalholm/brotli v1.0.0
)`
fiberGoMod := filepath.Join(dir, "fiber.mod")
require.NoError(t, os.WriteFile(fiberGoMod, []byte(fiberMod), 0o600))

restore := stubFiberDownload(t, fiberGoMod)
defer restore()

var buf bytes.Buffer
cmd := newCmd(&buf)
target := semver.MustParse("3.0.0")
require.NoError(t, migrations.MigrateDependencies(cmd, dir, nil, target))

content := readFile(t, filepath.Join(dir, "go.mod"))
assert.Contains(t, content, "github.com/valyala/fasthttp v1.10.0")
assert.Contains(t, content, "github.com/andybalholm/brotli v1.2.0")
assert.Empty(t, buf.String())
}
40 changes: 40 additions & 0 deletions cmd/internal/migrations/exec_stub_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package migrations_test

import (
"fmt"
"os"
"os/exec"
"path/filepath"
"testing"

"github.com/gofiber/cli/cmd/internal/migrations"
)

// stubFiberDownload replaces migrations.ExecCommand with a helper that
// returns the provided Fiber go.mod path in JSON format. It returns a
// function to restore the original ExecCommand.
func stubFiberDownload(t *testing.T, fiberGoMod string) func() {
t.Helper()
orig := migrations.ExecCommand
out := fmt.Sprintf(`{"GoMod":%q}`, filepath.ToSlash(fiberGoMod))
migrations.ExecCommand = func(string, ...string) *exec.Cmd {
cmd := exec.Command(os.Args[0], "-test.run=TestHelperProcess", "--") // #nosec G204 -- test helper
cmd.Env = []string{
"GO_WANT_HELPER_PROCESS=1",
"GO_HELPER_STDOUT=" + out,
}
return cmd
}
return func() { migrations.ExecCommand = orig }
}
Comment on lines +16 to +29
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 23, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Global override is not parallel-safe

Because migrations.ExecCommand is a global, parallel tests that also mutate it can race. Either guard tests that use this stub from running in parallel or serialize access with a package-level test mutex.

You can quickly check if any tests in this package use t.Parallel():

🏁 Script executed:

#!/bin/bash
# Search for parallel tests and stub usage within the migrations tests
rg -nP --type=go -C1 '\bt\.Parallel\(\)' cmd/internal/migrations
rg -nP --type=go -C2 '\bstubFiberDownload\s*\(' cmd/internal/migrations

Length of output: 20720

Protect migrations.ExecCommand Stub from Parallel Execution

The stubFiberDownload helper globally overrides migrations.ExecCommand, but it’s invoked inside parallel tests in lists_test.go, causing a data race when multiple subtests run concurrently. To resolve this, either serialize access to the global override or disable parallelism for those subtests.

• In cmd/internal/migrations/lists_test.go
– Subtest "silent" (t.Run at line 23) calls t.Parallel() then invokes stubFiberDownload (line 60).
– Subtest "verbose" (t.Run at line 33) calls t.Parallel() then invokes stubFiberDownload (line 83).
Options:

  • Remove the t.Parallel() calls in those subtests, or
  • Introduce a package-level sync.Mutex to guard the assignment/restoration in stubFiberDownload.

• In cmd/internal/migrations/dependencies_test.go
– stubFiberDownload is used (line 43) but the surrounding test isn’t marked parallel—no immediate conflict, though future parallelization could expose the same issue.

Addressing this will ensure test isolation and eliminate flakiness from concurrent overrides.

🤖 Prompt for AI Agents 
In cmd/internal/migrations/exec_stub_test.go around lines 16-29, the
stubFiberDownload helper globally overrides migrations.ExecCommand causing a
race when tests run in parallel; fix by adding a package-level sync.Mutex (e.g.,
var execCommandMu sync.Mutex) and wrap the assignment/restoration inside
stubFiberDownload with execCommandMu.Lock() before setting
migrations.ExecCommand and ensure execCommandMu.Unlock() happens in the returned
cleanup function (use defer-style unlock inside that cleanup), and add the sync
import; alternatively, remove t.Parallel() from the conflicting subtests in
cmd/internal/migrations/lists_test.go (the "silent" and "verbose" subtests) so
the global override is not used concurrently.


func TestHelperProcess(t *testing.T) {
t.Helper()
if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
return
}
if out := os.Getenv("GO_HELPER_STDOUT"); out != "" {
_, _ = fmt.Fprint(os.Stdout, out)
}
os.Exit(0) //nolint:revive // helper process exits intentionally
}
2 changes: 1 addition & 1 deletion cmd/internal/migrations/lists.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type Migration struct {
// Example structure:
// {"from": ">=2.0.0", "to": "<=3.*.*", "fn": [MigrateFN, MigrateFN]}
var Migrations = []Migration{
{From: ">=1.0.0-0", To: ">=0.0.0-0", Functions: []MigrationFn{MigrateGoPkgs}},
{From: ">=1.0.0-0", To: ">=0.0.0-0", Functions: []MigrationFn{MigrateGoPkgs, MigrateDependencies}},
{
From: ">=2.0.0-0",
To: "<4.0.0-0",
Expand Down
23 changes: 20 additions & 3 deletions cmd/internal/migrations/lists_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,22 @@ func Test_DoMigration_Verbose(t *testing.T) {
}

func Test_DoMigration_Verbose_Run(t *testing.T) {
t.Parallel()
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Aug 23, 2025

Choose a reason for hiding this comment

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

medium

Removing t.Parallel() from this test and its sub-tests slows down the test suite. This was likely done to prevent race conditions from concurrent tests modifying the global migrations.ExecCommand. A more robust solution would be to refactor the code to avoid a package-level variable for dependency injection. For example, you could pass the exec.Command function as an argument or make it a field on a struct. This would allow tests to run in parallel without interfering with each other.

curr := semver.MustParse("1.0.0")
target := semver.MustParse("2.0.0")

t.Run("no changes", func(t *testing.T) {
t.Parallel()
fiberMod := `module github.com/gofiber/fiber/v2

go 1.22

require github.com/valyala/fasthttp v1.0.0`
dir := t.TempDir()
fiberGoMod := filepath.Join(dir, "fiber.mod")
require.NoError(t, os.WriteFile(fiberGoMod, []byte(fiberMod), 0o600))

restore := stubFiberDownload(t, fiberGoMod)
t.Cleanup(restore)

require.NoError(t, os.WriteFile(filepath.Join(dir, "go.mod"), []byte("module test\n\nrequire github.com/gofiber/fiber/v2 v2.0.0\n"), 0o600))
var buf bytes.Buffer
cmd := &cobra.Command{}
Expand All @@ -63,9 +71,18 @@ func Test_DoMigration_Verbose_Run(t *testing.T) {
})

t.Run("changes", func(t *testing.T) {
t.Parallel()
fiberMod := `module github.com/gofiber/fiber/v1

go 1.22

require github.com/valyala/fasthttp v1.0.0`
dir := t.TempDir()
fiberGoMod := filepath.Join(dir, "fiber.mod")
require.NoError(t, os.WriteFile(fiberGoMod, []byte(fiberMod), 0o600))

restore := stubFiberDownload(t, fiberGoMod)
t.Cleanup(restore)

require.NoError(t, os.WriteFile(filepath.Join(dir, "go.mod"), []byte("module test\n\nrequire github.com/gofiber/fiber/v1 v1.0.0\n"), 0o600))
require.NoError(t, os.WriteFile(filepath.Join(dir, "main.go"), []byte("package main\nimport \"github.com/gofiber/fiber/v1\"\n"), 0o600))
var buf bytes.Buffer
Expand Down
37 changes: 37 additions & 0 deletions cmd/migrate_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"fmt"
"net/http"
"os"
"os/exec"
Expand All @@ -12,6 +13,7 @@ import (
"github.com/stretchr/testify/require"

cmdinternal "github.com/gofiber/cli/cmd/internal"
"github.com/gofiber/cli/cmd/internal/migrations"
)

func readFileTB(tb testing.TB, path string) string {
Expand All @@ -21,6 +23,41 @@ func readFileTB(tb testing.TB, path string) string {
return string(b)
}

func TestMain(m *testing.M) {
orig := migrations.ExecCommand

tmpDir, err := os.MkdirTemp("", "fiber_mod")
if err != nil {
panic(err)
}
fiberMod := `module github.com/gofiber/fiber/v2

go 1.22

require github.com/valyala/fasthttp v1.0.0`
fiberGoMod := filepath.Join(tmpDir, "go.mod")
if err := os.WriteFile(fiberGoMod, []byte(fiberMod), 0o600); err != nil {
panic(err)
}

migrations.ExecCommand = func(string, ...string) *exec.Cmd {
cmd := exec.Command(os.Args[0], "-test.run=TestHelperProcess", "--", "go", "mod", "download") // #nosec G204 -- test helper
cmd.Env = []string{
"GO_WANT_HELPER_PROCESS=1",
"GO_HELPER_STDOUT=" + fmt.Sprintf(`{"GoMod":%q}`, filepath.ToSlash(fiberGoMod)),
}
return cmd
}

code := m.Run()

migrations.ExecCommand = orig
if err := os.RemoveAll(tmpDir); err != nil {
panic(err)
}
os.Exit(code)
}

const goModV2 = `module example.com/demo

go 1.20
Expand Down
3 changes: 3 additions & 0 deletions cmd/tester_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ func TestHelperProcess(t *testing.T) {
testExit(1)
return
}
if out := os.Getenv("GO_HELPER_STDOUT"); out != "" {
_, _ = fmt.Fprint(os.Stdout, out)
}

testExit(0)
}
Expand Down
Loading