gomods / athens

A Go module datastore and proxy
https://docs.gomods.io
MIT License
4.41k stars 497 forks source link

should we error out for non-semver for @mod and @zip? #1318

Open xyt0056 opened 5 years ago

xyt0056 commented 5 years ago

Describe the bug currently we filter out non-semver versions in @list, but if user specifically asked for it, we return it in @info @mod @zip. However, it's not the case in proxy.golang.org, which return 200 for @info, 400 for @mod and @zip. Should we follow?

https://proxy.golang.org/github.com/google/gvisor/@v/release-20190529.1.mod http://localhost:3000/github.com/google/gvisor/@v/release-20190529.1.mod

xyt0056 commented 5 years ago

one thing to note is, currently athens populate .info, .mod, .zip at the same time upon either one of the calls. In order to achieve the same behavior as proxy.golang.org, we need to separate fetcher logic

arschles commented 5 years ago

@xytan123 Do you have a use case for adding this filter? We could add it as a config option if needed.

For context, we want to allow non-semver versions only if someone already had them specified in their go.mod because that could be helpful for folks who have non-semver release processes internally. We wanted to prevent folks from picking up a new non-semver release when they do a go get without a version explicitly specified in the package.

xyt0056 commented 5 years ago

@arschles I agree on user experience requirements. From what I tested, go get only needs @info to get the correct modules, but not @mod, @zip.

currently proxy.golang.org return 410 to malformed version for @mod, @zip. But go get resolves correctly

GOPROXY="direct" GO111MODULE=on go get github.com/google/gvisor@release-20190529.1
go: finding github.com/google/gvisor release-20190529.1
go: finding github.com/google/gvisor latest
diff --git a/go.mod b/go.mod
index 14e7108cc..2dcf34d28 100644
--- a/go.mod
+++ b/go.mod
@@ -29,6 +29,7 @@ require (
        github.com/golang/protobuf v1.3.1
        github.com/golang/snappy v0.0.1 // indirect
        github.com/google/go-cmp v0.3.0
+       github.com/google/gvisor v0.0.0-20190517204740-4a842836e560 // indirect

I think we need to be on par with proxy.golang.org just in case go toolchain use them elsewhere for validation

arschles commented 4 years ago

Sorry for the long delay @xytan123! I'm catching up on this issue, so I apologize for repeating what we already. said above.

So you are saying that Athens should return 200 and valid JSON on the .info endpoint, but 410 for .mod and .zip, correct? Is that to make sure Athens is in line with what proxy.golang.org, or do you have more reasons?

xyt0056 commented 4 years ago

@arschles yep. To make it as close to google's proxy as possible

arschles commented 4 years ago

@xyt0056 we can certainly do that. I agree that it would be nice to follow the Google proxy as much as possible.

as you said, we'll have to change the fetch logic. I'd also like to put this behind a config flag. Would you be up for opening a pull request for splitting up the fetching logic?