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

gob.Register panics on model types conflict #51

Closed
bcho opened this issue Aug 4, 2023 · 0 comments · Fixed by #52
Closed

gob.Register panics on model types conflict #51

bcho opened this issue Aug 4, 2023 · 0 comments · Fixed by #52

Comments

@bcho
Copy link
Contributor

bcho commented Aug 4, 2023

Actual behavior A clear and concise description of what the bug is.

When running this uber mockgen in a codebase with reference to the golang mockgen, the reflect program would panic on the gob.Register call:

panic: gob: registering duplicate types for "*model.ArrayType": *model.ArrayType != *model.ArrayType

goroutine 1 [running]:
encoding/gob.RegisterName({0x10fad4f, 0x10}, {0x110b460, 0xc000010468})
        /usr/local/go/src/encoding/gob/type.go:820 +0x3f4
encoding/gob.Register({0x110b460, 0xc000010468})
        /usr/local/go/src/encoding/gob/type.go:874 +0x1c5
github.com/golang/mock/mockgen/model.init.0()
        $HOME/workshop/go/pkg/mod/github.com/golang/[email protected]/mockgen/model/model.go:146 +0x39
panic: gob: registering duplicate types for "*model.ArrayType": *model.ArrayType != *model.ArrayType
...

Expected behavior A clear and concise description of what you expected to
happen.

This is because in our code base (or similar code base created before go mod area) contains import path like this:

_ "github.com/golang/mock/mockgen/model"

This call will register the model types with the same name, and result in gob panic. This import call is suggested by golang mockgen as a workaround for vendor usage:

https://github.com/golang/mock#reflect-vendoring-error

I believe this would be a blocker for migrating from golang mockgen as upstream libraries might have such import path, and it would take time to migrate all these upstream libraries as it requires multiple steps.

Therefore, I would like to propose a fix to register these model types with a prefix.

To Reproduce Steps to reproduce the behavior

To produce this issue, please check out this commit: https://github.com/bcho/gomock-migrate/tree/4148048b8a313fa0be6164d1cdcdc941e523c0c1

I have a fix poc in this commit: https://github.com/bcho/gomock-migrate/tree/2de73982ee899e4e0a3986a021b584c08feb3e5f , which updated the uber gomock to use the fixed model in my local fork.

Additional Information

  • gomock mode (reflect or source): reflect
  • gomock version or git ref: 0.2.0
  • golang version: go version go1.20.2 darwin/amd64

Triage Notes for the Maintainers

@bcho bcho changed the title gob.Register panic on model types conflict gob.Register panics on model types conflict Aug 4, 2023
sywhang pushed a commit that referenced this issue Aug 12, 2023
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.

1 participant