Merge pull request #457 from Luap99/gofumpt

image: enable gofumpt formatter

Author: mtrmac

CONTRIBUTING_GO.md

@@ -14,6 +14,7 @@ At present, this means the following repositories:
 ## Topics
 
 * [Unit Tests](#unit-tests)
+* [Go Format and lint](#go-format-and-lint)
 * [Go Dependency updates](#go-dependency-updates)
 * [Testing changes in a dependent repository](#testing-changes-in-a-dependent-repository)
 * [git bisect a change in a Go dependency](#git-bisect-a-change-in-a-go-dependency)
@@ -23,6 +24,11 @@ At present, this means the following repositories:
 Unit tests for Go code are added in a separate file within the same directory, named `..._test.go` (where the first part of the name is often the name of the file whose code is being tested).
 Our Go projects to not require unit tests, but contributors are strongly encouraged to unit test any code that can have a reasonable unit test written.
 
+### Go Format and lint
+
+We are using the [`gofumpt`](https://github.com/mvdan/gofumpt) formatter for our go code, you can either use it directly or format via `make fmt`.
+For linting we use [`golangci-lint`](github.com/golangci/golangci-lint), use `make validate` to run it together with some other basic commit checks.
+
 ## Go Dependency updates
 
 To automatically keep dependencies up to date we use the [renovate](https://github.com/renovatebot/renovate) bot.

Makefile

@@ -49,6 +49,12 @@ lint: .install.golangci-lint
 	@$(MAKE) -C image lint
 	@$(MAKE) -C storage lint
 
+.PHONY: fmt
+fmt: .install.golangci-lint
+	@$(MAKE) -C common fmt
+	@$(MAKE) -C image fmt
+	@$(MAKE) -C storage fmt
+
 .PHONY: vendor-in-container
 vendor-in-container:
 	podman run --privileged --rm --env HOME=/root -v `pwd`:/src -w /src golang make vendor

common/Makefile

@@ -74,6 +74,10 @@ docs:
 lint:
 	golangci-lint run
 
+.PHONY: fmt
+fmt:
+	golangci-lint fmt
+
 .PHONY: validate
 validate: lint
 	./tools/validate_seccomp.sh ./pkg/seccomp

image/.golangci.yml

@@ -1,4 +1,9 @@
 version: "2"
+
+formatters:
+  enable:
+    - gofumpt
+
 linters:
   enable:
     - errorlint

image/Makefile

@@ -75,7 +75,7 @@ test:
 
 .PHONY: fmt
 fmt:
-	@gofmt -l -s -w $(SOURCE_DIRS)
+	golangci-lint fmt
 
 .PHONY: lint
 lint:

image/copy/blob.go

@@ -18,7 +18,8 @@ import (
 // and returns a complete blobInfo of the copied blob.
 func (ic *imageCopier) copyBlobFromStream(ctx context.Context, srcReader io.Reader, srcInfo types.BlobInfo,
 	getOriginalLayerCopyWriter func(decompressor compressiontypes.DecompressorFunc) io.Writer,
-	isConfig bool, toEncrypt bool, bar *progressBar, layerIndex int, emptyLayer bool) (types.BlobInfo, error) {
+	isConfig bool, toEncrypt bool, bar *progressBar, layerIndex int, emptyLayer bool,
+) (types.BlobInfo, error) {
 	// The copying happens through a pipeline of connected io.Readers;
 	// that pipeline is built by updating stream.
 	// === Input: srcReader

image/copy/compression.go

@@ -109,7 +109,8 @@ const (
 // Returns data for other steps; the caller should eventually call updateCompressionEdits and perhaps recordValidatedBlobData,
 // and must eventually call close.
 func (ic *imageCopier) blobPipelineCompressionStep(stream *sourceStream, canModifyBlob bool, srcInfo types.BlobInfo,
-	detected bpDetectCompressionStepData) (*bpCompressionStepData, error) {
+	detected bpDetectCompressionStepData,
+) (*bpCompressionStepData, error) {
 	// WARNING: If you are adding new reasons to change the blob, update also the OptimizeDestinationImageAlreadyExists
 	// short-circuit conditions
 	layerCompressionChangeSupported := ic.src.CanChangeLayerCompression(stream.info.MediaType)
@@ -265,7 +266,8 @@ func (ic *imageCopier) bpcDecompressCompressed(stream *sourceStream, detected bp
 // This does not change the sourceStream parameter; we include it for symmetry with other
 // pipeline steps.
 func (ic *imageCopier) bpcPreserveOriginal(_ *sourceStream, detected bpDetectCompressionStepData,
-	layerCompressionChangeSupported bool) *bpCompressionStepData {
+	layerCompressionChangeSupported bool,
+) *bpCompressionStepData {
 	logrus.Debugf("Using original blob without modification")
 	// Remember if the original blob was compressed, and if so how, so that if
 	// LayerInfosForCopy() returned something that differs from what was in the
@@ -320,7 +322,8 @@ func (d *bpCompressionStepData) updateCompressionEdits(operation *types.LayerCom
 // and the original srcInfo (which the caller guarantees has been validated).
 // This must ONLY be called if all data has been validated by OUR code, and is not coming from third parties.
 func (d *bpCompressionStepData) recordValidatedDigestData(c *copier, uploadedInfo types.BlobInfo, srcInfo types.BlobInfo,
-	encryptionStep *bpEncryptionStepData, decryptionStep *bpDecryptionStepData) error {
+	encryptionStep *bpEncryptionStepData, decryptionStep *bpDecryptionStepData,
+) error {
 	// Don’t record any associations that involve encrypted data. This is a bit crude,
 	// some blob substitutions (replacing pulls of encrypted data with local reuse of known decryption outcomes)
 	// might be safe, but it’s not trivially obvious, so let’s be conservative for now.

image/copy/encryption.go

@@ -84,7 +84,8 @@ type bpEncryptionStepData struct {
 // srcInfo is primarily used for error messages.
 // Returns data for other steps; the caller should eventually call updateCryptoOperationAndAnnotations.
 func (ic *imageCopier) blobPipelineEncryptionStep(stream *sourceStream, toEncrypt bool, srcInfo types.BlobInfo,
-	decryptionStep *bpDecryptionStepData) (*bpEncryptionStepData, error) {
+	decryptionStep *bpDecryptionStepData,
+) (*bpEncryptionStepData, error) {
 	if !toEncrypt || isOciEncrypted(srcInfo.MediaType) || ic.c.options.OciEncryptConfig == nil {
 		return &bpEncryptionStepData{
 			encrypting: false,

image/copy/manifest_test.go

@@ -126,7 +126,8 @@ func TestDetermineManifestConversion(t *testing.T) {
 		},
 		// Conversion necessary, a preferred format is not acceptable
 		{
-			"s2→OCI", manifest.DockerV2Schema2MediaType, []string{v1.MediaTypeImageManifest},
+			"s2→OCI", manifest.DockerV2Schema2MediaType,
+			[]string{v1.MediaTypeImageManifest},
 			manifestConversionPlan{
 				preferredMIMEType:                v1.MediaTypeImageManifest,
 				preferredMIMETypeNeedsConversion: true,
@@ -135,7 +136,8 @@ func TestDetermineManifestConversion(t *testing.T) {
 		},
 		// text/plain is converted if the destination does not accept s1
 		{
-			"text→s2", "text/plain", []string{manifest.DockerV2Schema2MediaType},
+			"text→s2", "text/plain",
+			[]string{manifest.DockerV2Schema2MediaType},
 			manifestConversionPlan{
 				preferredMIMEType:                manifest.DockerV2Schema2MediaType,
 				preferredMIMETypeNeedsConversion: true,
@@ -162,7 +164,8 @@ func TestDetermineManifestConversion(t *testing.T) {
 			},
 		},
 		{
-			"special→OCI", manifest.DockerV2ListMediaType, []string{v1.MediaTypeImageManifest, "other options", "with lower priority"},
+			"special→OCI", manifest.DockerV2ListMediaType,
+			[]string{v1.MediaTypeImageManifest, "other options", "with lower priority"},
 			manifestConversionPlan{
 				preferredMIMEType:                v1.MediaTypeImageManifest,
 				preferredMIMETypeNeedsConversion: true,

image/copy/multiple.go

@@ -60,8 +60,9 @@ func platformV1ToPlatformComparable(platform *imgspecv1.Platform) platformCompar
 	}
 	osFeatures := slices.Clone(platform.OSFeatures)
 	sort.Strings(osFeatures)
-	return platformComparable{architecture: platform.Architecture,
-		os: platform.OS,
+	return platformComparable{
+		architecture: platform.Architecture,
+		os:           platform.OS,
 		// This is strictly speaking ambiguous, fields of OSFeatures can contain a ','. Probably good enough for now.
 		osFeatures: strings.Join(osFeatures, ","),
 		osVersion:  platform.OSVersion,
@@ -252,15 +253,17 @@ func (c *copier) copyMultipleImages(ctx context.Context) (copiedManifest []byte,
 				UpdateDigest:                updated.manifestDigest,
 				UpdateSize:                  int64(len(updated.manifest)),
 				UpdateCompressionAlgorithms: updated.compressionAlgorithms,
-				UpdateMediaType:             updated.manifestMIMEType})
+				UpdateMediaType:             updated.manifestMIMEType,
+			})
 		case instanceCopyClone:
 			logrus.Debugf("Replicating instance %s (%d/%d)", instance.sourceDigest, i+1, len(instanceCopyList))
 			c.Printf("Replicating image %s (%d/%d)\n", instance.sourceDigest, i+1, len(instanceCopyList))
 			unparsedInstance := image.UnparsedInstance(c.rawSource, &instanceCopyList[i].sourceDigest)
 			updated, err := c.copySingleImage(ctx, unparsedInstance, &instanceCopyList[i].sourceDigest, copySingleImageOptions{
 				requireCompressionFormatMatch: true,
 				compressionFormat:             &instance.cloneCompressionVariant.Algorithm,
-				compressionLevel:              instance.cloneCompressionVariant.Level})
+				compressionLevel:              instance.cloneCompressionVariant.Level,
+			})
 			if err != nil {
 				return nil, fmt.Errorf("replicating image %d/%d from manifest list: %w", i+1, len(instanceCopyList), err)
 			}