shouldn't this be posted in #37475 which tracks the implementation?

cc @jayconrod

cc @bcmills @matloob

This is somewhat working as intended: I reused some code from GOPATH go get, which forbids repository-within-repository, except Git-within-Git, which is allowed. In general though, there's a lot of ambiguity where repositories, modules, packages, and directories don't correspond, so it seemed better to report an error if anything was usual.

Could you say more about your use case? Would be good to understand how common this will be.

I agree that if the VCS containing the CWD matches the VCS containing the module root, we shouldn't fail due to other VCS directories further up.

The reason they were rejected in GOPATH-mode go get was to avoid VCS-injection attacks, because in GOPATH mode the go command runs its VCS commands in arbitrary directories that aren't directly controlled by the user. In this case, the user does control the path (it's always the CWD).

My use case is that I happen to have a catch-all .bzr directory in my home. I probably shouldn't (it's a historical accident) but nonetheless I have never had any problems it ever before, and I don't think I should now.

I appreciate that VCS within VCS is a specific issue when there's a VCS directory that's below the module root, but I don't think there should be a problem in this case.

AFAICS the logic should be something like: find the nearest enclosing VCS directory:; if it doesn't enclose the root of the current module, it's an error; otherwise use the revision from it.

I can't currently think of a reason to look outside the nearest enclosing VCS directory.

Change https://golang.org/cl/356309 mentions this issue: cmd/go: allow nested VCS repositories when preparing build stamp

One of my users on Gentoo is getting bitten by this because he has a fossil control directory in /var.
The sandbox violation means that go is trying to do things outside the package build staging area.
Apparently this is causing a number of go-based packages to not build any more.

https://bugs.gentoo.org/836261

As you can see, he is recommending that we disable the function for builds within our package manager. What do you suggest?

Thanks @williamh ! I'm that gentoo user. I have to say that, during each build, firing up software that will ascend the whole directory tree looking for control directories that may or may not be project related is probably not a good idea. Not everybody compiles using pristine machine images... Chances are that version information of that unrelated project may be used. And many people will have several VCS installed, meaning that the search will happen using git, hg, bzr, fossil and whatnot, increasing the attack or failure surface a lot.

I do see that sometimes a go project can be in a subdirectory of a larger projects' root. One way to avoid the problem would be to explicitly specify the VCS root in the build. Or to place a "stop VCS search here" file or directory in the project's root. Or, at least to never fire up the VCS search if a version tag is given explicitly (as in gentoo I believe it is).

One specific problem with fossil might also be that it opens the repository read/write even with only informational commands. Not sure that all the other systems open theirs readonly in this case. Sandbox systems would probably abort the build in this case as well, as the repository directory will often be out of bounds. So, without one of the provisions mentioned above avoiding the directory ascending altogether, each and every VCS would need a sandbox-safe "probe mode". This probably means asking too much and we're back to adding more logic to safely conclude that 1) we need info from the VCS and 2) we're actually inside a relevant project-specific checkout.

What can be done immediately is, of course, to set -buildvcs=false if we know that we're not building from a checkout but some tar release. Are there any objections to that?

There is going to be a new default of -buildvcs=auto (ref #51748) though I don't think it's going to help in your case. VCS info also includes clena/dirty info, not just version tags. If you know you're building from a source tarball that isn't going to contain vcs information, setting -buildvcs=false sounds like a reasonable default for your situation.

That means all package manager builds in Gentoo will end up having -buildvcs=false since we build from tarballs.
I can add that to GOFLAGS once I verify that it will not break go 1.17 since we keep the two most recent versions of go.

@williamh unfortunately Go 1.17 will try to consume GOFLAGS=-buildvcs=false and fail, as that version doesn't know the flag.

Unfortunately I just discovered this.
I'm going to think about what we can do on our side, but this is definitely a breaking change.
I maintain a basic framework that allows other gentoo developers to write packages for software written in Go, and one thing it does is set global GOFLAGS. The issue I'm going to have until go 1.17 is removed from Gentoo (which will happen when go 1.19 is released) is that I need to set those global GOFLAGS in a way that works for go 1.17 and 1.18 without knowing which version of Go is installed, or I need to find a way to know which version is installed, which I don't see right now.
I will update here, and if you have any suggestions other than not setting -buildvcs=false, that would be great.

If you can in any way control the environment where go build is called, perhaps you could hide the VCS programs like git from $PATH, or hide the VCS directory via e.g. chroot. I understand those aren't as easy as export GOFLAGS=..., but at least they wouldn't break Go 1.17.x.

Another potential idea: set up $PATH with a go wrapper that first calls the underlying go version; if it contains the string go1.17, export GOFLAGS=-buildvcs=false. Then, call the underlying Go normally via exec go "$@".

A wrapper might be an option, but it would need to be a wrapper that only the package manager can use. I don't really want to change the behavior of the go command outside the package manager context.
I'll explore this idea.

Sorry, this is where google leads with the error and this thread is a bit cluttered...

So, the solution is to just use go build -buildvcs=false . when using in a Docker container, e.g.? Worked for me, at least, but some word about general recommendation for dummies could be helpful. (As said, Google leads here, would be a cool nice-to-have)

This particular bug was about nested VCS repositories causing an error - that was solved, and no workaround is needed.

I assume the problem you have is that git may not be installed inside your Docker image, where you run go build inside a git directory. That is #51748, which is fixed in 1.18.2, so it does not need a workaround either.

Thanks for your answer!
I tried both using golang:1.18.2-buster instead of golang:1.18.1-buster and installing git via apt-get in my container before but I still get:

go: downloading github.com/nats-io/nats.go v1.14.0
go: downloading github.com/nats-io/nkeys v0.3.0
go: downloading github.com/nats-io/nuid v1.0.1
go: downloading golang.org/x/crypto v0.0.0-20220315160706-3147a52a75dd
error obtaining VCS status: exit status 128
	Use -buildvcs=false to disable VCS stamping.

But -buildcvs=false still works for me

Please file a new bug with steps to reproduce.

Did it with sample nested repo: #53640 (repo: https://github.com/MichaelVoelkel/vcs-test )

Just ran into this on GitHub Actions + Fly.io no idea what vcs is or why it only happened in that specific environment (fly.io manual deploy works fine, GitHub Actions can also build Docker images fine, but if I invoked fly deploy from a GitHub action, this confusing error appears)

So, just commenting here with some keywords to help others who may be searching for that exact combination of tools - because I couldn't find anything!

Also gonna ping @michaeldwan just in case this is something the Fly team can fix in their environment for Go applications :)