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

Naming clash for new cmp standard package #354

Open
Carrotman42 opened this issue Feb 9, 2024 · 5 comments
Open

Naming clash for new cmp standard package #354

Carrotman42 opened this issue Feb 9, 2024 · 5 comments

Comments

@Carrotman42
Copy link

I am not sure where the best place to have this discussion is, but I wanted to get @dsnet's opinion on this so this seemed like a reasonable place to start.

This is an extremely popular package and is imported into many Go tests across the world. Unfortunately, the new cmp package added within the last year has the same name as this package. I can see one possibly likely scenario where I would want to import both packages: in a unit test which wanted to use cmp.Diff along with a cmpopts.SortSlice which used cmp.Less[sometype]. There may be other situations as well.

The general solution to two packages having the same name is to rename one of them, but it isn't clear exactly which should be renamed: while it seems like a package in the stdlib is more central (leading to a preference to rename the third-party go-cmp), since go-cmp predates std/cmp by many years it will be a burden to retrain people with this new rename rule.

Does this package (or its maintainers) have an opinion about how to handle this naming clash?

PS: One out-of-the-box idea I had was to add forwarding code in go-cmp to all things in std/cmp, but I think that would cause people to import the heavier-weight go-cmp package into non-test code by accident so I am not certain that's a great idea.

@dsnet
Copy link
Collaborator

dsnet commented Feb 9, 2024

The name collision is unfortunate and was brought up during the proposal for the stdlib "cmp" package. The concern was dismissed as not being a problem: golang/go#59488 (comment)

Adding "go-cmp/cmp" to the stdlib has been proposed before, but rejected: golang/go#45200

I personally still find the name collision unfortunate, but don't see the benefit of just renaming "go-cmp/cmp" worth the cost in churn. If we were to do a v2 of the package or entirely move it elsewhere (e.g., the stdlib), there there might be enough benefits to warrant a rename as well.

\cc @neild if he has thoughts.

@neild
Copy link
Collaborator

neild commented Feb 9, 2024

The standard library cmp is obviously not being renamed.

We could rename github.com/google/go-cmp/cmp. But that's a lot of churn and confusion, for a questionable benefit.

I think the best thing to do for now is wait and see. Perhaps this isn't an issue at all, or is a minor one. We'll have more information in a year or two.

@daenney
Copy link

daenney commented Feb 28, 2024

I wonder how big of a problem that is in practice?

There's plenty of third party packages that clash with package names in stdlib too. Many libraries have their own errors package for example, that export the errors they use. Or a log package.

Any code base that's not already using stdlib's cmp can switch the import to gcmp "github.com/google/go-cmp" and now you can import cmp as normal. Or import stdlib cmp as stdcmp "cmp", though that one is bound to cause more confusion over time.

It may be possible to provide a go fix for the first approach that aliasses the go-cmp import?

@mxey
Copy link

mxey commented Apr 5, 2024

I agree on the churn. It would be nice to have an agreed upon suggested alias for the import, so people would clearly recognise it in other people's Go code.

@mrwormhole
Copy link

aliasing as gcmp or gocmp is the only solution forward, it is not that std cmp is extremely useful for every test file, it is very small addition to std lib so chances are slim that you will use both at the same time

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

6 participants