From d00e76ced121ee87ec4b2e44b4befb374d89636b Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Thu, 20 Mar 2025 11:12:33 +0100
Subject: [PATCH] main: tweak handling of --output-name to avoid adding double
 extensions

This commit tweaks the handling of the `--output-name` option so
that is a name with the same extension as the image is passed that
is just silently ignored. Its a common issue that first time
users run:
```console
$ image-builder build --output-name foo.qcow2 qcow2
```
which currently leads to a foo.qcow2.qcow2. With this commit
the expected "foo.qcow2" will appear.
---
 cmd/image-builder/export_test.go |  1 +
 cmd/image-builder/main.go        | 13 +++++++++++++
 cmd/image-builder/main_test.go   | 32 +++++++++++++++++++++++++++++++-
 cmd/image-builder/manifest.go    |  4 +---
 4 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/cmd/image-builder/export_test.go b/cmd/image-builder/export_test.go
index 28df1aa..7cd28a5 100644
--- a/cmd/image-builder/export_test.go
+++ b/cmd/image-builder/export_test.go
@@ -16,6 +16,7 @@ var (
 	FindDistro      = findDistro
 	DescribeImage   = describeImage
 	ProgressFromCmd = progressFromCmd
+	BasenameFor     = basenameFor
 )
 
 func MockOsArgs(new []string) (restore func()) {
diff --git a/cmd/image-builder/main.go b/cmd/image-builder/main.go
index 48ba064..bb9cee1 100644
--- a/cmd/image-builder/main.go
+++ b/cmd/image-builder/main.go
@@ -8,6 +8,7 @@ import (
 	"os"
 	"os/signal"
 	"path/filepath"
+
 	"strings"
 	"syscall"
 
@@ -32,6 +33,18 @@ var (
 // for the given imageType. This can be user overriden via userBasename.
 func basenameFor(img *imagefilter.Result, userBasename string) string {
 	if userBasename != "" {
+		// If the user provided a basename that already has the
+		// image extension just strip that off. I.e. when
+		// we get "foo.qcow2" for a qcow out basename is just
+		// "foo". This is mostly for convenience for our users.
+		//
+		// This code assumes that all our ImgType filesnames have
+		// $name.$ext.$extraExt (e.g. disk.qcow2 or disk.raw.xz)
+		l := strings.SplitN(img.ImgType.Filename(), ".", 2)
+		if len(l) > 1 && l[1] != "" {
+			imgExt := fmt.Sprintf(".%s", l[1])
+			userBasename = strings.TrimSuffix(userBasename, imgExt)
+		}
 		return userBasename
 	}
 	return fmt.Sprintf("%s-%s-%s", img.Distro.Name(), img.ImgType.Name(), img.Arch.Name())
diff --git a/cmd/image-builder/main_test.go b/cmd/image-builder/main_test.go
index 70f0f90..bf7eb65 100644
--- a/cmd/image-builder/main_test.go
+++ b/cmd/image-builder/main_test.go
@@ -892,7 +892,9 @@ func TestBuildIntegrationOutputFilename(t *testing.T) {
 		"--distro", "centos-9",
 		"--cache", tmpdir,
 		"--output-dir", outputDir,
-		"--output-name=foo.n.0",
+		// XXX: also test --output-name="foo.n.0" here which should
+		// have exactly the same result (once the depsolving is mocked)
+		"--output-name=foo.n.0.qcow2",
 		"--with-manifest",
 		"--with-sbom",
 		"--with-buildlog",
@@ -921,3 +923,31 @@ func TestBuildIntegrationOutputFilename(t *testing.T) {
 		assert.NoError(t, err, fmt.Sprintf("file %q missing from %v", expected, files))
 	}
 }
+
+func TestBasenameFor(t *testing.T) {
+	restore := main.MockNewRepoRegistry(testrepos.New)
+	defer restore()
+
+	for _, tc := range []struct {
+		imgTypeName string
+		basename    string
+		expected    string
+	}{
+		// no user provided output name
+		{"qcow2", "", "centos-9-qcow2-x86_64"},
+		{"minimal-raw", "", "centos-9-minimal-raw-x86_64"},
+		// simple
+		{"qcow2", "foo", "foo"},
+		{"qcow2", "foo.n.0", "foo.n.0"},
+		// with extension
+		{"qcow2", "foo.n.0.qcow2", "foo.n.0"},
+		{"minimal-raw", "foo.n.0.raw.xz", "foo.n.0"},
+		// with the "wrong" extension, we just ignore that and trust
+		// the user (what else could we do?)
+		{"qcow2", "foo.n.0.raw", "foo.n.0.raw"},
+	} {
+		res, err := main.GetOneImage("centos-9", tc.imgTypeName, "x86_64", nil)
+		require.NoError(t, err)
+		assert.Equal(t, tc.expected, main.BasenameFor(res, tc.basename))
+	}
+}
diff --git a/cmd/image-builder/manifest.go b/cmd/image-builder/manifest.go
index 3c1db7b..d2f8b12 100644
--- a/cmd/image-builder/manifest.go
+++ b/cmd/image-builder/manifest.go
@@ -59,9 +59,7 @@ func generateManifest(dataDir string, extraRepos []string, img *imagefilter.Resu
 	if opts.WithSBOM {
 		outputDir := basenameFor(img, opts.OutputDir)
 		manifestGenOpts.SBOMWriter = func(filename string, content io.Reader, docType sbom.StandardType) error {
-			if opts.OutputFilename != "" {
-				filename = fmt.Sprintf("%s.%s", opts.OutputFilename, strings.SplitN(filename, ".", 2)[1])
-			}
+			filename = fmt.Sprintf("%s.%s", basenameFor(img, opts.OutputFilename), strings.SplitN(filename, ".", 2)[1])
 			return sbomWriter(outputDir, filename, content)
 		}
 	}