image: enable gofumpt formatter

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)
 			}

image/copy/multiple_test.go

@@ -31,16 +31,20 @@ func TestPrepareCopyInstancesforInstanceCopyCopy(t *testing.T) {
 	compare := []instanceCopy{}
 
 	for _, instance := range sourceInstances {
-		compare = append(compare, instanceCopy{op: instanceCopyCopy,
-			sourceDigest: instance, copyForceCompressionFormat: false})
+		compare = append(compare, instanceCopy{
+			op:           instanceCopyCopy,
+			sourceDigest: instance, copyForceCompressionFormat: false,
+		})
 	}
 	assert.Equal(t, instancesToCopy, compare)
 
 	// Test CopySpecificImages where selected instance is sourceInstances[1]
 	instancesToCopy, err = prepareInstanceCopies(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages})
 	require.NoError(t, err)
-	compare = []instanceCopy{{op: instanceCopyCopy,
-		sourceDigest: sourceInstances[1]}}
+	compare = []instanceCopy{{
+		op:           instanceCopyCopy,
+		sourceDigest: sourceInstances[1],
+	}}
 	assert.Equal(t, instancesToCopy, compare)
 
 	_, err = prepareInstanceCopies(list, sourceInstances, &Options{Instances: []digest.Digest{sourceInstances[1]}, ImageListSelection: CopySpecificImages, ForceCompressionFormat: true})
@@ -64,9 +68,11 @@ func TestPrepareCopyInstancesforInstanceCopyClone(t *testing.T) {
 	}
 
 	// CopySpecificImage must fail with error
-	_, err = prepareInstanceCopies(list, sourceInstances, &Options{EnsureCompressionVariantsExist: ensureCompressionVariantsExist,
-		Instances:          []digest.Digest{sourceInstances[1]},
-		ImageListSelection: CopySpecificImages})
+	_, err = prepareInstanceCopies(list, sourceInstances, &Options{
+		EnsureCompressionVariantsExist: ensureCompressionVariantsExist,
+		Instances:                      []digest.Digest{sourceInstances[1]},
+		ImageListSelection:             CopySpecificImages,
+	})
 	require.EqualError(t, err, "EnsureCompressionVariantsExist is not implemented for CopySpecificImages")
 
 	// Test copying all images with replication
@@ -83,8 +89,10 @@ func TestPrepareCopyInstancesforInstanceCopyClone(t *testing.T) {
 	// and still copy `sourceInstance[2]`.
 	expectedResponse := []simplerInstanceCopy{}
 	for _, instance := range sourceInstances {
-		expectedResponse = append(expectedResponse, simplerInstanceCopy{op: instanceCopyCopy,
-			sourceDigest: instance})
+		expectedResponse = append(expectedResponse, simplerInstanceCopy{
+			op:           instanceCopyCopy,
+			sourceDigest: instance,
+		})
 		// If its `arm64` and sourceDigest[2] , expect a clone to happen
 		if instance == sourceInstances[2] {
 			expectedResponse = append(expectedResponse, simplerInstanceCopy{op: instanceCopyClone, sourceDigest: instance, cloneCompressionVariant: "zstd", clonePlatform: "arm64-linux-"})
@@ -100,8 +108,10 @@ func TestPrepareCopyInstancesforInstanceCopyClone(t *testing.T) {
 	require.NoError(t, err)
 	expectedResponse = []simplerInstanceCopy{}
 	for _, instance := range sourceInstances {
-		expectedResponse = append(expectedResponse, simplerInstanceCopy{op: instanceCopyCopy,
-			sourceDigest: instance})
+		expectedResponse = append(expectedResponse, simplerInstanceCopy{
+			op:           instanceCopyCopy,
+			sourceDigest: instance,
+		})
 		// If its `arm64` and sourceDigest[2] , expect a clone to happen
 		if instance == sourceInstances[2] {
 			expectedResponse = append(expectedResponse, simplerInstanceCopy{op: instanceCopyClone, sourceDigest: instance, cloneCompressionVariant: "zstd", clonePlatform: "arm64-linux-"})

image/copy/progress_channel_test.go

@@ -76,5 +76,4 @@ func TestReadWithEvent(t *testing.T) {
 	read, err := reader.Read(b)
 	assert.Equal(t, read, 5)
 	assert.Nil(t, err)
-
 }

image/copy/sign.go

@@ -54,7 +54,8 @@ func (c *copier) setupSigners() error {
 // and verifies that they can be used (to avoid copying a large image when we
 // can tell in advance that it would ultimately fail)
 func (c *copier) sourceSignatures(ctx context.Context, unparsed private.UnparsedImage,
-	gettingSignaturesMessage, checkingDestMessage string) ([]internalsig.Signature, error) {
+	gettingSignaturesMessage, checkingDestMessage string,
+) ([]internalsig.Signature, error) {
 	var sigs []internalsig.Signature
 	if c.options.RemoveSignatures {
 		sigs = []internalsig.Signature{}

image/copy/single.go

@@ -934,7 +934,8 @@ func updatedBlobInfoFromReuse(inputInfo types.BlobInfo, reusedBlob private.Reuse
 // perhaps (de/re/)compressing the stream,
 // and returns a complete blobInfo of the copied blob and perhaps a <-chan diffIDResult if diffIDIsNeeded, to be read by the caller.
 func (ic *imageCopier) copyLayerFromStream(ctx context.Context, srcStream io.Reader, srcInfo types.BlobInfo,
-	diffIDIsNeeded bool, toEncrypt bool, bar *progressBar, layerIndex int, emptyLayer bool) (types.BlobInfo, <-chan diffIDResult, error) {
+	diffIDIsNeeded bool, toEncrypt bool, bar *progressBar, layerIndex int, emptyLayer bool,
+) (types.BlobInfo, <-chan diffIDResult, error) {
 	var getDiffIDRecorder func(compressiontypes.DecompressorFunc) io.Writer // = nil
 	var diffIDChan chan diffIDResult
 

image/directory/directory_dest.go

@@ -90,12 +90,12 @@ func newImageDestination(sys *types.SystemContext, ref dirReference) (private.Im
 		}
 	} else {
 		// create directory if it doesn't exist
-		if err := os.MkdirAll(ref.resolvedPath, 0755); err != nil {
+		if err := os.MkdirAll(ref.resolvedPath, 0o755); err != nil {
 			return nil, fmt.Errorf("unable to create directory %q: %w", ref.resolvedPath, err)
 		}
 	}
 	// create version file
-	err = os.WriteFile(ref.versionPath(), []byte(version), 0644)
+	err = os.WriteFile(ref.versionPath(), []byte(version), 0o644)
 	if err != nil {
 		return nil, fmt.Errorf("creating version file %q: %w", ref.versionPath(), err)
 	}
@@ -170,7 +170,7 @@ func (d *dirImageDestination) PutBlobWithOptions(ctx context.Context, stream io.
 	// ignored and the file is already readable; besides, blobFile.Chmod, i.e. syscall.Fchmod,
 	// always fails on Windows.
 	if runtime.GOOS != "windows" {
-		if err := blobFile.Chmod(0644); err != nil {
+		if err := blobFile.Chmod(0o644); err != nil {
 			return private.UploadedBlob{}, err
 		}
 	}
@@ -228,7 +228,7 @@ func (d *dirImageDestination) PutManifest(ctx context.Context, manifest []byte,
 	if err != nil {
 		return err
 	}
-	return os.WriteFile(path, manifest, 0644)
+	return os.WriteFile(path, manifest, 0o644)
 }
 
 // PutSignaturesWithFormat writes a set of signatures to the destination.
@@ -245,7 +245,7 @@ func (d *dirImageDestination) PutSignaturesWithFormat(ctx context.Context, signa
 		if err != nil {
 			return err
 		}
-		if err := os.WriteFile(path, blob, 0644); err != nil {
+		if err := os.WriteFile(path, blob, 0o644); err != nil {
 			return err
 		}
 	}

image/directory/directory_test.go

@@ -17,8 +17,10 @@ import (
 	"go.podman.io/image/v5/types"
 )
 
-var _ private.ImageSource = (*dirImageSource)(nil)
-var _ private.ImageDestination = (*dirImageDestination)(nil)
+var (
+	_ private.ImageSource      = (*dirImageSource)(nil)
+	_ private.ImageDestination = (*dirImageDestination)(nil)
+)
 
 func TestDestinationReference(t *testing.T) {
 	ref, tmpDir := refToTempDir(t)