Skip to content
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.

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

Embedded FS on Windows #536

Closed
Dissociable opened this issue Apr 9, 2024 · 2 comments · Fixed by #537
Closed

Embedded FS on Windows #536

Dissociable opened this issue Apr 9, 2024 · 2 comments · Fixed by #537
Assignees
Labels

Comments

@Dissociable
Copy link
Contributor

Description
Basically, this is a bug only for Windows. readMigrationsDir sets the dir path via filepath.Clean(dir) which changes the / to \ on Windows.
fs.ReadDir Is not able to handle that. changing filepath.Clean(...) to path.Clean(...) would simply fix it.

  • Version: v2.14.0
  • Database: Postgresql
  • Operating System: Windows 10

Steps To Reproduce

//go:embed migrate/migrations/*
var EmbeddedMigrations embed.FS


	dbMate := dbmate.New(getAddr(dbName))
	dbMate.FS = ent.EmbeddedMigrations
	dbMate.MigrationsDir = []string{"migrate/migrations"}
	fmt.Println("Migrations:")
	migrations, err := dbMate.FindMigrations()
	if err != nil {
		panic(err)
	}

Expected Behavior
Find the embedded files at migrate/migrations

@dossy
Copy link
Collaborator

dossy commented Apr 9, 2024

@Dissociable thanks for reporting this.

This makes me wonder: is it worth investing time in extending our CI test to add a Windows runner to test dbmate on, as well? Similarly, should we add a macOS runner, too?

I know that GitHub hosted Windows and Mac action runners are considerably more expensive, and I don't know if @amacneil wants to incur those costs for the project or not.

dossy pushed a commit that referenced this issue Apr 11, 2024
@dossy dossy self-assigned this Apr 12, 2024
@boludoz
Copy link

boludoz commented Apr 12, 2024

@Dissociable contact with me in discord

amacneil added a commit that referenced this issue Apr 21, 2024
- Run unit tests on all platforms to discover/prevent issues like
#536
- Change dbmate package unit tests to use sqlite instead of postgres
(i.e. no external dependency, although sqlite binary is currently still
required due to `dbmate dump` tests)
- Integration tests can be run by passing the appropriate env vars such
as `POSTGRES_TEST_URL`. If URLs are not provided they will be skipped.
- In CI we only run integration tests in the linux docker job.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants