[release/2.8] Enable build tags in 2.8 #3927

milosgajdos · 2023-05-19T17:00:23Z

It would appear we were missing the Go build tags on 2.8.X branch so the images would not have the necessary support for some storage drivers causing breakages to end users trying to use them.

This commit fixes both the build and linting issues.

Fixes: #3919

codecov-commenter · 2023-05-19T17:15:17Z

Codecov Report

Patch and project coverage have no change.

Comparison is base (7c354a4) 58.87% compared to head (1d66ab6) 58.87%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@             Coverage Diff              @@
##           release/2.8    #3927   +/-   ##
============================================
  Coverage        58.87%   58.87%           
============================================
  Files              102      102           
  Lines             7120     7120           
============================================
  Hits              4192     4192           
  Misses            2284     2284           
  Partials           644      644

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

thaJeztah

can we apply the fixes on main first? Otherwise we don't update it there, which means that we're regressing in v3

thaJeztah · 2023-05-19T17:55:54Z

registry/storage/driver/gcs/gcs.go

-//
-//go:build include_gcs
-// +build include_gcs
-
 package gcs


Looks like we didn't fix these on the main branch (still has the empty line after, and the build-tags in the wrong spot);

distribution/registry/storage/driver/gcs/gcs.go

Lines 12 to 16 in 983358f

//

//go:build include_gcs

// +build include_gcs

package gcs

Done in #3950

thaJeztah · 2023-05-19T17:57:20Z

registry/storage/driver/oss/oss_test.go

 	keys, err := emptyRootDriver.List(ctx, "/")
+	if err != nil {
+		t.Fatalf("unexpected error listing empty root content: %v", err)
+	}


These area also missing on main;

distribution/registry/storage/driver/oss/oss_test.go

Line 130 in 983358f

keys, err := emptyRootDriver.List(ctx, "/")

Done in #3950

milosgajdos · 2023-05-19T18:04:09Z

@thaJeztah I'd prefer to add those in as part of #3702

That PR also bumps GCS dependency.

davidspek · 2023-08-15T12:24:30Z

@milosgajdos Is there a reason this PR was closed? Seems like this should get merged and a v2.8.3 release needs to be cut to resolve this error. See #3919 (comment).

milosgajdos · 2023-08-15T13:41:40Z

Reopening. Can't remember why I closed this 🤔

It would appear we were missing the Go build tags on 2.8.X branch so the images would not have the necessary support for some storage drivers causing breakages to end users trying to use them. This commit fixes both the build and linting issues. Signed-off-by: Milos Gajdos <milosthegajdos@gmail.com>

milosgajdos · 2023-08-15T14:04:53Z

@thaJeztah PTAL (I've reopened this because I somehow closed this by accident)

This is the same as #3950 but for 2.8.x branch

davidspek

LGTM

milosgajdos · 2023-08-16T07:24:13Z

Ping @wy65701436 @thaJeztah

davidspek · 2023-08-16T17:09:03Z

@deleteriousEffect maybe you can have a look at this since the current release is broken and we need this to fix it.

.github/workflows/ci.yml

milosgajdos · 2023-08-17T16:27:06Z

@thaJeztah this is blocked by your requiring changes 🙃

wy65701436

lgtm

thaJeztah

I did a quick branch (#4009) where I cherry-picked the changes from main (#3950), and rebased your PR on top to verify if there's things missing on main and to see the differences; noticed that one bit was missing, so opened a PR for that (and cherry-picked into that branch); #4008

registry/storage/driver/s3-aws/s3.go

thaJeztah · 2023-08-21T11:46:05Z

registry/storage/driver/oss/oss.go

@@ -38,12 +37,11 @@ const driverName = "oss"
 const minChunkSize = 5 << 20

 const defaultChunkSize = 2 * minChunkSize
-const defaultTimeout = 2 * time.Minute // 2 minute timeout per chunk


It's removed now on the main branch, but looks like this one was missed as well in 6b388b1 / #3950

I dont think I follow this comment. This was removed because it was failing to pass the lint step.

milosgajdos · 2023-08-21T12:48:25Z

I did a quick branch (#4009) where I cherry-picked the changes from main (#3950), and rebased your PR on top to verify if there's things missing on main and to see the differences; noticed that one bit was missing, so opened a PR for that (and cherry-picked into that branch); #4008

Merged

milosgajdos · 2023-08-22T14:01:48Z

closing in favour of #4009

milosgajdos mentioned this pull request May 19, 2023

Pass BUILDTAGS argument to go build #3926

Merged

milosgajdos requested review from wy65701436, thaJeztah and corhere May 19, 2023 17:10

milosgajdos force-pushed the enable-build-tags branch from f465ee4 to 1d66ab6 Compare May 19, 2023 17:35

thaJeztah requested changes May 19, 2023

View reviewed changes

milosgajdos requested a review from thaJeztah May 19, 2023 18:51

milosgajdos closed this May 19, 2023

milosgajdos reopened this Aug 15, 2023

milosgajdos force-pushed the enable-build-tags branch from 1d66ab6 to afe4484 Compare August 15, 2023 14:03

davidspek approved these changes Aug 15, 2023

View reviewed changes

deleteriousEffect approved these changes Aug 17, 2023

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

milosgajdos added the 2.x label Aug 20, 2023

wy65701436 approved these changes Aug 21, 2023

View reviewed changes

milosgajdos mentioned this pull request Aug 21, 2023

Enable bodyclose linter #4006

Merged

This was referenced Aug 21, 2023

s3: add interface assertion #4008

Merged

[release/2.8 backport] Enable Go build tags #4009

Merged

thaJeztah reviewed Aug 21, 2023

View reviewed changes

milosgajdos requested a review from thaJeztah August 21, 2023 13:05

milosgajdos closed this Aug 22, 2023

[release/2.8] Enable build tags in 2.8 #3927

[release/2.8] Enable build tags in 2.8 #3927

Uh oh!

Conversation

milosgajdos commented May 19, 2023

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah May 19, 2023

Choose a reason for hiding this comment

Uh oh!

milosgajdos Aug 15, 2023

Choose a reason for hiding this comment

Uh oh!

thaJeztah May 19, 2023

Choose a reason for hiding this comment

Uh oh!

milosgajdos Aug 15, 2023

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

davidspek commented Aug 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

milosgajdos commented Aug 15, 2023

Uh oh!

milosgajdos commented Aug 15, 2023

Uh oh!

davidspek left a comment

Choose a reason for hiding this comment

Uh oh!

milosgajdos commented Aug 16, 2023

Uh oh!

davidspek commented Aug 16, 2023

Uh oh!

Uh oh!

milosgajdos commented Aug 17, 2023

Uh oh!

wy65701436 left a comment

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thaJeztah Aug 21, 2023

Choose a reason for hiding this comment

Uh oh!

milosgajdos Aug 21, 2023

Choose a reason for hiding this comment

Uh oh!

milosgajdos commented Aug 21, 2023

Uh oh!

milosgajdos commented Aug 22, 2023

Uh oh!

Uh oh!

codecov-commenter commented May 19, 2023 •

edited

Loading

milosgajdos commented May 19, 2023 •

edited

Loading

davidspek commented Aug 15, 2023 •

edited

Loading