From cab639d15e5705b6bbcccc67218bc0043bdd8bb6 Mon Sep 17 00:00:00 2001 From: thotam <78630736+thotam@users.noreply.github.com> Date: Sun, 5 Jul 2026 12:26:42 +0700 Subject: [PATCH] fix(migrate): use iofs source so migrations load on Windows (#1358) `goclaw migrate up` (and every other migrate subcommand) failed on Windows with `create migrator: failed to open source, "file:///D:/.../migrations": open .: The filename, directory name, or volume label syntax is incorrect.` golang-migrate's file source driver mis-parses absolute drive-letter file:// URLs; the drive-letter formatting in absoluteToFileURI produced a URL the driver could not open. Replace the file:// URL with an iofs source over os.DirFS, which uses native OS path handling and works identically on every platform. Drop the now-unused absoluteToFileURI/migrationsSourceURL helpers and their URL-shape tests, and add a DB-free regression test that opens the real migrations directory via newMigrationSource(). Verified end-to-end on Windows: `migrate version` and `migrate up` now run. --- cmd/migrate.go | 44 ++++++++++++++++------------------------ cmd/migrate_test.go | 49 +++++++++++++++++---------------------------- 2 files changed, 35 insertions(+), 58 deletions(-) diff --git a/cmd/migrate.go b/cmd/migrate.go index 0941f76d..a7e41b4d 100644 --- a/cmd/migrate.go +++ b/cmd/migrate.go @@ -9,11 +9,11 @@ import ( "os" "path/filepath" "strconv" - "strings" "github.com/golang-migrate/migrate/v4" _ "github.com/golang-migrate/migrate/v4/database/postgres" - _ "github.com/golang-migrate/migrate/v4/source/file" + "github.com/golang-migrate/migrate/v4/source" + "github.com/golang-migrate/migrate/v4/source/iofs" _ "github.com/jackc/pgx/v5/stdlib" "github.com/spf13/cobra" @@ -39,36 +39,26 @@ func resolveMigrationsDir() string { return filepath.Join(filepath.Dir(exe), "migrations") } -// absoluteToFileURI formats an already-absolute path into an RFC 8089-compliant -// file:// URL. golang-migrate's file source driver rejects Windows paths like -// "file://F:\\project\\migrations" because "F" is parsed as the host and -// ":\\..." as the port. The fix is shape-driven (presence of a drive-letter -// colon at index 1), so no runtime.GOOS branch is needed — the same code is -// correct for POSIX inputs ("/app/x" → "file:///app/x") and for Windows inputs -// on any OS ("F:\\x" → "file:///F:/x"). strings.ReplaceAll covers the case -// where a Windows path is seen on a non-Windows runner (filepath.ToSlash is a -// no-op outside Windows). -func absoluteToFileURI(abs string) string { - abs = strings.ReplaceAll(filepath.ToSlash(abs), `\`, `/`) - if len(abs) >= 2 && abs[1] == ':' { - abs = "/" + abs - } - return "file://" + abs -} - -// migrationsSourceURL resolves dir to an absolute path and formats it for -// golang-migrate's file source driver. -func migrationsSourceURL(dir string) string { - abs, err := filepath.Abs(dir) +// newMigrationSource opens the migrations directory as a golang-migrate source. +// It uses an iofs source over os.DirFS rather than a file:// URL: golang-migrate's +// file source driver mis-parses Windows absolute paths — the drive-letter URL +// "file:///D:/..." fails with "open ." errors — whereas os.DirFS uses native OS +// path handling and behaves identically on every platform. +func newMigrationSource() (source.Driver, error) { + dir := resolveMigrationsDir() + src, err := iofs.New(os.DirFS(dir), ".") if err != nil { - abs = dir + return nil, fmt.Errorf("open migrations dir %q: %w", dir, err) } - return absoluteToFileURI(abs) + return src, nil } func newMigrator(dsn string) (*migrate.Migrate, error) { - dir := resolveMigrationsDir() - m, err := migrate.New(migrationsSourceURL(dir), dsn) + src, err := newMigrationSource() + if err != nil { + return nil, err + } + m, err := migrate.NewWithSourceInstance("iofs", src, dsn) if err != nil { return nil, fmt.Errorf("create migrator: %w", err) } diff --git a/cmd/migrate_test.go b/cmd/migrate_test.go index 1b1d9676..a621524d 100644 --- a/cmd/migrate_test.go +++ b/cmd/migrate_test.go @@ -2,41 +2,28 @@ package cmd import ( "path/filepath" - "strings" "testing" ) -func TestAbsoluteToFileURI(t *testing.T) { - cases := []struct { - name string - in string - want string - }{ - {"posix absolute", "/app/migrations", "file:///app/migrations"}, - // Windows drive-letter path with backslashes: the exact shape - // golang-migrate needs. Before the fix, "file://F:\\..." was parsed - // with "F" as host and ":\\..." as port → "invalid port" error. - {"windows backslash", `F:\project\goclaw\migrations`, "file:///F:/project/goclaw/migrations"}, - {"windows mixed separators", `C:/already/forward`, "file:///C:/already/forward"}, - } - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - if got := absoluteToFileURI(c.in); got != c.want { - t.Errorf("got %q, want %q", got, c.want) - } - }) - } -} +// TestNewMigrationSource_LoadsMigrations guards against the Windows regression +// where golang-migrate's file:// source driver failed to open an absolute +// drive-letter path (e.g. "file:///D:/..." → "open ." error). The iofs source +// over os.DirFS must load the real migrations directory on every platform. +func TestNewMigrationSource_LoadsMigrations(t *testing.T) { + migrationsDir = filepath.Join("..", "migrations") + t.Cleanup(func() { migrationsDir = "" }) -// TestMigrationsSourceURLRelative verifies the helper resolves a relative -// input via filepath.Abs before formatting — exact output depends on CWD, -// so we only assert invariants that must hold on every runner. -func TestMigrationsSourceURLRelative(t *testing.T) { - got := migrationsSourceURL("migrations") - if !strings.HasPrefix(got, "file://") { - t.Fatalf("missing file:// prefix: %q", got) + src, err := newMigrationSource() + if err != nil { + t.Fatalf("newMigrationSource: %v", err) } - if !strings.Contains(filepath.ToSlash(got), "/migrations") { - t.Errorf("missing /migrations segment: %q", got) + defer src.Close() + + first, err := src.First() + if err != nil { + t.Fatalf("read first migration: %v", err) + } + if first != 1 { + t.Errorf("first migration version = %d, want 1", first) } }