Pass `BUILDTAGS` argument to `go build` #3926

marcusirgens · 2023-05-19T16:41:48Z

See discussion in #3919.

It looks like this might have happened by mistake in #3903. The Dockerfile used to build the v2.8.1 tag clearly passes the BUILDTAGS argument to Go's build command, which no longer is the case for the Dockerfile used to build the v2.8.2 tag.

Maintainers, please let me know if basing on release/2.8 is not the right way to do this, and I'll update my PR accordingly.

Fixes #3919

Signed-off-by: Marcus Pettersen Irgens <m@mrcus.dev>

milosgajdos · 2023-05-19T17:09:21Z

Ooops, I just noticed this PR after finishing mine #3927

Mine is more extensive, though, as it fixes linting issues as well as artifact builds. I'm hapy to close mine and let you take over if you are willing to take it over.

thaJeztah · 2023-05-19T18:59:04Z

The linting issues are not directly related though (afaics), and (per my comment on the other PR) should fixed on main as well. From that perspective, I think it'd make sense to take the most minimal patch to fix the issue at hand.

thaJeztah

LGTM

/cc @crazy-max

milosgajdos · 2023-05-19T19:05:22Z

The linting issues are not directly related though (afaics), and (per my comment on the other PR) should fixed on main as well. From that perspective, I think it'd make sense to take the most minimal patch to fix the issue at hand.

Sure, but once you properly open the build tags can of worms, you may as well fix it properly. Please beware my PR fixes other things in the build, not just the linting issues,

	-tags tag,list
		a comma-separated list of additional build tags to consider satisfied
		during the build. For more information about build tags, see
		'go help buildconstraint'. (Earlier versions of Go used a
		space-separated list, and that form is deprecated but still recognized.)

☝️ suggests the env var must contain comma-separated build tags. This also needs some changing of Makefile targets and docs, etc...it's all in my PR, but I'm happy to close it.

marcusirgens · 2023-05-20T08:55:39Z

I'm not at all opposed to someone on the team solving this in a different PR or commit, I just figured I as well might push what I had.

I also noticed that the -tags flag has moved to being comma-separated, but I figured, as @milosgajdos found, that I would have to track down where those (edit: the tags) might pop up from CI, which seems like a job for someone who is more familiar with this project than I am.

milosgajdos · 2023-05-20T09:13:09Z

I'm leaving it to @thaJeztah to take this over.

marcusirgens · 2023-06-01T06:19:26Z

@thaJeztah Do you need something from me here?

wy65701436 · 2023-06-06T06:37:40Z

The BUILDTAGS issue in the CI should also be addressed. However, I'm OK with merging this pull request as-is, and I will submit another one to fix the BUILDTAGS issue separately from the lint problem.

wy65701436

lgtm

milosgajdos · 2023-06-06T06:54:16Z

@wy65701436 there is a PR I closed few weeks ago #3927

I dont know what this PR is solving. It adds an env var to dockerfile build step but the env var is not populated properly - it uses spaces for build tags instead of comas, so this change will have no effect imho.

thaJeztah · 2023-06-06T08:42:22Z

I'm still a bit curious about the spaces vs comma, as I think many projects use space-separated (not against fixing though if we can)

As to the changes needed;

can we check if main has the same issue?
ideally, fix main (and backport) otherwise we regress when doing a v3 build
^^ where possible keep the changes as limited as possible; only fixing the issue at hand (I know I commented about that on the other PR, as it was fixing linting issues that were not directly related, and not fixed on main)

On a side-note; Go is doing a security release Today, so we should consider updating Go versions before we do a new release.

milosgajdos · 2023-06-06T08:47:37Z

From the official docs:

-tags tag,list
a comma-separated list of additional build tags to consider satisfied
during the build. For more information about build tags, see
'go help buildconstraint'. (Earlier versions of Go used a
space-separated list, and that form is deprecated but still recognized.)

thaJeztah · 2023-06-06T08:51:23Z

Earlier versions of Go used a
space-separated list, and that form is deprecated but still recognized.

Right, "nice to have", but not strictly required

milosgajdos · 2023-06-06T12:17:57Z

that form is deprecated but still recognized.

if we can address the deprecation why wouldn't we since we're trying to address the problem ?

marcusirgens · 2023-06-06T14:18:56Z

if we can address the deprecation why wouldn't we since we're trying to address the problem ?

The problem I was trying to address was the regression caused in #3903 where BUILDTAGS were removed from Dockerfile. I restored that flag using the same syntax.

I would love to get this issue solved, and I don't see how this fix would make it harder to make subsequent fixes in the future.

For context (and to illustrate why this invokes a sense of urgency to me, at least), we had mistakenly used a too lax image specifier ( registry:2) in our Kubernetes cluster, and when our internal registry restarted during a cluster upgrade, it couldn't connect to our image storage backend (GCS), which in turn took down almost every single deployment in our infrastructure.

If anyone wants to initiate a more thorough update to the Dockerfile or CI stuff in this repository, feel free to let me know and I'll close this PR.

milosgajdos · 2023-06-06T16:31:22Z

I am going to merge this just so it doesn't get stalled, but we need to do a follow-up to this. We will have to do a patch release I'm assuming?

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [registry](https://github.com/distribution/distribution) | patch | `2.8.2` -> `2.8.3` | --- ### ⚠ Dependency Lookup Warnings ⚠ Warnings were logged while processing this repo. Please check the logs for more information. --- ### Release Notes <details> <summary>distribution/distribution (registry)</summary> ### [`v2.8.3`](https://github.com/distribution/distribution/releases/tag/v2.8.3) [Compare Source](distribution/distribution@v2.8.2...v2.8.3) ##### What's Changed - Pass `BUILDTAGS` argument to `go build` by [@marcusirgens](https://github.com/marcusirgens) in distribution/distribution#3926 - Enable Go build tags by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4009 - `reference`: replace deprecated function `SplitHostname` by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4032 - Dont parse errors as JSON unless Content-Type is set to JSON by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4054 - update to go 1.20.8 by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4056 - Set `Content-Type` header in registry client `ReadFrom` by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4053 - deprecate reference package, migrate to github.com/distribution/reference by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4063 - `digestset`: deprecate package in favor of `go-digest/digestset` by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4064 - Do not close HTTP request body in HTTP handler by [@milosgajdos](https://github.com/milosgajdos) in distribution/distribution#4068 - Add v2.8.3 release notes by [@milosgajdos](https://github.com/milosgajdos) in distribution/distribution#4088 ##### New Contributors - [@marcusirgens](https://github.com/marcusirgens) made their first contribution in distribution/distribution#3926 **Full Changelog**: distribution/distribution@v2.8.2...v2.8.3 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.home/nrdufour/home-ops/pulls/127 Co-authored-by: Renovate <renovate@ptinem.io> Co-committed-by: Renovate <renovate@ptinem.io>

registry 2.8.3 Welcome to the v2.8.3 release of registry! Welcome to the 2.8.3 release of registry! The 2.8.3 registry release fixes a few bugs and makes a few dependency updates. The Go runtime has been bumped to 1.20.8. See the changelog below for full list of changes. * Do not close HTTP request body in HTTP handler ([distribution#4068](distribution#4068)) * Set Content-Type header in registry client ReadFrom ([distribution#4053](distribution#4053)) * Do not parse errors as JSON unless Content-Type is set to JSON ([distribution#4054](distribution#4054)) * Enable Go build tags ([distribution#4009](distribution#4009)) * replace deprecated function SplitHostname ([distribution#4032](distribution#4032)) * update to go1.20.8 ([distribution#4056](distribution#4056)) * deprecate digestset package in favor of go-digest/digestset ([distribution#4064](distribution#4064)) * deprecate reference package, migrate to github.com/distribution/reference ([distribution#4063](distribution#4063)) Previous release can be found at [v2.8.2](https://github.com/distribution/distribution/releases/tag/v2.8.2) Please try out the release binaries and report any issues at https://github.com/distribution/distribution/issues. * Milos Gajdos * Sebastiaan van Stijn * Milos Gajdos * James Hewitt * Ben Manuel * Marcus Pettersen Irgens * zounengren <details><summary>30 commits</summary> <p> * Add v2.8.3 release notes ([distribution#4088](distribution#4088)) * [`a4fa6992`](distribution@a4fa699) Add v2.8.3 release notes * [release/2.8] Do not close HTTP request body in HTTP handler ([distribution#4068](distribution#4068)) * [`5e6b1b5c`](distribution@5e6b1b5) Do not close HTTP request body in HTTP handler * [release/2.8] digestset: deprecate package in favor of go-digest/digestset ([distribution#4064](distribution#4064)) * [`29b00e8b`](distribution@29b00e8) digestset: deprecate package in favor of go-digest/digestset * [`d1ab2430`](distribution@d1ab243) [release/2.8] vendor: github.com/opencontainers/go-digest v1.0.0 * [release/2.8] deprecate reference package, migrate to github.com/distribution/reference ([distribution#4063](distribution#4063)) * [`3dda0677`](distribution@3dda067) deprecate reference package, migrate to github.com/distribution/reference * [release/2.8 backport] Set Content-Type header in registry client ReadFrom ([distribution#4053](distribution#4053)) * [`cb121c3f`](distribution@cb121c3) Set Content-Type header in registry client ReadFrom * [release/2.8] update to go1.20.8 ([distribution#4056](distribution#4056)) * [`3c6f7788`](distribution@3c6f778) update to go1.20.8 * [`31f5cd48`](distribution@31f5cd4) Handle rand deprecations in go 1.20 * [`29b8ba0b`](distribution@29b8ba0) Update to go 1.20 * [`3316b198`](distribution@3316b19) Update to golang 1.19.10 * [`444d053e`](distribution@444d053) update golangci-lint to v1.52 * [`b800af44`](distribution@b800af4) ignore SA1019: ac.(*accessController).rootCerts.Subjects has been deprecated * [`0a98a00d`](distribution@0a98a00) Ignore SA1019: SplitHostname is deprecated. * [release/2.8 backport] Dont parse errors as JSON unless Content-Type is set to JSON ([distribution#4054](distribution#4054)) * [`2ec0471b`](distribution@2ec0471) Dont parse errors as JSON unless Content-Type is set to JSON * [release/2.8 backport] reference: replace deprecated function SplitHostname ([distribution#4032](distribution#4032)) * [`b57133cc`](distribution@b57133c) referene: fix formatting of "deprecated" comment. * [`2c4bf1a6`](distribution@2c4bf1a) replace deprecated function * [release/2.8 backport] Enable Go build tags ([distribution#4009](distribution#4009)) * [`110cb753`](distribution@110cb75) Enable build tags in 2.8 * [`2d62a402`](distribution@2d62a40) s3: add interface assertion * [`2548973b`](distribution@2548973) Enable Go build tags * Pass `BUILDTAGS` argument to `go build` ([distribution#3926](distribution#3926)) * [`ab7178cc`](distribution@ab7178c) Pass BUILDTAGS argument to go build </p> </details> * **github.com/distribution/reference** 49c28499d219 **_new_** * **github.com/opencontainers/go-digest** a6d0ee40d420 -> ea51bea511f7 Previous release can be found at [v2.8.2](https://github.com/distribution/distribution/releases/tag/v2.8.2)

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [registry](https://github.com/distribution/distribution) | patch | `2.8.2` -> `2.8.3` | --- > ⚠ **Warning** > > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>distribution/distribution (registry)</summary> ### [`v2.8.3`](https://github.com/distribution/distribution/releases/tag/v2.8.3) [Compare Source](distribution/distribution@v2.8.2...v2.8.3) #### What's Changed - Pass `BUILDTAGS` argument to `go build` by [@marcusirgens](https://github.com/marcusirgens) in distribution/distribution#3926 - Enable Go build tags by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4009 - `reference`: replace deprecated function `SplitHostname` by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4032 - Dont parse errors as JSON unless Content-Type is set to JSON by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4054 - update to go 1.20.8 by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4056 - Set `Content-Type` header in registry client `ReadFrom` by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4053 - deprecate reference package, migrate to github.com/distribution/reference by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4063 - `digestset`: deprecate package in favor of `go-digest/digestset` by [@thaJeztah](https://github.com/thaJeztah) in distribution/distribution#4064 - Do not close HTTP request body in HTTP handler by [@milosgajdos](https://github.com/milosgajdos) in distribution/distribution#4068 - Add v2.8.3 release notes by [@milosgajdos](https://github.com/milosgajdos) in distribution/distribution#4088 #### New Contributors - [@marcusirgens](https://github.com/marcusirgens) made their first contribution in distribution/distribution#3926 **Full Changelog**: distribution/distribution@v2.8.2...v2.8.3 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.mgrote.net/mg/homeserver/pulls/667 Co-authored-by: Renovate Bot <renovate@mgrote.net> Co-committed-by: Renovate Bot <renovate@mgrote.net>

Pass BUILDTAGS argument to go build

ab7178c

Signed-off-by: Marcus Pettersen Irgens <m@mrcus.dev>

thaJeztah approved these changes May 19, 2023

View reviewed changes

milosgajdos mentioned this pull request May 23, 2023

Dockerfile: pass BUILDTAGS to go build #3703

Closed

wy65701436 approved these changes Jun 6, 2023

View reviewed changes

crazy-max approved these changes Jun 6, 2023

View reviewed changes

milosgajdos merged commit 8728c52 into distribution:release/2.8 Jun 7, 2023

Pass BUILDTAGS argument to go build #3926

Pass BUILDTAGS argument to go build #3926

Uh oh!

Conversation

marcusirgens commented May 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

milosgajdos commented May 19, 2023

Uh oh!

thaJeztah commented May 19, 2023

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

milosgajdos commented May 19, 2023

Uh oh!

marcusirgens commented May 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

milosgajdos commented May 20, 2023

Uh oh!

marcusirgens commented Jun 1, 2023

Uh oh!

wy65701436 commented Jun 6, 2023

Uh oh!

wy65701436 left a comment

Choose a reason for hiding this comment

Uh oh!

milosgajdos commented Jun 6, 2023

Uh oh!

thaJeztah commented Jun 6, 2023

Uh oh!

milosgajdos commented Jun 6, 2023

Uh oh!

thaJeztah commented Jun 6, 2023

Uh oh!

milosgajdos commented Jun 6, 2023

Uh oh!

marcusirgens commented Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

milosgajdos commented Jun 6, 2023

Uh oh!

Uh oh!

Pass `BUILDTAGS` argument to `go build` #3926

Pass `BUILDTAGS` argument to `go build` #3926

marcusirgens commented May 19, 2023 •

edited

Loading

marcusirgens commented May 20, 2023 •

edited

Loading

marcusirgens commented Jun 6, 2023 •

edited

Loading