Add go1.16 embed

[cryptix]

Not intended for merge but more as a discussion starter. All done with backwards compat!

Since all the sources are in that big migrate.go I wasn't sure how to go about this but the new source could be pulled into a separate file with a // +build so that the rest of the code still works on previous versions.

Adding a root directory to findMigrations() was necessary because dir.Open("/") just returns a NotFound error and thus migrationFromFile() also had to change a little.

[rubenv]

Yes, great, we need this!

A version flag will be needed for as long as we support older Go (which we should for a while). Shouldn't be a problem though.

[cryptix]

Yup, done. Moved to a separate file.

I also took the liberty to add Go 1.15 and 1.16 to the .travis.yml

[cryptix]

I tested the code locally with 1.15 and .16 and it passed.

The failure on travis seems to be related to one of the dependencies and the stricter checking of go.mod.

[rubenv]

I also took the liberty to add Go 1.15 and 1.16 to the .travis.yml

Awesome, thanks!

[rubenv]

Yeah they're no longer doing that automatically. Nothing problematic.

…

On Mon, Feb 22, 2021, 11:29 Henry ***@***.***> wrote: I tested the code locally with 1.15 and .16 and it passed. The failure on travis <https://travis-ci.org/github/rubenv/sql-migrate/jobs/759974466#L302> seems to be related to one of the dependencies and the stricter checking of go.mod. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#189 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAKPGGA2JQUXTMHRQZSYPLTAIWZBANCNFSM4X2JGQKQ> .

[cryptix]

Ah right I just realized this tests all the deps. Oh well.. :)

Feel free to squash and merge this, then!

[rubenv]

Could that be fixed by upgrading the dependency?

[cryptix]

Could that be fixed by upgrading the dependency?

I guess so. The previous mssqldb was a commit from 2019. I bumped it to v0.9.0 but I have zero capacity to test that anywhere beyond checking that it builds.

[cryptix]

Oh well.. still fails.

Actually not sure what is up there. I see golang.org/x/crypto in the go.sum of mssqldb but maybe I'm reading it wrong or that version didn't have md4? 😕

[cryptix]

I opened denisenkom/go-mssqldb#641 just in case.

[rubenv]

Thanks! We can't just ignore it, if it fails for us it'll fail for our users as well.

…

On Mon, Feb 22, 2021, 14:03 Henry ***@***.***> wrote: I opened denisenkom/go-mssqldb#641 <denisenkom/go-mssqldb#641> just in case. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#189 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAKPGDNZVDHQ4XWTLEH7W3TAJI3RANCNFSM4X2JGQKQ> .

[rugwirobaker]

This looks great. Is there any work around the module issue for people who want to use this(before it's merged)?

[cryptix]

@rugwirobaker I don't think you will run into this last issue unless you test the dependencies with ./....

[codefromthecrypt]

this will fix #129 ya?

[titolins]

This looks great. Is there any work around the module issue for people who want to use this(before it's merged)?

Great work, would really love to see this merged as well!

While this is not the case tho, this worked for me (I have the migrations in the migrations/ dir obviously):

//go:embed migrations/*.sql
var migrations embed.FS

func runMigrations(db *sql.DB, steps int) int {
        .
        .
        .
	m := &migrate.AssetMigrationSource{
		Asset: migrations.ReadFile,
		AssetDir: func() func(string) ([]string, error) {
			return func(path string) ([]string, error) {
				dirEntry, err := migrations.ReadDir(path)
				if err != nil {
					return nil, err
				}
				entries := make([]string, 0)
				for _, e := range dirEntry {
					entries = append(entries, e.Name())
				}

				return entries, nil
			}
		}(),
		Dir: "migrations",
	}
        .
        .
        .
	n, err := migrate.ExecMax(db, "postgres", m, direction, steps)
	if err != nil {
		log.Fatal(err)
	}

        return n
}

[insidieux]

Hi there.

I'd like suggest some notes.

May be we'll not change previous methods and sources? And provide new MigrationSource implementation with support embed.FS and without dependencies from other FS?

I'll provide example below.

import (
	"bytes"
	"errors"
	"fmt"
	"io/fs"
	"strings"
)

// EmbedFS interface for supporting embed.FS as injected filesystem and provide possibility to mock.
type EmbedFS interface {
	fs.ReadFileFS
}

// EmbedFileMigrationSource implements MigrationSource and provide migrations from a native embed filesystem.
type EmbedFileMigrationSource struct {
	Filesystem EmbedFS
}

var _ MigrationSource = (*EmbedFileMigrationSource)(nil)

var (
	ErrEmbedWalkFailed    = errors.New(`failed to walk recursive over embed source directory`)
	ErrEmbedReadDirFailed = errors.New(`directory read failed`)
)

// FindMigrations is part of MigrationSource implementation
func (f *EmbedFileMigrationSource) FindMigrations() ([]*Migration, error) {
	items := make([]*Migration, 0)
	err := fs.WalkDir(f.Filesystem, `.`, func(path string, entry fs.DirEntry, err error) error {
		if err != nil {
			return fmt.Errorf(`%v: %v`, ErrEmbedReadDirFailed, err)
		}
                 // from now we always return nil cause if we send err, WalkDir stop processing
		if entry.IsDir() {
			return nil
		}
		if !strings.HasSuffix(entry.Name(), `.sql`) {
			return nil
		}
		content, err := f.Filesystem.ReadFile(path)
		if err != nil {
			return nil
		}
		migration, err := ParseMigration(entry.Name(), bytes.NewReader(content))
		if err != nil {
			return nil
		}
		items = append(items, migration)
		return nil
	})
	if err != nil {
		return items, fmt.Errorf(`%v: %v`, ErrEmbedWalkFailed, err)
	}
	return items, nil
}

This implementation does not depends on other FS or other implementations and just use internal package ParseMigration function to prepare migration slice.

Also,fs.WalkDir support internal subdirectories, so can parse migrations recursive.

[cryptix]

May be we'll not change previous methods and sources?

This PR doesn't do that. It simply reuses the iteration code.

The only blocking issue seems to be some testing headache that I can't fix since I don't have time to investigate it origin and don't have knowledge about MSSQL, which I already said weeks ago. Additionally it doesn't seem like the people on of package care much either 🤷‍♂️. Again, to me it seems very tangential to the changes here.

People are welcome to test this branch with a replace statement. We have been using it since I opened this PR without any issues.

[insidieux]

@cryptix

This PR doesn't do that. It simply reuses the iteration code.

And i suggest just write new dedicated code. As you see in my example - implementation is "easier" cause of we need only pass embed.FS without passing Root/Dir param. And it rely on own, native golang iteration. May be use it?

The only blocking issue seems to be some testing headache that I can't fix since I don't have time to investigate it origin

Just need to update go.mod and go.sum and tests will be passed, you can see example in other my pull request #192

[rubenv]

I went ahead and merged this, without the go.{mod,sum} changes. Thanks!