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

Jump to bottom

cmd/go, x/tools/go/packages: handle os-specific command length limitations #54800

Open
findleyr opened this issue Aug 31, 2022 · 2 comments
Open

cmd/go, x/tools/go/packages: handle os-specific command length limitations #54800

findleyr opened this issue Aug 31, 2022 · 2 comments
Labels
NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@findleyr
Copy link
Contributor

While working on #54509, I encountered a surprising bug in go/packages: by renaming "golang.org/x/tools/internal/lsp" to "golang.org/x/tools/gopls/internal/lsp", it appears I pushed a go list invocation within our test setup past a windows limit for command line length:
https://storage.googleapis.com/go-build-log/0d6a7f9d/windows-amd64-2016_664249aa.log

(@adonovan found a reference citing a 32kb limit, which appears to be exceeded by the collective set of package directories in that test output).

Shortening our test module path resolved the failure (for now), but I expect we'll hit it again soon as we add more test data. Furthermore, our test data is certainly smaller than even medium-sized repositories, so it seems likely that our users are hitting this limit in practice.

Ideally, go/packages.Load should handle these limitations within the go list driver, perhaps by breaking up large queries.

CC @bcmills
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Aug 31, 2022
@gopherbot gopherbot added this to the Unreleased milestone Aug 31, 2022
@gopherbot
Copy link

Change https://go.dev/cl/426801 mentions this issue: internal/lsp/tests: use a shorter module path for test data

gopherbot pushed a commit to golang/tools that referenced this issue Aug 31, 2022
@findleyr
internal/lsp/tests: use a shorter module path for test data
550e1f5 
While moving internal/lsp to gopls/internal/lsp, we discovered that
we're bumping up against a command line length limit on windows. Use an
arbitrary shorter module path to avoid this, for now.

Updates golang/go#54800
Updates golang/go#54509

Change-Id: I7be07da29a769c1ce7c31cbbd374ca47b0944132
Reviewed-on: https://go-review.googlesource.com/c/tools/+/426801
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
@heschi heschi added the NeedsFix The path to resolution is known, but the work has not been done. label Sep 1, 2022
@findleyr
Copy link
Contributor Author

findleyr commented Sep 8, 2022

perhaps by breaking up large queries.

On second thought, I doubt this is feasible. While tools like gopls necessarily have to deal with the go command seeing a different version of the filesystem, it must be an invariant that the view of the filesystem observed by go list is consistent.

@findleyr findleyr changed the title x/tools/go/packages: handle os-specific command length limitations cmd/go, x/tools/go/packages: handle os-specific command length limitations Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsFix The path to resolution is known, but the work has not been done. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@gopherbot @heschi @findleyr