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/exp/cmd/gorelease: module source tree too large #40263

Closed
carnott-snap opened this issue Jul 17, 2020 · 16 comments
Closed

x/exp/cmd/gorelease: module source tree too large #40263

carnott-snap opened this issue Jul 17, 2020 · 16 comments
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@carnott-snap
Copy link

carnott-snap commented Jul 17, 2020

What version of Go are you using (go version)?

$ go version
go version go1.14.6 linux/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOENV="/home/user/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/user/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/user/.local/share/umake/go/go-lang"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/user/.local/share/umake/go/go-lang/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/user/path/to/module/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build965764173=/tmp/go-build -gno-record-gcc-switches"

What did you do?

gorelease

What did you expect to see?

Suggested version: v1.2.3

What did you see instead?

gorelease: create zip /home/user/path/to/module: module source tree too large (max size is 524288000 bytes)
@gopherbot gopherbot added this to the Unreleased milestone Jul 17, 2020
@dmitshur
Copy link
Contributor

Is this a regression?

What are the contents and the size of the directory where you ran gorelease?

/cc @jayconrod

@dmitshur dmitshur added modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Jul 17, 2020
@hyangah
Copy link
Contributor

hyangah commented Jul 17, 2020

https://github.com/golang/mod/blob/89ce4c7ba8043b0e8a8c03f1a266fb0e56637314/zip/zip.go#L28-L30
This is the limit used by the go command.

@carnott-snap
Copy link
Author

carnott-snap commented Jul 17, 2020

The reproducer is an internal repo, so I am not sure I can share it. That being said, it does not reproduce on other modules or with an older version of gorelease, CL 197299/4.

@jayconrod
Copy link
Contributor

Is the source tree actually that large? Or can this be reproduced with a smaller tree? If so, how?

The go command limits modules to 500 MiB. It's important that gorelease reports that error to warn developers against tagging a release that others won't be able to download.

@carnott-snap
Copy link
Author

Considering that gorelease@master, but not go build, go test, go mod tidy or old gorelease, fails during ci, I strongly suspect that there is a bug in gorelease. Because of the proprietary nature of the source that triggers this, let me know if there is debugging information that I can provide.

@dmitshur
Copy link
Contributor

dmitshur commented Jul 17, 2020

it does not reproduce on other modules or with an older version of gorelease, CL 197299/4.

In the diff between patch set 4 of CL 197299 and the final patch set 15 that was merged, the go.mod went from requiring golang.org/x/mod at v0.1.0 to v0.1.1-0.20191107180719-034126e5016b. That change implemented the check that "total size in bytes of a module zip file may be at most MaxZipFile bytes (500 MiB)" that @hyangah pointed out above.

@carnott-snap
Copy link
Author

Awesome thanks for the bisect!. Is this supposed to break for go build, and other toolchain commands, or is there some restriction that only applies to gorelease/golang.org/x/mod and if so, why?

@jayconrod
Copy link
Contributor

When downloading a module go build and other commands will report an error if the size of the module zip file is over 500 MiB or if the total size of the files within the zip are over 500 MiB.

go build and other commands won't report an error if the main module exceeds that size though. There's nothing wrong with that. But gorelease does need to report an error.

For debugging, can you confirm the size of files within the module? I can't give a good one-liner to do that: du will help but make sure to exclude .git, vendor, and any nested modules, and don't follow symlinks. zip.CreateFromDir is the function that gorelease uses, so you could try that.

@carnott-snap
Copy link
Author

carnott-snap commented Jul 17, 2020

Found the issue, the module is for a library that is polyglottal and node_modules is blowing through our quota. It is not committed, and really should not be part of the module, so I will change some names to use _s. Thanks a bunch. May be worth having a more descriptive error or writing up the problem in a blog post somewhere when go release comes out?

@jayconrod
Copy link
Contributor

A more descriptive error would be good. Or a pointer to a more complete description (#37414).

I realize from this discussion that it's not at all obvious why gorelease would be doing anything with zip files.

@dmitshur
Copy link
Contributor

dmitshur commented Jul 17, 2020

It is not committed

If a very large file or directory is not committed, it would not be a real problem, so gorelease reporting that is a false positive as far as I understand. Is there already an issue tracking whether improving that is feasible?

Edit: On the other hand, it may be a real problem if the code requires that file to be present in the module, so things work during local testing, but not after the module is published. But I don't know for sure if detecting that type of issue is in scope for gorelease.

@carnott-snap
Copy link
Author

carnott-snap commented Jul 17, 2020

I concur, in general I think the error is warranted, just a little confusing. I was including non Go artefacts in a module, so that issue is mine.

Relatedly, I moved all NodeJS code in the repo to _js, .js, and testdata, but none had an effect, is there something I am missing wrt excluding folders from modules? Do I need to drop a submodule to exclude this directory?

@jayconrod
Copy link
Contributor

If a very large file or directory is not committed, it would not be a real problem, so gorelease reporting that is a false positive as far as I understand. Is there already an issue tracking whether improving that is feasible?

gorelease mostly just looks at the source tree and ignores the VCS repository. It could be changed to report an error when there are pending changes.

Relatedly, I moved all NodeJS code in the repo to _js, .js, and testdata, but none had an effect, is there something I am missing wrt excluding folders from modules? Do I need to drop a submodule to exclude this directory?

Yes, that would probably be the easiest thing. _js, .js, and testdata would all be included in the module zip file.

@carnott-snap
Copy link
Author

That is more than a little ungainly, but _js is not a boon for readability either. Do we have this requirement documented somewhere? Clearly I was working off bad information about what the build toolchain ignores, so it would be nice to point to something authoritative.

@jayconrod
Copy link
Contributor

Module zip files describes this restriction and others. I can't claim it's authoritative yet (still in draft state), but it will be in the future when it's complete.

@carnott-snap
Copy link
Author

sgtm, I think using #37414 to track the new glossary code is fine, so I will close this out.

@golang golang locked and limited conversation to collaborators Jul 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

5 participants