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

feat: embed, io/fs support #471

Closed
johejo opened this issue Nov 11, 2020 · 17 comments · Fixed by #472 or #560
Closed

feat: embed, io/fs support #471

johejo opened this issue Nov 11, 2020 · 17 comments · Fixed by #472 or #560

Comments

@johejo
Copy link
Contributor

johejo commented Nov 11, 2020

Is your feature request related to a problem? Please describe.

The next Go 1.16 will add official file system interface and file embedding.

golang/go#41190
https://github.com/golang/go/tree/master/src/io/fs

golang/go#41191
https://github.com/golang/go/tree/master/src/embed

It would be great if golang-migrate supports these as source.Driver.

It replaces statik, go-bindata, and pkger.
Of course, this does not mean that they cannot be used.

Describe the solution you'd like

Describe alternatives you've considered

Additional context

We will also need to use the build tag properly.

@dhui
Copy link
Member

dhui commented Nov 11, 2020

Thanks for sharing!

There's already support for a httpfs source (net/http/FileSystem), so we could do something similar for io/fs/FS.

I believe, if an io/fs/FS source driver existed, we wouldn't need an explicit embed driver.

We'll need to add a build constraint for go1.16 for the io/fs/FS source driver.

johejo added a commit to johejo/migrate that referenced this issue Nov 12, 2020
@dhui dhui closed this as completed in #472 Nov 18, 2020
dhui pushed a commit that referenced this issue Nov 18, 2020
* Add io/fs#FS Driver #471

* Refactor iofs.Iofs to be independent of httpfs

* Rename Iofs to IoFS, accepts fs.FS and use fs.DirEntry.Info() to undo the changes in ErrDuplicateMigration

* fix documents and migration directory for testing

* Fix iofs.Driver so that it can be used by embedding it like httpfs.PartialDriver

* iofs.Driver closes the file system if possible

* Rename receiver

* Refactor file driver to use iofs.Driver

* iofs build constraint is not needed so remove it

* Properly return an error in a corner case

* Increase golangci-lint timeout

* Fix example code

* Revert "Increase golangci-lint timeout"

This reverts commit 2cd12a6.

* unexport driver, add iofs.PartialDriver, remove type alias from file driver

* do not panic in Open, return error
johejo added a commit to johejo/migrate that referenced this issue Nov 21, 2020
dhui added a commit that referenced this issue Nov 22, 2020
@johejo
Copy link
Contributor Author

johejo commented Nov 24, 2020

We will not be able to include this feature until Go 1.15 is no longer supported. #480
However, some users may want to use this feature, so I created an extra module.
https://github.com/johejo/golang-migrate-extra
This is based on #472 .

@dhui
Copy link
Member

dhui commented Nov 24, 2020

Re-opening and will revert the revert when Go 1.15 is no longer supported by migrate

@dhui dhui reopened this Nov 24, 2020
@wkoszek
Copy link

wkoszek commented Jan 21, 2021

@dhui @johejo Any way we (segmed.ai) can help here? We can provide 2 pairs of eyes to help with getting this support integrated, and also could help with testing. @Fenges

@dhui
Copy link
Member

dhui commented Jan 22, 2021

@wkoszek There's nothing anyone can do but wait for Go 1.15 to be not be officially supported e.g. when the old supported version of Go (1.16) has the io/fs package. Otherwise, people running migrate on older versions of Go will run into this issue

klingtnet pushed a commit to spreadshirt/migrate that referenced this issue Feb 18, 2021
Update: I was to late, this was already implemented with golang-migrate#471.

Closes golang-migrate#507
@philippta
Copy link

Possible workaround until Go 1.15 is no longer supported:

main.go
migrations/
  1_create_tables.up.sql
  1_create_tables.down.sql
import "github.com/golang-migrate/migrate/v4/source/httpfs"

//go:embed migrations
var migrations embed.FS

source, err := httpfs.New(http.FS(migrations), "migrations")
// migrate.NewInstance("httpfs", source, ...)

@prochac
Copy link

prochac commented Feb 21, 2021

We should use build tags to support 1.16 features and keep the support for 1.15 at least until 1.17 is released
(Following https://golang.org/doc/devel/release.html#policy)

Take an example from here:
https://github.com/pkg/errors/blob/master/go113.go

@dhui
Copy link
Member

dhui commented Feb 22, 2021

The PR that added support for io/fs had build constraints but modules ignore built constraints. See: #480 (comment)

@ivanduka
Copy link

ivanduka commented Mar 9, 2021

Possible workaround until Go 1.15 is no longer supported:

main.go
migrations/
  1_create_tables.up.sql
  1_create_tables.down.sql
import "github.com/golang-migrate/migrate/v4/source/httpfs"

//go:embed migrations
var migrations embed.FS

source, err := httpfs.New(http.FS(migrations), "migrations")
// migrate.NewInstance("httpfs", source, ...)

Thank you so much!
Small correction: we need to use NewWithSourceInstance instead of NewInstance in migrate.NewInstance("httpfs", source, ...)

@zikaeroh
Copy link
Contributor

Go 1.16.2 and Go 1.15.10 (which were just released) include a fix for golang/go#44557, meaning that io/fs and embed can be depended on and not break downstream users when they go mod tidy (so long as they update their tooling to these latest patch releases), so this should be unblocked whenever you feel like those patch releases have been out long enough.

@vtolstov
Copy link

any chance to get embed support in golang-migrate ?

@dhui
Copy link
Member

dhui commented Mar 22, 2021

Let's wait a bit more time (about a month or two) for the newer versions of Go to be adopted. Your other option is to fork this library and merge the PR into your fork.

@alovak
Copy link

alovak commented May 8, 2021

Did we wait enough time? :)

@alovak
Copy link

alovak commented May 10, 2021

https://groups.google.com/g/golang-announce/c/cu9SP4eSXMc?pli=1

We have just released Go versions 1.16.4 and 1.15.12, minor point releases.
This minor release includes a security fix according to the new security policy (#44918).

1.15.12 is already here!

P.S. just to support my previous comment :)

@vtolstov
Copy link

i think that we wait enough. embed stuff not changed and not contains any bugs

@philippta
Copy link

What about using build tag for this, that restricts the compilation to Go 1.16 and onwards?

@dhui dhui closed this as completed in #560 May 11, 2021
@dhui
Copy link
Member

dhui commented May 11, 2021

Fixed by #560
This io/fs support is now available in the master branch but won't be available in a release until CI works for releases again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants