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

go vet generic type error and go test [build failed] on stmutil pkg #5

Closed
datewu opened this issue Oct 20, 2022 · 15 comments
Closed

go vet generic type error and go test [build failed] on stmutil pkg #5

datewu opened this issue Oct 20, 2022 · 15 comments

Comments

@datewu
Copy link

datewu commented Oct 20, 2022

Very strange, github ci marks go vet ./... and go test fails, while i'm pretty sure i change no stm/stmutil pkg dependency.

Then i found out go build works on my machine, but go vet ./... and go test fails like the github ci:

Both local machine and ci machine's go version are go 1.19.2.

# github.com/anacrolix/stm/stmutil
/Users/r/go/pkg/mod/github.com/anacrolix/stm@v0.4.0/stmutil/containers.go:72:3: cannot use generic type immutable.Map[K constraints.Ordered, V any] without instantiation
/Users/r/go/pkg/mod/github.com/anacrolix/stm@v0.4.0/stmutil/containers.go:76:9: cannot use Map{…} (value of type Map) as type Mappish in return statement:
        Map does not implement Mappish (missing Get method)
/Users/r/go/pkg/mod/github.com/anacrolix/stm@v0.4.0/stmutil/containers.go:76:30: type interhash of interhash{} does not match immutable.Hasher[K] (cannot infer K and V)
/Users/r/go/pkg/mod/github.com/anacrolix/stm@v0.4.0/stmutil/containers.go:79:17: cannot use Map{} (value of type Map) as type Mappish in variable declaration:
        Map does not implement Mappish (missing Get method)
/Users/r/go/pkg/mod/github.com/anacrolix/stm@v0.4.0/stmutil/containers.go:83:9: cannot use m (variable of type Map) as type Mappish in return statement:
        Map does not implement Mappish (missing Get method)
/Users/r/go/pkg/mod/github.com/anacrolix/stm@v0.4.0/stmutil/containers.go:88:9: cannot use m (variable of type Map) as type Mappish in return statement:
        Map does not implement Mappish (missing Get method)
/Users/r/go/pkg/mod/github.com/anacrolix/stm@v0.4.0/stmutil/containers.go:107:3: cannot use generic type immutable.SortedMap[K constraints.Ordered, V any] without instantiation
/Users/r/go/pkg/mod/github.com/anacrolix/stm@v0.4.0/stmutil/containers.go:112:9: cannot use sm (variable of type SortedMap) as type Mappish in return statement:
        SortedMap does not implement Mappish (missing Get method)
/Users/r/go/pkg/mod/github.com/anacrolix/stm@v0.4.0/stmutil/containers.go:117:9: cannot use sm (variable of type SortedMap) as type Mappish in return statement:
        SortedMap does not implement Mappish (missing Get method)
/Users/r/go/pkg/mod/github.com/anacrolix/stm@v0.4.0/stmutil/containers.go:182:14: cannot use generic type immutable.List[T comparable] without instantiation
/Users/r/go/pkg/mod/github.com/anacrolix/stm@v0.4.0/stmutil/containers.go:117:9: too many errors
# github.com/anacrolix/dht/v2/k-nearest-nodes
/Users/r/go/pkg/mod/github.com/anacrolix/dht/v2@v2.19.0/k-nearest-nodes/k-nearest-nodes.go.go:21:9: cannot use generic type immutable.SortedMap[K constraints.Ordered, V any] without instantiation
/Users/r/go/pkg/mod/github.com/anacrolix/dht/v2@v2.19.0/k-nearest-nodes/k-nearest-nodes.go.go:29:33: type lessComparer of lessComparer{…} does not match immutable.Comparer[K] (cannot infer K and V)

This github action may provide you more info.

@anacrolix
Copy link
Owner

That's very peculiar. I tried a bunch of theories and can't see why. I'm also not able to reproduce. I think I did see this problem early on with go1.19 before it was released.

@ucwong
Copy link
Contributor

ucwong commented Oct 25, 2022

@anacrolix #6

@anacrolix
Copy link
Owner

Turns out this issue is due to github.com/benbjohnson/immutable being upgraded to v0.4.0. Did you by any chance do go get -u from a downstream project? FWIW, go get -u doesn't do what people think it does for the most part, it forces dependencies of your dependencies to be updated, rather than just your immediate dependencies.

@ucwong
Copy link
Contributor

ucwong commented Oct 26, 2022

#7 @anacrolix

@anacrolix
Copy link
Owner

@ucwong here is an alternate line of fixes, let me know your thoughts: https://github.com/anacrolix/stm/tree/immutable-v0.4.0. Based on the change in constraints on map keys, I'm not sure updating to v0.4.0 is going to be plausible if this is still in use from anacrolix/dht. It certainly doesn't pass one of the tests that uses a set of *stm.Var.

# github.com/anacrolix/stm_test [github.com/anacrolix/stm.test]
./external_test.go:102:40: *stm.Var[bool] does not implement constraints.Ordered (*stm.Var[bool] missing in ~int | ~int8 | ~int16 | ~int32 | ~int64 | ~uint | ~uint8 | ~uint16 | ~uint32 | ~uint64 | ~uintptr | ~float32 | ~float64 | ~string)

@ucwong
Copy link
Contributor

ucwong commented Oct 26, 2022

@anacrolix I think update to latest version and follow the constrains on the Map like var define should be the reasonable way, if we can't abandon immutable dependency right now. And the future Golang version may be also strict in Map like vars defined. Issues about go get -u ./... seems not to be very graceful.

@anacrolix
Copy link
Owner

I don't quite understand. For now you may find it easiest to progress without updating immutable, until something is figured out.

@ucwong
Copy link
Contributor

ucwong commented Oct 26, 2022

I don't quite understand. For now you may find it easiest to progress without updating immutable, until something is figured out.

yes, just implement the missing func of interface used. Is this OK for merging #7

@anacrolix
Copy link
Owner

Are you able to pass all the tests in anacrolix/stm, with immutable v0.4.0, with your changes?

@ucwong
Copy link
Contributor

ucwong commented Oct 27, 2022

Are you able to pass all the tests in anacrolix/stm, with immutable v0.4.0, with your changes?

benbjohnson/immutable@v0.3.0...v0.4.0
I think it is not necessary to add strict constrains to a collections project like this, it should give more space to the users for convenience. Lock the version at 0.3.0 is enough.

@anacrolix
Copy link
Owner

I'm happy to take a PR that updates to v0.3.0 if you like (and ensuring it works with anacrolix/dht). I suspect you may want to undo some of the recent changes you made?

@ucwong
Copy link
Contributor

ucwong commented Oct 28, 2022

@anacrolix #8 revert pre-pr

@datewu
Copy link
Author

datewu commented Oct 28, 2022

Turns out this issue is due to github.com/benbjohnson/immutable being upgraded to v0.4.0. Did you by any chance do go get -u from a downstream project? FWIW, go get -u doesn't do what people think it does for the most part, it forces dependencies of your dependencies to be updated, rather than just your immediate dependencies.

Yes, i do go get -u ./... and i did it a lot. It's in my Makefile 🤣 .
After using replace directive replace github.com/benbjohnson/immutable v0.4.0 => github.com/benbjohnson/immutable v0.3.0 in my go.mod file, things work out. Both make and stmutil are happy.
Thanks again.

@anacrolix
Copy link
Owner

No worries, thanks @datewu. As I mentioned above, go get -u is a bit of an anti-pattern, but that's my strong opinion after being burned by it many times.

@anacrolix
Copy link
Owner

This should be fixed by 32174bf.

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

No branches or pull requests

3 participants