From 362ca082f2b374b0d3bea257abb6a422f661ddcf Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Sat, 25 Jan 2025 22:16:59 +0100
Subject: [PATCH 1/5] progress: tweak progress error reporting

This commit adds catpure of os.Std{out,err} when running osbuild so
that we capture the error log and can display it as part of
an error from osbuild, e.g. when osbuild itself crashes.

This gives us more accurate error reporting if anything fails
during the osbuild building.
---
 bib/pkg/progress/export_test.go   |  8 ++++++++
 bib/pkg/progress/progress.go      | 21 +++++++--------------
 bib/pkg/progress/progress_test.go | 26 ++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/bib/pkg/progress/export_test.go b/bib/pkg/progress/export_test.go
index 252d5329f..ae12d166e 100644
--- a/bib/pkg/progress/export_test.go
+++ b/bib/pkg/progress/export_test.go
@@ -25,3 +25,11 @@ func MockIsattyIsTerminal(fn func(uintptr) bool) (restore func()) {
 		isattyIsTerminal = saved
 	}
 }
+
+func MockOsbuildCmd(s string) (restore func()) {
+	saved := osbuildCmd
+	osbuildCmd = s
+	return func() {
+		osbuildCmd = saved
+	}
+}
diff --git a/bib/pkg/progress/progress.go b/bib/pkg/progress/progress.go
index 2112b3976..1e1a3a541 100644
--- a/bib/pkg/progress/progress.go
+++ b/bib/pkg/progress/progress.go
@@ -333,6 +333,8 @@ func runOSBuildNoProgress(pb ProgressBar, manifest []byte, store, outputDirector
 	return err
 }
 
+var osbuildCmd = "osbuild"
+
 func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirectory string, exports, extraEnv []string) error {
 	rp, wp, err := os.Pipe()
 	if err != nil {
@@ -342,7 +344,7 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirect
 	defer wp.Close()
 
 	cmd := exec.Command(
-		"osbuild",
+		osbuildCmd,
 		"--store", store,
 		"--output-directory", outputDirectory,
 		"--monitor=JSONSeqMonitor",
@@ -353,12 +355,11 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirect
 		cmd.Args = append(cmd.Args, "--export", export)
 	}
 
+	var stdio bytes.Buffer
 	cmd.Env = append(os.Environ(), extraEnv...)
 	cmd.Stdin = bytes.NewBuffer(manifest)
-	cmd.Stderr = os.Stderr
-	// we could use "--json" here and would get the build-result
-	// exported here
-	cmd.Stdout = nil
+	cmd.Stdout = &stdio
+	cmd.Stderr = &stdio
 	cmd.ExtraFiles = []*os.File{wp}
 
 	osbuildStatus := osbuild.NewStatusScanner(rp)
@@ -367,7 +368,6 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirect
 	}
 	wp.Close()
 
-	var tracesMsgs []string
 	for {
 		st, err := osbuildStatus.Status()
 		if err != nil {
@@ -383,13 +383,6 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirect
 			}
 			i++
 		}
-		// keep the messages/traces for better error reporting
-		if st.Message != "" {
-			tracesMsgs = append(tracesMsgs, st.Message)
-		}
-		if st.Trace != "" {
-			tracesMsgs = append(tracesMsgs, st.Trace)
-		}
 		// forward to user
 		if st.Message != "" {
 			pb.SetMessagef(st.Message)
@@ -397,7 +390,7 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirect
 	}
 
 	if err := cmd.Wait(); err != nil {
-		return fmt.Errorf("error running osbuild: %w\nLog:\n%s", err, strings.Join(tracesMsgs, "\n"))
+		return fmt.Errorf("error running osbuild: %w\nOutput:\n%s", err, stdio.String())
 	}
 
 	return nil
diff --git a/bib/pkg/progress/progress_test.go b/bib/pkg/progress/progress_test.go
index 67a720d16..31734ec9b 100644
--- a/bib/pkg/progress/progress_test.go
+++ b/bib/pkg/progress/progress_test.go
@@ -3,6 +3,8 @@ package progress_test
 import (
 	"bytes"
 	"fmt"
+	"os"
+	"path/filepath"
 	"reflect"
 	"testing"
 
@@ -130,3 +132,27 @@ func TestProgressNewAutoselect(t *testing.T) {
 		assert.Equal(t, reflect.TypeOf(pb), reflect.TypeOf(tc.expected), fmt.Sprintf("[%v] %T not the expected %T", tc.onTerm, pb, tc.expected))
 	}
 }
+
+func makeFakeOsbuild(t *testing.T, content string) string {
+	p := filepath.Join(t.TempDir(), "fake-osbuild")
+	err := os.WriteFile(p, []byte("#!/bin/sh\n"+content), 0755)
+	assert.NoError(t, err)
+	return p
+}
+
+func TestRunOSBuildWithProgress(t *testing.T) {
+	restore := progress.MockOsbuildCmd(makeFakeOsbuild(t, `echo osbuild-stdout-output
+>&2 echo osbuild-stderr-output
+exit 112
+`))
+	defer restore()
+
+	pbar, err := progress.New("debug")
+	assert.NoError(t, err)
+	err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), "", "", nil, nil)
+	assert.EqualError(t, err, `error running osbuild: exit status 112
+Output:
+osbuild-stdout-output
+osbuild-stderr-output
+`)
+}

From 4833fb4bf584cfcbe2c3fd6d8f0e05860b96d5ef Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Mon, 27 Jan 2025 13:53:23 +0100
Subject: [PATCH 2/5] progress: do not return if osbuild status json reading
 fails

This commit tweaks an issue that potentially an incorrect status
from osbuild would fail the build with a bad error message and
without us getting the full buildlog.
---
 bib/pkg/progress/progress.go      |  7 ++++++-
 bib/pkg/progress/progress_test.go | 16 +++++++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/bib/pkg/progress/progress.go b/bib/pkg/progress/progress.go
index 1e1a3a541..44f7da7c6 100644
--- a/bib/pkg/progress/progress.go
+++ b/bib/pkg/progress/progress.go
@@ -368,10 +368,12 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirect
 	}
 	wp.Close()
 
+	var statusErrs []error
 	for {
 		st, err := osbuildStatus.Status()
 		if err != nil {
-			return fmt.Errorf("error reading osbuild status: %w", err)
+			statusErrs = append(statusErrs, err)
+			continue
 		}
 		if st == nil {
 			break
@@ -392,6 +394,9 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirect
 	if err := cmd.Wait(); err != nil {
 		return fmt.Errorf("error running osbuild: %w\nOutput:\n%s", err, stdio.String())
 	}
+	if len(statusErrs) > 0 {
+		return fmt.Errorf("errors parsing osbuild status:\n%w", errors.Join(statusErrs...))
+	}
 
 	return nil
 }
diff --git a/bib/pkg/progress/progress_test.go b/bib/pkg/progress/progress_test.go
index 31734ec9b..8bd2f93b5 100644
--- a/bib/pkg/progress/progress_test.go
+++ b/bib/pkg/progress/progress_test.go
@@ -140,7 +140,7 @@ func makeFakeOsbuild(t *testing.T, content string) string {
 	return p
 }
 
-func TestRunOSBuildWithProgress(t *testing.T) {
+func TestRunOSBuildWithProgressErrorReporting(t *testing.T) {
 	restore := progress.MockOsbuildCmd(makeFakeOsbuild(t, `echo osbuild-stdout-output
 >&2 echo osbuild-stderr-output
 exit 112
@@ -156,3 +156,17 @@ osbuild-stdout-output
 osbuild-stderr-output
 `)
 }
+
+func TestRunOSBuildWithProgressIncorrectJSON(t *testing.T) {
+	restore := progress.MockOsbuildCmd(makeFakeOsbuild(t, `echo osbuild-stdout-output
+>&2 echo osbuild-stderr-output
+>&3 echo invalid-json
+`))
+	defer restore()
+
+	pbar, err := progress.New("debug")
+	assert.NoError(t, err)
+	err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), "", "", nil, nil)
+	assert.EqualError(t, err, `errors parsing osbuild status:
+cannot scan line "invalid-json": invalid character 'i' looking for beginning of value`)
+}

From 7143525f820d41c25fc2625c7ad6944023f3eccd Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Mon, 27 Jan 2025 11:52:08 +0100
Subject: [PATCH 3/5] progress: add `progress.OSBuildOptions` struct

This commit adds a `OSBuildOptions` struct that can be used to
pass (optional) options to the `progress.RunOSBuild()` helper.

This make it easier to expand with more options.
---
 bib/cmd/bootc-image-builder/main.go |  7 ++++++-
 bib/pkg/progress/progress.go        | 28 +++++++++++++++++++---------
 bib/pkg/progress/progress_test.go   |  2 +-
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/bib/cmd/bootc-image-builder/main.go b/bib/cmd/bootc-image-builder/main.go
index 68daeedc9..50d32ef14 100644
--- a/bib/cmd/bootc-image-builder/main.go
+++ b/bib/cmd/bootc-image-builder/main.go
@@ -488,7 +488,12 @@ func cmdBuild(cmd *cobra.Command, args []string) error {
 		osbuildEnv = append(osbuildEnv, envVars...)
 	}
 
-	if err = progress.RunOSBuild(pbar, mf, osbuildStore, outputDir, exports, osbuildEnv); err != nil {
+	osbuildOpts := progress.OSBuildOptions{
+		StoreDir:  osbuildStore,
+		OutputDir: outputDir,
+		ExtraEnv:  osbuildEnv,
+	}
+	if err = progress.RunOSBuild(pbar, mf, exports, &osbuildOpts); err != nil {
 		return fmt.Errorf("cannot run osbuild: %w", err)
 	}
 
diff --git a/bib/pkg/progress/progress.go b/bib/pkg/progress/progress.go
index 44f7da7c6..d406cf35f 100644
--- a/bib/pkg/progress/progress.go
+++ b/bib/pkg/progress/progress.go
@@ -311,8 +311,18 @@ func (b *debugProgressBar) SetProgress(subLevel int, msg string, done int, total
 	return nil
 }
 
+type OSBuildOptions struct {
+	StoreDir  string
+	OutputDir string
+	ExtraEnv  []string
+}
+
 // XXX: merge variant back into images/pkg/osbuild/osbuild-exec.go
-func RunOSBuild(pb ProgressBar, manifest []byte, store, outputDirectory string, exports, extraEnv []string) error {
+func RunOSBuild(pb ProgressBar, manifest []byte, exports []string, opts *OSBuildOptions) error {
+	if opts == nil {
+		opts = &OSBuildOptions{}
+	}
+
 	// To keep maximum compatibility keep the old behavior to run osbuild
 	// directly and show all messages unless we have a "real" progress bar.
 	//
@@ -322,20 +332,20 @@ func RunOSBuild(pb ProgressBar, manifest []byte, store, outputDirectory string,
 	// just run with the new runOSBuildWithProgress() helper.
 	switch pb.(type) {
 	case *terminalProgressBar, *debugProgressBar:
-		return runOSBuildWithProgress(pb, manifest, store, outputDirectory, exports, extraEnv)
+		return runOSBuildWithProgress(pb, manifest, exports, opts)
 	default:
-		return runOSBuildNoProgress(pb, manifest, store, outputDirectory, exports, extraEnv)
+		return runOSBuildNoProgress(pb, manifest, exports, opts)
 	}
 }
 
-func runOSBuildNoProgress(pb ProgressBar, manifest []byte, store, outputDirectory string, exports, extraEnv []string) error {
-	_, err := osbuild.RunOSBuild(manifest, store, outputDirectory, exports, nil, extraEnv, false, os.Stderr)
+func runOSBuildNoProgress(pb ProgressBar, manifest []byte, exports []string, opts *OSBuildOptions) error {
+	_, err := osbuild.RunOSBuild(manifest, opts.StoreDir, opts.OutputDir, exports, nil, opts.ExtraEnv, false, os.Stderr)
 	return err
 }
 
 var osbuildCmd = "osbuild"
 
-func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirectory string, exports, extraEnv []string) error {
+func runOSBuildWithProgress(pb ProgressBar, manifest []byte, exports []string, opts *OSBuildOptions) error {
 	rp, wp, err := os.Pipe()
 	if err != nil {
 		return fmt.Errorf("cannot create pipe for osbuild: %w", err)
@@ -345,8 +355,8 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirect
 
 	cmd := exec.Command(
 		osbuildCmd,
-		"--store", store,
-		"--output-directory", outputDirectory,
+		"--store", opts.StoreDir,
+		"--output-directory", opts.OutputDir,
 		"--monitor=JSONSeqMonitor",
 		"--monitor-fd=3",
 		"-",
@@ -356,7 +366,7 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, store, outputDirect
 	}
 
 	var stdio bytes.Buffer
-	cmd.Env = append(os.Environ(), extraEnv...)
+	cmd.Env = append(os.Environ(), opts.ExtraEnv...)
 	cmd.Stdin = bytes.NewBuffer(manifest)
 	cmd.Stdout = &stdio
 	cmd.Stderr = &stdio
diff --git a/bib/pkg/progress/progress_test.go b/bib/pkg/progress/progress_test.go
index 8bd2f93b5..9e131d303 100644
--- a/bib/pkg/progress/progress_test.go
+++ b/bib/pkg/progress/progress_test.go
@@ -149,7 +149,7 @@ exit 112
 
 	pbar, err := progress.New("debug")
 	assert.NoError(t, err)
-	err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), "", "", nil, nil)
+	err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), nil, nil)
 	assert.EqualError(t, err, `error running osbuild: exit status 112
 Output:
 osbuild-stdout-output

From f4ee6892c4fc6a0ff0022d19eaf9c0c1ac88fdad Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Mon, 27 Jan 2025 12:46:27 +0100
Subject: [PATCH 4/5] progress: add new `BuildLog` option

This commit adds a new `BuildLog` option to the `OSBuildOptions`
that can be used to generate a streamed buildlog (e.g. to a file
or a websocket).

This will be used in `ibcli` with a new `--with-buildlog` option.
---
 bib/pkg/progress/export_test.go   |  8 ++++
 bib/pkg/progress/progress.go      | 67 +++++++++++++++++++++----------
 bib/pkg/progress/progress_test.go | 35 +++++++++++++++-
 3 files changed, 88 insertions(+), 22 deletions(-)

diff --git a/bib/pkg/progress/export_test.go b/bib/pkg/progress/export_test.go
index ae12d166e..720dd3ff1 100644
--- a/bib/pkg/progress/export_test.go
+++ b/bib/pkg/progress/export_test.go
@@ -10,6 +10,14 @@ type (
 	VerboseProgressBar  = verboseProgressBar
 )
 
+func MockOsStdout(w io.Writer) (restore func()) {
+	saved := osStdout
+	osStdout = w
+	return func() {
+		osStdout = saved
+	}
+}
+
 func MockOsStderr(w io.Writer) (restore func()) {
 	saved := osStderr
 	osStderr = w
diff --git a/bib/pkg/progress/progress.go b/bib/pkg/progress/progress.go
index d406cf35f..b29b38fe2 100644
--- a/bib/pkg/progress/progress.go
+++ b/bib/pkg/progress/progress.go
@@ -18,6 +18,7 @@ import (
 )
 
 var (
+	osStdout io.Writer = os.Stdout
 	osStderr io.Writer = os.Stderr
 
 	// This is only needed because pb.Pool require a real terminal.
@@ -315,37 +316,60 @@ type OSBuildOptions struct {
 	StoreDir  string
 	OutputDir string
 	ExtraEnv  []string
-}
 
-// XXX: merge variant back into images/pkg/osbuild/osbuild-exec.go
-func RunOSBuild(pb ProgressBar, manifest []byte, exports []string, opts *OSBuildOptions) error {
-	if opts == nil {
-		opts = &OSBuildOptions{}
-	}
+	// BuildLog writes the osbuild output to the given writer
+	BuildLog io.Writer
+}
 
+func writersForOSBuild(pb ProgressBar, opts *OSBuildOptions, internalBuildLog io.Writer) (osbuildStdout io.Writer, osbuildStderr io.Writer) {
 	// To keep maximum compatibility keep the old behavior to run osbuild
 	// directly and show all messages unless we have a "real" progress bar.
 	//
 	// This should ensure that e.g. "podman bootc" keeps working as it
-	// is currently expecting the raw osbuild output. Once we double
-	// checked with them we can remove the runOSBuildNoProgress() and
-	// just run with the new runOSBuildWithProgress() helper.
+	// is currently expecting the raw osbuild output.
 	switch pb.(type) {
-	case *terminalProgressBar, *debugProgressBar:
-		return runOSBuildWithProgress(pb, manifest, exports, opts)
+	case *verboseProgressBar:
+		// No external build log requested and we won't need an
+		// internal one because all output goes directly to
+		// stdout/stderr. This is for maximum compatibility with
+		// the existing bootc-image-builder in "verbose" mode
+		// where stdout, stderr come directly from osbuild.
+		if opts.BuildLog == nil {
+			return osStdout, osStderr
+		}
+		// With a build log we need a single output stream
+		osbuildStdout = osStdout
 	default:
-		return runOSBuildNoProgress(pb, manifest, exports, opts)
+		// hide the direct osbuild output by default
+		osbuildStdout = io.Discard
 	}
-}
 
-func runOSBuildNoProgress(pb ProgressBar, manifest []byte, exports []string, opts *OSBuildOptions) error {
-	_, err := osbuild.RunOSBuild(manifest, opts.StoreDir, opts.OutputDir, exports, nil, opts.ExtraEnv, false, os.Stderr)
-	return err
+	// There is a slight wrinkle here: when requesting a buildlog
+	// we can no longer write to separate stdout/stderr streams
+	// without being racy and give potential out-of-order output
+	// (which is very bad and confusing in a log). The reason is
+	// that if cmd.Std{out,err} are different "go" will start two
+	// go-routine to monitor/copy those are racy when both stdout,stderr
+	// output happens close together (TestRunOSBuildWithBuildlog demos
+	// that). We cannot have our cake and eat it so here we need to
+	// combine osbuilds stderr into our stdout.
+	if opts.BuildLog == nil {
+		opts.BuildLog = io.Discard
+	}
+	mw := io.MultiWriter(osbuildStdout, internalBuildLog, opts.BuildLog)
+	return mw, mw
 }
 
 var osbuildCmd = "osbuild"
 
-func runOSBuildWithProgress(pb ProgressBar, manifest []byte, exports []string, opts *OSBuildOptions) error {
+// XXX: merge variant back into images/pkg/osbuild/osbuild-exec.go
+func RunOSBuild(pb ProgressBar, manifest []byte, exports []string, opts *OSBuildOptions) error {
+	if opts == nil {
+		opts = &OSBuildOptions{}
+	}
+	var internalBuildLog bytes.Buffer
+	osbuildStdout, osbuildStderr := writersForOSBuild(pb, opts, &internalBuildLog)
+
 	rp, wp, err := os.Pipe()
 	if err != nil {
 		return fmt.Errorf("cannot create pipe for osbuild: %w", err)
@@ -365,11 +389,10 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, exports []string, o
 		cmd.Args = append(cmd.Args, "--export", export)
 	}
 
-	var stdio bytes.Buffer
 	cmd.Env = append(os.Environ(), opts.ExtraEnv...)
 	cmd.Stdin = bytes.NewBuffer(manifest)
-	cmd.Stdout = &stdio
-	cmd.Stderr = &stdio
+	cmd.Stdout = osbuildStdout
+	cmd.Stderr = osbuildStderr
 	cmd.ExtraFiles = []*os.File{wp}
 
 	osbuildStatus := osbuild.NewStatusScanner(rp)
@@ -381,6 +404,8 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, exports []string, o
 	var statusErrs []error
 	for {
 		st, err := osbuildStatus.Status()
+		// XXX: we cannot exit here, this would mean we lose error
+		// information if osbuild reading fails
 		if err != nil {
 			statusErrs = append(statusErrs, err)
 			continue
@@ -402,7 +427,7 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, exports []string, o
 	}
 
 	if err := cmd.Wait(); err != nil {
-		return fmt.Errorf("error running osbuild: %w\nOutput:\n%s", err, stdio.String())
+		return fmt.Errorf("error running osbuild: %w\nOutput:\n%s", err, internalBuildLog.String())
 	}
 	if len(statusErrs) > 0 {
 		return fmt.Errorf("errors parsing osbuild status:\n%w", errors.Join(statusErrs...))
diff --git a/bib/pkg/progress/progress_test.go b/bib/pkg/progress/progress_test.go
index 9e131d303..9ba3c5abe 100644
--- a/bib/pkg/progress/progress_test.go
+++ b/bib/pkg/progress/progress_test.go
@@ -157,6 +157,38 @@ osbuild-stderr-output
 `)
 }
 
+func TestRunOSBuildWithBuildlog(t *testing.T) {
+	restore := progress.MockOsbuildCmd(makeFakeOsbuild(t, `
+echo osbuild-stdout-output
+>&2 echo osbuild-stderr-output
+`))
+	defer restore()
+
+	var fakeStdout, fakeStderr bytes.Buffer
+	restore = progress.MockOsStdout(&fakeStdout)
+	defer restore()
+	restore = progress.MockOsStderr(&fakeStderr)
+	defer restore()
+
+	for _, pbarType := range []string{"debug", "verbose"} {
+		t.Run(pbarType, func(t *testing.T) {
+			pbar, err := progress.New(pbarType)
+			assert.NoError(t, err)
+
+			var buildLog bytes.Buffer
+			opts := &progress.OSBuildOptions{
+				BuildLog: &buildLog,
+			}
+			err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), nil, opts)
+			assert.NoError(t, err)
+			expectedOutput := `osbuild-stdout-output
+osbuild-stderr-output
+`
+			assert.Equal(t, expectedOutput, buildLog.String())
+		})
+	}
+}
+
 func TestRunOSBuildWithProgressIncorrectJSON(t *testing.T) {
 	restore := progress.MockOsbuildCmd(makeFakeOsbuild(t, `echo osbuild-stdout-output
 >&2 echo osbuild-stderr-output
@@ -166,7 +198,8 @@ func TestRunOSBuildWithProgressIncorrectJSON(t *testing.T) {
 
 	pbar, err := progress.New("debug")
 	assert.NoError(t, err)
-	err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), "", "", nil, nil)
+
+	err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), nil, nil)
 	assert.EqualError(t, err, `errors parsing osbuild status:
 cannot scan line "invalid-json": invalid character 'i' looking for beginning of value`)
 }

From 6bda3d8992ad5b29bf886782cc0aab923b168338 Mon Sep 17 00:00:00 2001
From: Michael Vogt <michael.vogt@gmail.com>
Date: Mon, 27 Jan 2025 18:01:09 +0100
Subject: [PATCH 5/5] progress: set --cache-max-size in osbuild

This commit allows controlling the `osbuild --cache-max-size`
option. By default it will set the cache to 20GiB but allows
this to be controlled by the user.

Thanks to Simon for raising this.
---
 bib/pkg/progress/progress.go      |  8 ++++++++
 bib/pkg/progress/progress_test.go | 18 ++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/bib/pkg/progress/progress.go b/bib/pkg/progress/progress.go
index b29b38fe2..a59a511e9 100644
--- a/bib/pkg/progress/progress.go
+++ b/bib/pkg/progress/progress.go
@@ -14,6 +14,7 @@ import (
 	"github.com/mattn/go-isatty"
 	"github.com/sirupsen/logrus"
 
+	"github.com/osbuild/images/pkg/datasizes"
 	"github.com/osbuild/images/pkg/osbuild"
 )
 
@@ -319,6 +320,8 @@ type OSBuildOptions struct {
 
 	// BuildLog writes the osbuild output to the given writer
 	BuildLog io.Writer
+
+	CacheMaxSize int64
 }
 
 func writersForOSBuild(pb ProgressBar, opts *OSBuildOptions, internalBuildLog io.Writer) (osbuildStdout io.Writer, osbuildStderr io.Writer) {
@@ -377,12 +380,17 @@ func RunOSBuild(pb ProgressBar, manifest []byte, exports []string, opts *OSBuild
 	defer rp.Close()
 	defer wp.Close()
 
+	cacheMaxSize := int64(20 * datasizes.GiB)
+	if opts.CacheMaxSize != 0 {
+		cacheMaxSize = opts.CacheMaxSize
+	}
 	cmd := exec.Command(
 		osbuildCmd,
 		"--store", opts.StoreDir,
 		"--output-directory", opts.OutputDir,
 		"--monitor=JSONSeqMonitor",
 		"--monitor-fd=3",
+		fmt.Sprintf("--cache-max-size=%v", cacheMaxSize),
 		"-",
 	)
 	for _, export := range exports {
diff --git a/bib/pkg/progress/progress_test.go b/bib/pkg/progress/progress_test.go
index 9ba3c5abe..88d76c7d1 100644
--- a/bib/pkg/progress/progress_test.go
+++ b/bib/pkg/progress/progress_test.go
@@ -203,3 +203,21 @@ func TestRunOSBuildWithProgressIncorrectJSON(t *testing.T) {
 	assert.EqualError(t, err, `errors parsing osbuild status:
 cannot scan line "invalid-json": invalid character 'i' looking for beginning of value`)
 }
+
+func TestRunOSBuildCacheMaxSize(t *testing.T) {
+	fakeOsbuildBinary := makeFakeOsbuild(t, `echo "$@" > "$0".cmdline`)
+	restore := progress.MockOsbuildCmd(fakeOsbuildBinary)
+	defer restore()
+
+	pbar, err := progress.New("debug")
+	assert.NoError(t, err)
+
+	osbuildOpts := &progress.OSBuildOptions{
+		CacheMaxSize: 77,
+	}
+	err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), nil, osbuildOpts)
+	assert.NoError(t, err)
+	cmdline, err := os.ReadFile(fakeOsbuildBinary + ".cmdline")
+	assert.NoError(t, err)
+	assert.Contains(t, string(cmdline), "--cache-max-size=77")
+}