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

x/pkgsite: invalid versions for subdirectories #51720

Closed
stevenh opened this issue Mar 16, 2022 · 10 comments
Closed

x/pkgsite: invalid versions for subdirectories #51720

stevenh opened this issue Mar 16, 2022 · 10 comments
Labels
FrozenDueToAge pkgsite WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@stevenh
Copy link
Contributor

stevenh commented Mar 16, 2022

What is the URL of the page with the issue?

https://pkg.go.dev/github.com/gomodule/redigo/redis

What is your user agent?

Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/99.0.4844.51 Safari/537.36

Screenshot

First go to the URL directly
image
Second clicks the sub directory link from the main repo, which shows the latest link in red indicating the link from latest gomodules/redigo doesnt link to the latest gomodules/redigo/redis which should be the same.
image

Versions aren't listed in the sub directory:
image

What did you do?

Two flows:

Issue 1

  1. Go to the redis main page with no version: https://pkg.go.dev/github.com/gomodule/redigo/redis
  2. See that the version is: v0.0.0-...-e14091d instead of 1.8.8 which is currently the latest.

Issue 2

  1. Go to to the page repo root: https://pkg.go.dev/github.com/gomodule/redigo
  2. Click Directories -> redis link
  3. See the second image above where the "Go to Latest" button is red and shouldn't be

What did you expect to see?

Sub directories in repos use the latest version by default.

What did you see instead?

Sub directories doesn't even list any version

Relevant information

We redigo have an experimental v2.0.0 branch before go modules we're introduced which has been challenging. With the introduction of retract in v1.16 we recently retracted this version in this commit so wondering if this is an edge case which pkgsite isn't dealing with?

Thanks for pabigot for raising this gomodule/redigo#602

@gopherbot gopherbot added this to the pkgsite/unplanned milestone Mar 16, 2022
@seankhliao
Copy link
Member

go treats github.com/gomodule/redigo/redis and github.com/gomodule/redigo as 2 independent modules. A retraction for github.com/gomodule/redigo/redis has to happen in that module and cover all previous versions (including plain commits) as well as the version doing the retracting (blog post if it helps).

The second issue is working as intended, go modules match to the longest module name.

@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 16, 2022
@stevenh
Copy link
Contributor Author

stevenh commented Mar 16, 2022

Thanks for the quick response @seankhliao :)

To clarify github.com/gomodule/redigo/redis was never a separate module, the blog post seems to deal with the case it was but this is not the case.

As far as I can see all other go tools seem to work as expected. So for example if I use github.com/gomodule/redigo/redis in a package and run go mod tidy it knows that the latest version is v1.8.1 the same for go get

go mod tidy
go: finding module for package github.com/gomodule/redigo/redis
go: found github.com/gomodule/redigo/redis in github.com/gomodule/redigo v1.8.8
go get -v github.com/gomodule/redigo/redis@latest
go: added github.com/gomodule/redigo v1.8.8

Prior to adding the retract to the go.mod this wasn't the case and it would pick up a v2.0.0+incompatible.

So unless I'm missing something, it seems like x/pkgsite is the only part of the ecosystem which doesn't correctly follow the chain and result in the correct versions?

The second issue is working as intended, go modules match to the longest module name.

To clarify when you say second issue do you mean the versions not listing correctly?

If so this isn't how something like testify/require is working and hence shows the versions associate with parent.

@seankhliao
Copy link
Member

The redis submodule certainly existed at some point: https://github.com/gomodule/redigo/blob/e14091dffc1b085ace903ff7c41916e17c2daca3/redis/go.mod
(Clicking "valid go.mod file on the pkgsite sidebar will take you there).
go tooling chooses the correct one because 1. tagged overrides plain commits, and 2. it checks for the existence of a matching package (the redis module at the tagged version is empty).
Users only need to use a single version, so that works for them, but pkgsite needs to work with all versions (past, present, and future), so it can't follow the same logic. If a module exists/existed without being retracted, it will be served.

The only way to fix this is to properly retract the redis module, serving a module without files like the v0.0.0-do-not-use you have now isn't the correct fix.

@stevenh
Copy link
Contributor Author

stevenh commented Mar 17, 2022

Thanks for the clarification on why pkgsite is special, makes sense.

The blog article details how to retract a module which has been tagged, but not clear how to do the same for one that has never been tagged.

I thought I understood your statement:

The only way to fix this is to properly retract the redis module, serving a module without files like the v0.0.0-do-not-use you have now isn't the correct fix.

To mean that we would need a redis/go.mod and redisx/go.mod with:

retract v0.0.0-do-not-use // Never used only present to fix pkg.go.dev.

But re-reading I'm not sure this was your intention?

@pabigot
Copy link

pabigot commented Mar 17, 2022

I can't speak to the right way to do this, but I thought that "you can file a request for the pkgsite team to remove your package" from https://pkg.go.dev/about#removing-a-package might be a workable approach? gomodule/redigo#604 kinda scares me.

@seankhliao
Copy link
Member

you would need:

// redis/go.mod
module github.com/gomodule/redigo/redis

retract [v0.0.0-0, v0.0.1]

and tag it as redis/v0.0.1.

  • you need to cover any plain commits with the go.mod file
  • v0.0.0-do-not-use already existed/exists, and you need a version that counts as newer (by semver).

The next commit has to be to remove the file (unless you do it on some other branch and just leave it there).

Removing a module by request will likely impact the module you want to keep as it shares a path.

@stevenh
Copy link
Contributor Author

stevenh commented Mar 17, 2022

Thanks again @seankhliao I've prepared a PR that includes the changes you suggested.

Note that no go.sum files present, would appreciate a review to ensure everything looks good?

As @pabigot mentioned I'm not totally comfortable with this as if it doesn't work or creates any unintended impact, I don't know a way to fix as the tag v0.0.1 will exist, which isn't ideal.

@seankhliao
Copy link
Member

The tag only needs to exist long enough for the proxy to index/cache it. If you do it wrong, bump up the version and try again, the retractions at the highest version are the ones take effect.

@stevenh
Copy link
Contributor Author

stevenh commented Mar 18, 2022

Is there a way to force the proxy to index/cache it?

@stevenh
Copy link
Contributor Author

stevenh commented Mar 18, 2022

For those which might find this issue I managed to trigger the index to cache it by manually visiting:
https://pkg.go.dev/github.com/gomodule/redigo/redis@v0.0.1
https://pkg.go.dev/github.com/gomodule/redigo/redisx@v0.0.1

Thanks @seankhliao for the help in figuring out how to fix.

@stevenh stevenh closed this as completed Mar 18, 2022
@golang golang locked and limited conversation to collaborators Mar 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge pkgsite WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants