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

Accidental deletion of modules go.mod files causing ambiguous imports #796

Open
nesty92 opened this issue Mar 11, 2025 · 7 comments · May be fixed by #797
Open

Accidental deletion of modules go.mod files causing ambiguous imports #796

nesty92 opened this issue Mar 11, 2025 · 7 comments · May be fixed by #797

Comments

@nesty92
Copy link
Contributor

nesty92 commented Mar 11, 2025

There were two incidents where module go.mod files were accidentally deleted:

  1. rueidislimiter go.mod was deleted in commit 34c5717
  2. rueidisaside go.mod was deleted in commit 87327f9

RueidisAside:
https://go.dev/play/p/iC7P-EDfbff

RueidisLimiter:
https://go.dev/play/p/KTVW5CW_2XJ

Current Status

  • rueidislimiter has been fixed after perf: reduce allocations in rueidislimiter #795, but a new release is still needed
  • rueidisaside still needs its go.mod restored
  • rueidislock was never a module in self so it's fine but maybe it should be
@rueian
Copy link
Collaborator

rueian commented Mar 11, 2025

Well, the removals are actually intended, so that users don’t need additional go get to use them.

@nesty92
Copy link
Contributor Author

nesty92 commented Mar 11, 2025

The current module setup creates broken behavior in our examples. Specifically, users must first fetch the rueidis module before trying to use rueidisaside or rueidislimiter, which then indicates that no additional fetch is needed. This requirement disrupts the expected workflow. like in the go.dev examples.

Having it as a module will respect the previous behavior and having an additional go mod tidy or go get is not a big deal. Once a module is published it should not be removed or it will cause this problem

If this is the intended behavior then we must remove the separate go.mod file in rueidislimiter before the next release

@rueian
Copy link
Collaborator

rueian commented Mar 11, 2025

like in the go.dev examples.

Which ones? Let's update those examples.

If this is the intended behavior then we must remove the separate go.mod file in rueidislimiter before the next release

I prefer no additional go.mod files, but you added tests with gomock, so I think additional go.mod is fine.

Once a module is published it should not be removed or it will cause this problem

Will there be problems other than an additional go mod tidy or go get?

@nesty92
Copy link
Contributor Author

nesty92 commented Mar 11, 2025

Try to run the go.dev examples in the description

https://go.dev/play/p/KTVW5CW_2XJ
https://go.dev/play/p/iC7P-EDfbff

You have to run it first with only rueidis being used to them be able to add the adapter or the limiter if you try to add both on a new project it fails

go: finding module for package github.com/redis/rueidis/rueidislimiter
go: finding module for package github.com/redis/rueidis
go: downloading github.com/redis/rueidis v1.0.55
go: downloading github.com/redis/rueidis/rueidislimiter v1.0.53
go: found github.com/redis/rueidis in github.com/redis/rueidis v1.0.55
go: found github.com/redis/rueidis/rueidislimiter in github.com/redis/rueidis/rueidislimiter v1.0.53
go: downloading golang.org/x/sys v0.30.0
prog.go:9:2: ambiguous import: found package github.com/redis/rueidis/rueidislimiter in multiple modules:
	github.com/redis/rueidis v1.0.55 (/tmp/gopath3632442833/pkg/mod/github.com/redis/rueidis@v1.0.55/rueidislimiter)
	github.com/redis/rueidis/rueidislimiter v1.0.53 (/tmp/gopath3632442833/pkg/mod/github.com/redis/rueidis/rueidislimiter@v1.0.53)

Go build failed.

@rueian
Copy link
Collaborator

rueian commented Mar 11, 2025

Hmm, I just tried the example, but what I only needed to do was go get github.com/redis/rueidis. It didn't show ambiguous import: found package github.com/redis/rueidis/rueidislimiter.

Is that because you already used the old rueidis in your project?

@nesty92
Copy link
Contributor Author

nesty92 commented Mar 11, 2025

go get github.com/redis/rueidis

in that case it works because you try to load the rueidis first that is the case I mentioned before, try this with the main.go from the example

> go mod init test
go: creating new go.mod: module test
go: to add module requirements and sums:
        go mod tidy
> go mod tidy
go: finding module for package github.com/redis/rueidis/rueidisaside
go: finding module for package github.com/redis/rueidis
go: found github.com/redis/rueidis in github.com/redis/rueidis v1.0.55
go: found github.com/redis/rueidis/rueidisaside in github.com/redis/rueidis/rueidisaside v1.0.39
go: test imports
        github.com/redis/rueidis/rueidisaside: ambiguous import: found package github.com/redis/rueidis/rueidisaside in multiple modules:
        github.com/redis/rueidis v1.0.55 (/home/nesty/go/pkg/mod/github.com/redis/rueidis@v1.0.55/rueidisaside)
        github.com/redis/rueidis/rueidisaside v1.0.39 (/home/nesty/go/pkg/mod/github.com/redis/rueidis/rueidisaside@v1.0.39)

@rueian
Copy link
Collaborator

rueian commented Mar 11, 2025

Ok, let’s add go.mod back.

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.

2 participants