Disable buildvcs

[AbdulrhmnGhanem]

• Fixes Docker build fails if gitea is a git submodule #20055

[techknowlogick]

You can achieve this with goflags env var, or extra goflags. This also disables it for all builds, not just your specific use case.

[wxiaoguang]

I think Golang's buildvcs=true/auto is not ideal.

By default, version control information is stamped into a binary if the main package and the main module containing it are in the repository containing the current directory (if there is a repository)

It doesn't help Gitea's build, and may break reproducible build (even the code the exactly the same, the output differs due to different repository information).

[42wim]

A bit off topic, but about reproducible builds:

• https://words.filippo.io/reproducing-go-binaries-byte-by-byte/
• https://hackernoon.com/building-reproducible-verifiable-binaries-with-golang
• proposal: cmd/go: add -reproducible flag golang/go#51812

If we want those we should at least do:

• vendor code (was removed a few months ago)
• use -trimpath
• -ldflags=-buildid=
• -buildvcs=false

But as I read the pages above because we're using CGO it's going to be tricky anyway to have reproducible builds.

[AbdulrhmnGhanem]

You can achieve this with goflags env var, or extra goflags. This also disables it for all builds, not just your specific use case.

You are right, though does it make sense the docker build should fail depending on the VCS status? Failing when building inside a submodule is just a symptom I am sure there are other cases where it would fail too.

[techknowlogick]

Yeah, what @42wim said has changed my pov. I'll drop my review, although I'll suggest adding the flag into the existing goflag env var instead of hardcoding it directly.

stale

[zeripath]

Why aren't you just setting EXTRA_GOFLAGS to use this flag for your specific need. Most people building Gitea do not need this flag and will not need it.

[AbdulrhmnGhanem]

Even if the reasons above don't justify the use of this flag, what value does this flag remove from other users? It just resets the build to behave as pre-1.18.

[AbdulrhmnGhanem]

I'll suggest adding the flag into the existing goflag env var instead of hardcoding it directly.

Adding

GOFLAGS += -buildvcs=false

the build fails with the same error, is this wrong?

[techknowlogick]

what about just adding it as EXTRA_GOFLAGS ?= -buildvcs=false to

gitea/Makefile

Line 68 in 389496b

EXTRA_GOFLAGS ?=

?

It'll be overwritten if someone tries to add their own extra goflags, but I think that something that can be dealt with in a later PR.

[AbdulrhmnGhanem]

what about just adding it as EXTRA_GOFLAGS ?= -buildvcs=false to

gitea/Makefile

Line 68 in 389496b

EXTRA_GOFLAGS ?=

?
It'll be overwritten if someone tries to add their own extra goflags, but I think that something that can be dealt with in a later PR.

It doesn't work!

[Gusted]

Why modifying the makefile? Using env variables is just easier:

EXTRA_GOFLAGS="-buildvcs=false" TAGS="sqlite sqlite_unlock_notify" make build

Which passes the go flag correctly:

CGO_CFLAGS="-g -O2 -DSQLITE_MAX_VARIABLE_NUMBER=32766" go build -v -buildvcs=false -tags 'sqlite sqlite_unlock_notify' -ldflags '-s -w  -X "main.MakeVersion=GNU Make 4.3" -X "main.Version=1.18.0+dev-58-g9d9bf66c3" -X "main.Tags=sqlite sqlite_unlock_notify"' -o gitea

[AbdulrhmnGhanem]

This doesn't resolve the issue addressed by this PR. Running the repro steps in the issue and adding ENV EXTRA_GOFLAGS "-buildvcs=false" to the docker file, it still fails. Also, in all previous trials above the flag gets passed correctly but it still fails.

[Gusted]

From my point of view, it's clear that we(the Gitea maintainers) don't see the benefit of adding this flag globally, as it doesn't provide any benefits to any other use-case.

You can configure build flags like that via environments, there's no need to hard-code them. You mention that the flag is passed along with the build command, but it still fails, at that point it's out of Gitea's hand and likely a Go/Docker bug. The current PR doesn't fix the issue it still results into the same error. Unless this PR is to fix of why the error still exists even with -buildvcs=false passed, I don't see a reason to hold this PR open.

Also as mentioned on the original issue, Go1.19 won't have this issue anymore.