multiformats / go-multiaddr

Composable and future-proof network addresses
https://github.com/multiformats/multiaddr
MIT License
271 stars 109 forks source link

breaking changes to 'slices.SortFunc' #208

Closed KEINOS closed 1 year ago

KEINOS commented 1 year ago

A breaking change have been made to slices package in golang.org/x/exp module (slices.SortFunc in particular).

slices: update to current standard library version Update x/exp/slices to the current standard library slices package, while retaining the ability to use it with Go 1.18 through Go 1.20.

Note that this changes some of the sorting functions to use a comparison function rather than a less function. We don't promise backward compatibility in x/exp packages. Being compatible with the Go 1.21 package seems more useful for people not yet using 1.21, as it will make the transition to 1.21 easier.

It is the comparison function, an argument of slices.SortFunc, that affects our package. func(a, b E) bool vs func(a, b E) int.

  - func SortFunc[E any](x []E, less func(a, b E) bool) {
  + func SortFunc[S ~[]E, E any](x S, cmp func(a, b E) int) {

Currently there are no problems, but when updating dependent modules the following error occurs:

How to reproduce error

$ cat version.json
{
  "version": "v0.10.1"
}

$ go test ./...
?       github.com/multiformats/go-multiaddr/multiaddr  [no test files]
ok      github.com/multiformats/go-multiaddr    0.262s
ok      github.com/multiformats/go-multiaddr/net        0.533s

$ go get -u golang.org/x/exp
go: upgraded golang.org/x/exp v0.0.0-20230626212559-97b1e661b5df => v0.0.0-20230725093048-515e97ebf090

$ go test ./...
# github.com/multiformats/go-multiaddr
./multiaddr.go:223:25: type func(a Multiaddr, b Multiaddr) bool of func(a, b Multiaddr) bool {…} does not match inferred type func(a Multiaddr, b Multiaddr) int for func(a E, b E) int
FAIL    github.com/multiformats/go-multiaddr [build failed]
FAIL    github.com/multiformats/go-multiaddr/net [build failed]
FAIL

Proposed fix

https://github.com/multiformats/go-multiaddr/blob/07e6e8da9164552c482f6dd4bc95c7477915b528/multiaddr.go#L223

  - slices.SortFunc(addrs, func(a, b Multiaddr) bool { return bytes.Compare(a.Bytes(), b.Bytes()) < 0 })
  + slices.SortFunc(addrs, func(a, b Multiaddr) int { return bytes.Compare(a.Bytes(), b.Bytes()) })

Note

Even if all other modules are updated, no further changes seem to be necessary at this stage.

  $ go get -u ./...
  go: upgraded github.com/ipfs/go-cid v0.0.7 => v0.4.1
  go: added github.com/klauspost/cpuid/v2 v2.2.5
  go: upgraded github.com/minio/sha256-simd v0.1.1-0.20190913151208-6de447530771 => v1.0.1
  go: upgraded github.com/mr-tron/base58 v1.1.3 => v1.2.0
  go: upgraded github.com/multiformats/go-base32 v0.0.3 => v0.1.0
  go: upgraded github.com/multiformats/go-base36 v0.1.0 => v0.2.0
  go: upgraded github.com/multiformats/go-multibase v0.0.3 => v0.2.0
  go: upgraded github.com/multiformats/go-multihash v0.0.14 => v0.2.3
  go: upgraded github.com/multiformats/go-varint v0.0.6 => v0.0.7
  go: upgraded golang.org/x/crypto v0.1.0 => v0.11.0
  go: upgraded golang.org/x/exp v0.0.0-20230626212559-97b1e661b5df => v0.0.0-20230725093048-515e97ebf090
  go: upgraded golang.org/x/sys v0.1.0 => v0.10.0
  go: added lukechampine.com/blake3 v1.2.1

  $ go test ./...
  ?       github.com/multiformats/go-multiaddr/multiaddr  [no test files]
  ok      github.com/multiformats/go-multiaddr    0.727s
  ok      github.com/multiformats/go-multiaddr/net        0.567s

Env info

$ go version
go version go1.20.6 darwin/amd64

$ # go-multiaddr ver
$ cat version.json
{
  "version": "v0.10.1"
}
KEINOS commented 1 year ago
marten-seemann commented 1 year ago

This has been resolved.