cmd: rework argument handling

This commit tweaks the argument handling based on the suggestion
by Achilleas and Ondrej (thanks!). Now it takes
```console
$ image-builder manifest qcow2 ./path/to/blueprint
...
$ image-builder manifest --arch s390x --distro centos-9 qcow2
...
```
If no arch is specified the hostname arch is used. If no distro
is specified in either the blueprint or the commandline it is
auto-detected based on the host.

Note that if the distro from the comandline and the blueprint
diverge an error is raised. We can relax this rule and just
add precedence, e.g. commandline always overrides the blueprint
but ideally we would have a clear use-case here so we start
conservative and can always relax this rule later (the inverse
is much harder).

This means it is no longer copy/paste friendly from `list-images`
by default but instead we can provide a new
`list-images --output=shell` option that outputs in exactly the
format that `image-builder [manifest|build]` need.

cmd/image-builder/distro.go

@@ -0,0 +1,30 @@
+package main
+
+import (
+	"fmt"
+
+	"github.com/osbuild/images/pkg/distro"
+)
+
+var distroGetHostDistroName = distro.GetHostDistroName
+
+// findDistro will ensure that the given distro argument do not
+// diverge. If no distro is set via the blueprint or the argument
+// the host is used to derive the distro.
+func findDistro(argDistroName, bpDistroName string) (string, error) {
+	switch {
+	case argDistroName != "" && bpDistroName != "" && argDistroName != bpDistroName:
+		return "", fmt.Errorf("error selecting distro name, cmdline argument %q is different from blueprint %q", argDistroName, bpDistroName)
+	case argDistroName != "":
+		return argDistroName, nil
+	case bpDistroName != "":
+		return bpDistroName, nil
+	}
+	// nothing selected by the user, derive from host
+	distroStr, err := distroGetHostDistroName()
+	if err != nil {
+		return "", fmt.Errorf("error deriving host distro %w", err)
+	}
+	fmt.Fprintf(osStderr, "No distro name specified, selecting %q based on host, use --distro to override", distroStr)
+	return distroStr, nil
+}

cmd/image-builder/distro_test.go

@@ -0,0 +1,48 @@
+package main_test
+
+import (
+	"bytes"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/osbuild/image-builder-cli/cmd/image-builder"
+)
+
+func TestFindDistro(t *testing.T) {
+	for _, tc := range []struct {
+		argDistro      string
+		bpDistro       string
+		expectedDistro string
+		expectedErr    string
+	}{
+		{"arg", "", "arg", ""},
+		{"", "bp", "bp", ""},
+		{"arg", "bp", "", `error selecting distro name, cmdline argument "arg" is different from blueprint "bp"`},
+		// the argDistro,bpDistro == "" case is tested below
+	} {
+		distro, err := main.FindDistro(tc.argDistro, tc.bpDistro)
+		if tc.expectedErr != "" {
+			assert.Equal(t, tc.expectedErr, err.Error())
+		} else {
+			assert.NoError(t, err)
+			assert.Equal(t, tc.expectedDistro, distro)
+		}
+	}
+}
+
+func TestFindDistroAutoDetect(t *testing.T) {
+	var buf bytes.Buffer
+	restore := main.MockOsStderr(&buf)
+	defer restore()
+
+	restore = main.MockDistroGetHostDistroName(func() (string, error) {
+		return "mocked-host-distro", nil
+	})
+	defer restore()
+
+	distro, err := main.FindDistro("", "")
+	assert.NoError(t, err)
+	assert.Equal(t, "mocked-host-distro", distro)
+	assert.Equal(t, `No distro name specified, selecting "mocked-host-distro" based on host, use --distro to override`, buf.String())
+}

cmd/image-builder/export_test.go

@@ -11,6 +11,7 @@ import (
 var (
 	GetOneImage = getOneImage
 	Run         = run
+	FindDistro  = findDistro
 )
 
 func MockOsArgs(new []string) (restore func()) {
@@ -49,3 +50,11 @@ func MockNewRepoRegistry(f func() (*reporegistry.RepoRegistry, error)) (restore
 		newRepoRegistry = saved
 	}
 }
+
+func MockDistroGetHostDistroName(f func() (string, error)) (restore func()) {
+	saved := distroGetHostDistroName
+	distroGetHostDistroName = f
+	return func() {
+		distroGetHostDistroName = saved
+	}
+}

cmd/image-builder/main.go

@@ -41,19 +41,32 @@ func cmdManifest(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	blueprintPath, err := cmd.Flags().GetString("blueprint")
+	archStr, err := cmd.Flags().GetString("arch")
 	if err != nil {
 		return err
 	}
-
-	distroStr := args[0]
-	imgTypeStr := args[1]
-	var archStr string
-	if len(args) > 2 {
-		archStr = args[2]
-	} else {
+	if archStr == "" {
 		archStr = arch.Current().String()
 	}
+	distroStr, err := cmd.Flags().GetString("distro")
+	if err != nil {
+		return err
+	}
+
+	var blueprintPath string
+	imgTypeStr := args[0]
+	if len(args) > 1 {
+		blueprintPath = args[1]
+	}
+	bp, err := blueprintload.Load(blueprintPath)
+	if err != nil {
+		return err
+	}
+	distroStr, err = findDistro(distroStr, bp.Distro)
+	if err != nil {
+		return err
+	}
+
 	res, err := getOneImage(dataDir, distroStr, imgTypeStr, archStr)
 	if err != nil {
 		return err
@@ -69,10 +82,6 @@ func cmdManifest(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	bp, err := blueprintload.Load(blueprintPath)
-	if err != nil {
-		return err
-	}
 
 	return mg.Generate(bp, res.Distro, res.ImgType, res.Arch, nil)
 }
@@ -108,15 +117,15 @@ operating sytsems like centos and RHEL with easy customizations support.`,
 	rootCmd.AddCommand(listImagesCmd)
 
 	manifestCmd := &cobra.Command{
-		Use:          "manifest <distro> <image-type> [<arch>]",
+		Use:          "manifest <image-type> [blueprint]",
 		Short:        "Build manifest for the given distro/image-type, e.g. centos-9 qcow2",
 		RunE:         cmdManifest,
 		SilenceUsage: true,
-		Args:         cobra.MinimumNArgs(2),
+		Args:         cobra.RangeArgs(1, 2),
 		Hidden:       true,
 	}
-	// XXX: share with build
-	manifestCmd.Flags().String("blueprint", "", `pass a blueprint file`)
+	manifestCmd.Flags().String("arch", "", `build manifest for a different architecture`)
+	manifestCmd.Flags().String("distro", "", `build manifest for a different distroname (e.g. centos-9)`)
 	rootCmd.AddCommand(manifestCmd)
 
 	return rootCmd.Execute()

cmd/image-builder/main_test.go

@@ -170,9 +170,10 @@ func TestManifestIntegrationSmoke(t *testing.T) {
 
 	restore = main.MockOsArgs([]string{
 		"manifest",
-		"--blueprint", makeTestBlueprint(t, testBlueprint),
-		"centos-9", "qcow2"},
-	)
+		"qcow2",
+		"--distro=centos-9",
+		makeTestBlueprint(t, testBlueprint),
+	})
 	defer restore()
 
 	var fakeStdout bytes.Buffer
@@ -190,3 +191,37 @@ func TestManifestIntegrationSmoke(t *testing.T) {
 	assert.Contains(t, fakeStdout.String(), `{"type":"org.osbuild.users","options":{"users":{"alice":{}}}}`)
 	assert.Contains(t, fakeStdout.String(), `"image":{"name":"registry.gitlab.com/redhat/services/products/image-builder/ci/osbuild-composer/fedora-minimal"`)
 }
+
+func TestManifestIntegrationCrossArch(t *testing.T) {
+	if testing.Short() {
+		t.Skip("manifest generation takes a while")
+	}
+	if !hasDepsolveDnf() {
+		t.Skip("no osbuild-depsolve-dnf binary found")
+	}
+
+	restore := main.MockNewRepoRegistry(testrepos.New)
+	defer restore()
+
+	restore = main.MockOsArgs([]string{
+		"manifest",
+		"tar",
+		"--distro", "centos-9",
+		"--arch", "s390x",
+	})
+	defer restore()
+
+	var fakeStdout bytes.Buffer
+	restore = main.MockOsStdout(&fakeStdout)
+	defer restore()
+
+	err := main.Run()
+	assert.NoError(t, err)
+
+	pipelineNames, err := manifesttest.PipelineNamesFrom(fakeStdout.Bytes())
+	assert.NoError(t, err)
+	assert.Contains(t, pipelineNames, "archive")
+
+	// XXX: provide helpers in manifesttest to extract this in a nicer way
+	assert.Contains(t, fakeStdout.String(), `.el9.s390x.rpm`)
+}