cmd/go: non-top-level packages ending in "/vendor" are omitted from module zip files
bep opened this issue · 10 comments
The following program fails in both go1.14rc1 and go1.13.7 darwin/amd64.
package main
import (
"fmt"
"github.com/bep/testmodlib"
"github.com/bep/testmodlib/somepackage"
"github.com/bep/testmodlib/somepackage/vendor"
"github.com/bep/testmodlib/somepackage/vendors"
)
func main() {
fmt.Println(testmodlib.Hello())
fmt.Println(somepackage.Hello())
fmt.Println(vendor.Hello())
fmt.Println(vendors.Hello())
}
Fails with:
build github.com/bep/temp: cannot load github.com/bep/testmodlib/somepackage/vendor: module github.com/bep/testmodlib@latest found (v1.0.1), but does not contain package github.com/bep/testmodlib/somepackage/vendor
If I add a replace directive for that module:
replace github.com/bep/testmodlib => /Users/bep/dev/go/bep/testmodlib
It compiles/runs fine:
testmodlib
somepackage
vendor package
vendors package
I understand that the vendor top level folder in a module has a special meaning, but according to https://golang.org/ref/spec#Packages that package name should be fine -- and just deleting it sounds rather harsh.
I assume this is related to #31562 -- I have not read that one in detail, but I don't see how the conclusion can be correct.
github.com/bep/testmodlib/somepackage/vendor
seem to be a valid package with a replace, and I assume that the compiler works as expected here.- Also, a module may contain non-Go folders (test resources etc. to make the test pass).
Change https://golang.org/cl/220640 mentions this issue: cmd/go/internal/modfetch: delete unused isVendoredPackage function
I assume this is related to #31562 -- I have not read that one in detail, but I don't see how the conclusion can be correct.
You are correct: this is closely related. https://play.golang.org/p/zCP7U8cKAGc
CC @jayconrod @matloob, @FiloSottile FYI.
(I don't think we can plausibly fix this without invalidating checksums, though.)
Change https://golang.org/cl/220657 mentions this issue: zip: document isVendoredPackage false-positives
I suppose we could gate this sort of changes on the go.mod
version?
@FiloSottile Unfortunately not. Old versions of cmd/go
should still work with go.mod
files that declare newer versions. So different versions of Go would produce different sums.
(I don't think we can plausibly fix this without invalidating checksums, though.)
To be more specific and confirm, do you mean without invalidating checksums of the affected modules; i.e., modules that have a non-top-level package with "/vendor" import path suffix that was unfortunately excluded from the .zip?
To be more specific and confirm, do you mean without invalidating checksums of the affected modules; i.e., modules that have a non-top-level package with "/vendor" import path suffix that was unfortunately excluded from the .zip?
Right. If we fixed this, then for a given repository at a given commit, we would include a different set of files in the module zip file. The zip file would have a different sum.
We don't have any safe mechanism for making that kind of change (though #30369, if implemented, is a possibility). The only change we can make is adding files that previously caused hard errors (for example, we could expand the set of allowed characters or increase the file size limit). We can't add files that were previously excluded.
The unfortunate label indicates this is a valid issue (though possibly fairly minor; given it affects a small percentage of possible package names) but there doesn't appear to exist any good way to make progress on resolving it.
(I don't think we can plausibly fix this without invalidating checksums, though.)
I suppose we could gate this sort of changes on the
go.mod
version?Unfortunately not. Old versions of
cmd/go
should still work withgo.mod
files that declare newer versions. So different versions of Go would produce different sums.
Perhaps it can work for supported versions of Go if we gate the change on a future go.mod
Go version, and teach all earlier Go releases to handle it. So by the time module versions start to declare the target go.mod
Go version 1.N, the supported Go 1.(N-1) and Go 1.N releases will know to do the right thing, and Go1.(N-2) that doesn't will be unsupported. Similar to how the // +build
→ //go:build
change was rolled out in multiple stages.
(I also considered gating on a future date, but that seems strictly worse.)
The overhead in such a change might be large enough that it'd be worth bundling multiple known module boundary improvements into one rather than doing it individually. Also, all future Go releases would need to implement both behaviours to be able to handle both old and new modules—so there are long-term costs in addition to transition costs.