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/tools/gopls: consider to turn off buildvcs inside gopls #51999

Open
hyangah opened this issue Mar 29, 2022 · 5 comments
Open

x/tools/gopls: consider to turn off buildvcs inside gopls #51999

hyangah opened this issue Mar 29, 2022 · 5 comments
Labels
FeatureRequest gopls/metadata gopls NeedsInvestigation Tools
Milestone

Comments

@hyangah
Copy link
Contributor

@hyangah hyangah commented Mar 29, 2022

gopls version

golang.org/x/tools/gopls v0.8.1
compiled with go1.18 (google-golang)

What did you do?

We have a project that has a third-party go project as its git submodule.

my_project
  +--- go.work
  +--- their_project
               +--- go.mod
               +--- main.go

go.work has only ./their_project in it.

I opened vscode from my_project , so gopls sees my_project as the workspace root.

What did you expect to see?

Working normally.

What did you see instead?

All go files in the their_project directory get errors with source go list.
gopls log:

[Trace - 23:38:54.242 PM] Received notification 'textDocument/publishDiagnostics'.
Params: {"uri":"file:///home/hakim/projects/my_project/their_project/cmd/tester/main.go",
"diagnostics":[{"range":{"start":{"line":0,"character":0},"end":{"line":0,"character":0}},
"severity":1,
"source":"go list",
"message":"error obtaining VCS status: main package is in repository 
\"/home/hakim/projects/my_project/their_project\" but current directory is in repository 
\"/home/hakim/projects/my_project\"\n\tUse -buildvcs=false to disable VCS stamping."}]}

I understand this is an error message from go, and I can work around by disabling the buildvcs flag.

However, what's puzzling is, if I run go list from their_project, it runs without an error.

$ go list ./triage-party/...
error obtaining VCS status: main package is in repository "/home/hakim/projects/my_project/their_project" but current directory is in repository "/home/hakim/projects/my_project"
        Use -buildvcs=false to disable VCS stamping.
error obtaining VCS status: main package is in repository "/home/hakim/projects/my_project/their_project" but current directory is in repository "/home/hakim/projects/my_project"
        Use -buildvcs=false to disable VCS stamping.

$ cd ./triage-party; go list ./...
go list ./...
github.com/they/their_project/cmd/server
...        

I don't know what's right default behavior for the go command (this is probably a niche use case) cc @bcmills

but these confusing error messages make me think gopls disable vcs build stamping by default when running go commands.
(why does gopls have to care about build stamping?)

@gopherbot gopherbot added Tools gopls labels Mar 29, 2022
@gopherbot gopherbot added this to the Unreleased milestone Mar 29, 2022
@hyangah hyangah added FeatureRequest NeedsInvestigation labels Mar 29, 2022
@hyangah hyangah removed this from the Unreleased milestone Apr 1, 2022
@hyangah hyangah added this to the gopls/on-deck milestone Apr 1, 2022
@bcmills
Copy link
Member

@bcmills bcmills commented Apr 7, 2022

(why does gopls have to care about build stamping?)

This is somewhat related to #29666, in that go list is doing (a lot of!) extra work to compute the .Stale field for executable targets, which gopls probably doesn't really care about in this context.

@bcmills
Copy link
Member

@bcmills bcmills commented Apr 7, 2022

If gopls doesn't care about staleness (as I suspect), then -buildvcs=false seems like a fine workaround.

@bcmills
Copy link
Member

@bcmills bcmills commented Apr 7, 2022

This may also be another situation in which -buildvcs=auto (which will be the default after #51748 is addressed) should omit the VCS metadata rather than emitting an error.

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 8, 2022

Change https://go.dev/cl/398855 mentions this issue: cmd/go: allow '-buildvcs=auto' and treat it as the default

gopherbot pushed a commit that referenced this issue Apr 12, 2022
When we added VCS stamping in the Go 1.18 release, we defaulted to
-buildvcs=true, on the theory that most folks will actually want VCS
information stamped.

We also made -buildvcs=true error out if a VCS directory is found and
no VCS tool is available, on the theory that a user who builds with
'-buildvcs=true' will be very surprised if the VCS metadata is
silently missing.

However, that causes a problem for CI environments that don't have the
appropriate VCS tool installed. (And we know that's a common situation
because we're in that situation ourselves — see #46693!)

The new '-buildvcs=auto' setting provides a middle ground: it stamps
VCS information by default when the tool is present (and reports
explicit errors if the tool errors out), but omits the metadata
when the tool isn't present at all.

Fixes #51748.
Updates #51999.

Change-Id: Iebc955c2af0abca9b7517f62ca48b1d944eb2df4
Reviewed-on: https://go-review.googlesource.com/c/go/+/398855
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 14, 2022

Change https://go.dev/cl/400454 mentions this issue: [release-branch.go1.18] cmd/go: allow '-buildvcs=auto' and treat it as the default

gopherbot pushed a commit that referenced this issue Apr 20, 2022
…s the default

When we added VCS stamping in the Go 1.18 release, we defaulted to
-buildvcs=true, on the theory that most folks will actually want VCS
information stamped.

We also made -buildvcs=true error out if a VCS directory is found and
no VCS tool is available, on the theory that a user who builds with
'-buildvcs=true' will be very surprised if the VCS metadata is
silently missing.

However, that causes a problem for CI environments that don't have the
appropriate VCS tool installed. (And we know that's a common situation
because we're in that situation ourselves — see #46693!)

The new '-buildvcs=auto' setting provides a middle ground: it stamps
VCS information by default when the tool is present (and reports
explicit errors if the tool errors out), but omits the metadata
when the tool isn't present at all.

Updates #51748.
Updates #51999.
Fixes #51798.

Change-Id: Iebc955c2af0abca9b7517f62ca48b1d944eb2df4
Reviewed-on: https://go-review.googlesource.com/c/go/+/398855
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 4569fe6)
Reviewed-on: https://go-review.googlesource.com/c/go/+/400454
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
@findleyr findleyr added the gopls/metadata label May 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest gopls/metadata gopls NeedsInvestigation Tools
Projects
None yet
Development

No branches or pull requests

4 participants