Test via Github actions. #140
Conversation
And remove travis configuration.
|
Let's try |
|
I don't think it should be blocking this from being merged but maybe we could also setup unified CI for this repo? I could look into it next week. |
|
Unfortunately, this is going to be annoying. We have no guarantees that any of the binaries will be forwards compatible with new versions of Go (this is primarily due to quic-go's reliance on unsafely targeting Go's TLS libraries, although there could be others). Ideally migrations should be written such that we don't need to take a dependency on any of those libraries, but I think that'll take a bit of refactoring such that go-ipfs datastore plugins don't require importing go-ipfs (maybe the Go 1.17 module pruning changes will help us out here). This makes unified CI not really a great candidate here (some things we'll probably want to copy, but others likely not). We also will be having submodules here which may not work nicely with the unified CI release workflows (not sure though). We may end up needing to have a mapping of migration versions to versions of Go (e.g. everything up until 10-11 gets 1.15, 10-11 gets 1.15+1.16, 11-12 gets 1.16+1.17) and running them independently. At some point Actions will probably stop supporting certain versions of Go and that's fine we can drop support, it's pretty rare (although it does occasionally happen) that someone will report a bug in an old migration. If handling this correctly is too painful right now, we can merge this PR targeting Go 1.16 and then @galargh can resolve this later. At least then we'd have some CI running here. |
|
The main things seems to be: I'm going to try with 1.16 and see if that makes anything. We could potentially create separate jobs for separate migrations, each of them using a working go-version. |
Submodules, if I understand it correctly, are not a problem because we use https://github.com/protocol/multiple-go-modules with all relevant go commands.
This sounds much more tricky and I feel it might be out of scope for unified CI - my intuition tells me it might be special enough of a case to just tackle it here rather than generalising.
I strongly agree with this :) This is the first failure I can see from a run with Go 1.16: |
|
@galargh it seems like that might've just been a temporary CI issue, things are passing now. As discussed we'll merge and then figure out the CI testing situation across multiple Go versions later. |
|
Thanks @hsanjuan 🙏 |
|
They are not passing? |
https://github.com/ipfs/fs-repo-migrations/releases/tag/v2.0.2 This is pretty much a complete rewrite of the ipfs-migrator package. In version 2.0.0 a major change was made to the way the migrator works. Before, there was one binary that contained every migration. Now every migration has its own binary. If fs-repo-migrations can't find a required binary in the PATH, it will download it off the internet. To prevent that, build every migration individually, symlink them all into one package and then wrap fs-repo-migrations so it finds the package with all the migrations. The change to the IPFS NixOS module and the IPFS package is needed because without explicitly specifying a repo version to migrate to, fs-repo-migrations will query the internet to find the latest version. This fails in the sandbox, for example when testing the ipfs passthru tests. While it may seem like the repoVersion and IPFS version are in sync and the code could be simplified, this is not the case. See https://github.com/ipfs/fs-repo-migrations#when-should-i-migrate for a table with the IPFS versions and corresponding repo versions. Go 1.17 breaks the migrations, so use Go 1.16 instead. This is also the Go version used in their CI, see https://github.com/ipfs/fs-repo-migrations/blob/3dc218e3006adac25e1cb5e969d7c9d961f15ddd/.github/workflows/test.yml#L4. See ipfs/fs-repo-migrations#140 (comment) for a previous mention of this issue. The issue manifests itself when doing anything with a migration, for example `fs-repo-11-to-12 --help`: ``` panic: qtls.ClientHelloInfo doesn't match goroutine 1 [running]: github.com/marten-seemann/qtls-go1-15.init.0() github.com/marten-seemann/[email protected]/unsafe.go:20 +0x132 ``` Also add myself as a maintainer for this package. This fixes the test failure discovered in NixOS#160914. See ipfs/fs-repo-migrations#148 to read some of my struggles with updating this package.
https://github.com/ipfs/fs-repo-migrations/releases/tag/v2.0.2 This is pretty much a complete rewrite of the ipfs-migrator package. In version 2.0.0 a major change was made to the way the migrator works. Before, there was one binary that contained every migration. Now every migration has its own binary. If fs-repo-migrations can't find a required binary in the PATH, it will download it off the internet. To prevent that, build every migration individually, symlink them all into one package and then wrap fs-repo-migrations so it finds the package with all the migrations. The change to the IPFS NixOS module and the IPFS package is needed because without explicitly specifying a repo version to migrate to, fs-repo-migrations will query the internet to find the latest version. This fails in the sandbox, for example when testing the ipfs passthru tests. While it may seem like the repoVersion and IPFS version are in sync and the code could be simplified, this is not the case. See https://github.com/ipfs/fs-repo-migrations#when-should-i-migrate for a table with the IPFS versions and corresponding repo versions. Go 1.17 breaks the migrations, so use Go 1.16 instead. This is also the Go version used in their CI, see https://github.com/ipfs/fs-repo-migrations/blob/3dc218e3006adac25e1cb5e969d7c9d961f15ddd/.github/workflows/test.yml#L4. See ipfs/fs-repo-migrations#140 (comment) for a previous mention of this issue. The issue manifests itself when doing anything with a migration, for example `fs-repo-11-to-12 --help`: ``` panic: qtls.ClientHelloInfo doesn't match goroutine 1 [running]: github.com/marten-seemann/qtls-go1-15.init.0() github.com/marten-seemann/[email protected]/unsafe.go:20 +0x132 ``` Also add myself as a maintainer for this package. This fixes the test failure discovered in #160914. See ipfs/fs-repo-migrations#148 to read some of my struggles with updating this package.
And remove travis configuration.