mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-11 03:13:24 +00:00
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.
This commit is contained in:
1 parent
25eaa0166f
commit
cab639d15e
2 files changed
+35
-58
No files matched your search
+17
-27
@@ -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)
|
||||
}
|
||||
|
||||
+18
-31
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user