progress: use images/osbuild-exec as much as possible

The following moved into images:
- OSBuildOptions;
- construction of the osbuild command;
- build log support;
- synced writer;
- monitor support.

See osbuild/images#1966.

Author: croissanne

cmd/image-builder/build.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/osbuild/image-builder-cli/pkg/progress"
 	"github.com/osbuild/images/pkg/imagefilter"
+	"github.com/osbuild/images/pkg/osbuild"
 )
 
 type buildOptions struct {
@@ -35,7 +36,7 @@ func buildImage(pbar progress.ProgressBar, res *imagefilter.Result, osbuildManif
 		}
 	}
 
-	osbuildOpts := &progress.OSBuildOptions{
+	osbuildOpts := &osbuild.OSBuildOptions{
 		StoreDir:  opts.StoreDir,
 		OutputDir: opts.OutputDir,
 	}

go.mod

@@ -141,3 +141,5 @@ require (
 	gopkg.in/ini.v1 v1.67.0 // indirect
 	libvirt.org/go/libvirt v1.11006.0 // indirect
 )
+
+replace github.com/osbuild/images => github.com/croissanne/images v0.0.0-20251023114920-81eefc443b5e

go.sum

@@ -90,6 +90,8 @@ github.com/containers/storage v1.59.1/go.mod h1:KoAYHnAjP3/cTsRS+mmWZGkufSY2GACi
 github.com/coreos/go-semver v0.3.1 h1:yi21YpKnrx1gt5R+la8n5WgS0kCrsPp33dmEyHReZr4=
 github.com/coreos/go-semver v0.3.1/go.mod h1:irMmmIw/7yzSRPWryHsK7EYSg09caPQL03VsM8rvUec=
 github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
+github.com/croissanne/images v0.0.0-20251023114920-81eefc443b5e h1:MuftBLwWLE8gWoJY1/P26tw0TL9HIkb4LcoTepZjVyY=
+github.com/croissanne/images v0.0.0-20251023114920-81eefc443b5e/go.mod h1:tZqcrs3eNUA0VPs1h3YCnbnpAskVVfo36CIi2USSfDs=
 github.com/cyberphone/json-canonicalization v0.0.0-20241213102144-19d51d7fe467 h1:uX1JmpONuD549D73r6cgnxyUu18Zb7yHAy5AYU0Pm4Q=
 github.com/cyberphone/json-canonicalization v0.0.0-20241213102144-19d51d7fe467/go.mod h1:uzvlm1mxhHkdfqitSA92i7Se+S9ksOn3a3qmv/kyOCw=
 github.com/cyphar/filepath-securejoin v0.4.1 h1:JyxxyPEaktOD+GAnqIqTf9A8tHyAG22rowi7HkoSU1s=
@@ -245,10 +247,6 @@ github.com/opencontainers/selinux v1.12.0 h1:6n5JV4Cf+4y0KNXW48TLj5DwfXpvWlxXplU
 github.com/opencontainers/selinux v1.12.0/go.mod h1:BTPX+bjVbWGXw7ZZWUbdENt8w0htPSrlgOOysQaU62U=
 github.com/osbuild/blueprint v1.16.0 h1:f/kHih+xpeJ1v7wtIfzdHPZTsiXsqKeDQ1+rrue6298=
 github.com/osbuild/blueprint v1.16.0/go.mod h1:HPlJzkEl7q5g8hzaGksUk7ifFAy9QFw9LmzhuFOAVm4=
-github.com/osbuild/images v0.206.0 h1:F9G6dnqrURRUcWE2eWIPQLzHutSCT0OyWMcITK28uCQ=
-github.com/osbuild/images v0.206.0/go.mod h1:iF6bTLzBtyp9l27fexsD5AzwHEn9+bXF5Jr4HHQecmI=
-github.com/osbuild/images v0.209.0 h1:9BRf+N0op1WbQkc+7zVRBZxg4dqS4lty3i2stF3G9lo=
-github.com/osbuild/images v0.209.0/go.mod h1:tZqcrs3eNUA0VPs1h3YCnbnpAskVVfo36CIi2USSfDs=
 github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
 github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=

pkg/progress/command.go

@@ -7,30 +7,17 @@ import (
 	"io"
 	"log"
 	"os"
-	"os/exec"
 	"strings"
 	"sync"
 	"syscall"
 
-	"github.com/osbuild/images/pkg/datasizes"
 	"github.com/osbuild/images/pkg/osbuild"
 )
 
-type OSBuildOptions struct {
-	StoreDir  string
-	OutputDir string
-	ExtraEnv  []string
-
-	// BuildLog writes the osbuild output to the given writer
-	BuildLog io.Writer
-
-	CacheMaxSize int64
-}
-
 // XXX: merge variant back into images/pkg/osbuild/osbuild-exec.go
-func RunOSBuild(pb ProgressBar, manifest []byte, exports []string, opts *OSBuildOptions) error {
+func RunOSBuild(pb ProgressBar, manifest []byte, exports []string, opts *osbuild.OSBuildOptions) error {
 	if opts == nil {
-		opts = &OSBuildOptions{}
+		opts = &osbuild.OSBuildOptions{}
 	}
 
 	// To keep maximum compatibility keep the old behavior to run osbuild
@@ -48,92 +35,37 @@ func RunOSBuild(pb ProgressBar, manifest []byte, exports []string, opts *OSBuild
 	}
 }
 
-func newOsbuildCmd(manifest []byte, exports []string, opts *OSBuildOptions) *exec.Cmd {
-	cacheMaxSize := int64(20 * datasizes.GiB)
-	if opts.CacheMaxSize != 0 {
-		cacheMaxSize = opts.CacheMaxSize
-	}
-	cmd := exec.Command(
-		osbuildCmd,
-		"--store", opts.StoreDir,
-		"--output-directory", opts.OutputDir,
-		fmt.Sprintf("--cache-max-size=%v", cacheMaxSize),
-		"-",
-	)
-	for _, export := range exports {
-		cmd.Args = append(cmd.Args, "--export", export)
-	}
-	cmd.Env = append(os.Environ(), opts.ExtraEnv...)
-	cmd.Stdin = bytes.NewBuffer(manifest)
-	return cmd
-}
-
-func runOSBuildNoProgress(pb ProgressBar, manifest []byte, exports []string, opts *OSBuildOptions) error {
-	var stdout, stderr io.Writer
-
-	var writeMu sync.Mutex
-	if opts.BuildLog == nil {
-		// 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.
-		stdout = osStdout()
-		stderr = osStderr()
-	} else {
-		// 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.
-		mw := newSyncedWriter(&writeMu, io.MultiWriter(osStdout(), opts.BuildLog))
-		stdout = mw
-		stderr = mw
-	}
-
-	cmd := newOsbuildCmd(manifest, exports, opts)
-	cmd.Stdout = stdout
-	cmd.Stderr = stderr
+func runOSBuildNoProgress(pb ProgressBar, manifest []byte, exports []string, opts *osbuild.OSBuildOptions) error {
+	cmd := osbuild.NewOSBuildCmd(manifest, exports, opts)
 	if err := cmd.Run(); err != nil {
 		return fmt.Errorf("error running osbuild: %w", err)
 	}
 	return nil
 }
 
-var osbuildCmd = "osbuild"
-
-func runOSBuildWithProgress(pb ProgressBar, manifest []byte, exports []string, opts *OSBuildOptions) (err error) {
+func runOSBuildWithProgress(pb ProgressBar, manifest []byte, exports []string, opts *osbuild.OSBuildOptions) (err error) {
 	rp, wp, err := os.Pipe()
 	if err != nil {
 		return fmt.Errorf("cannot create pipe for osbuild: %w", err)
 	}
 	defer rp.Close()
 	defer wp.Close()
 
-	cmd := newOsbuildCmd(manifest, exports, opts)
-	cmd.Args = append(cmd.Args, "--monitor=JSONSeqMonitor")
-	cmd.Args = append(cmd.Args, "--monitor-fd=3")
 
+	opts.Monitor = osbuild.MonitorJSONSeq
+	opts.MonitorFD = 3
+	opts.MonitorFile = wp
 	var stdio bytes.Buffer
-	var mw, buildLog io.Writer
-	var writeMu sync.Mutex
-	if opts.BuildLog != nil {
-		mw = newSyncedWriter(&writeMu, io.MultiWriter(&stdio, opts.BuildLog))
-		buildLog = newSyncedWriter(&writeMu, opts.BuildLog)
-	} else {
-		mw = &stdio
+	var mu sync.Mutex
+	buildLog := opts.BuildLog
+	opts.BuildLogMu = &mu
+	if opts.BuildLog == nil {
+		mw := &stdio
+		opts.Stdout = mw
+		opts.Stderr = mw
 		buildLog = io.Discard
 	}
-
-	cmd.Stdout = mw
-	cmd.Stderr = mw
-	cmd.ExtraFiles = []*os.File{wp}
+	cmd := osbuild.NewOSBuildCmd(manifest, exports, opts)
 
 	osbuildStatus := osbuild.NewStatusScanner(rp)
 	if err := cmd.Start(); err != nil {
@@ -186,10 +118,14 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, exports []string, o
 		// external build log
 		if st.Message != "" {
 			tracesMsgs = append(tracesMsgs, st.Message)
+			mu.Lock()
+			defer mu.Unlock()
 			fmt.Fprintln(buildLog, st.Message)
 		}
 		if st.Trace != "" {
 			tracesMsgs = append(tracesMsgs, st.Trace)
+			mu.Lock()
+			defer mu.Unlock()
 			fmt.Fprintln(buildLog, st.Trace)
 		}
 	}

pkg/progress/command_test.go

@@ -1,5 +1,4 @@
 package progress_test
-
 import (
 	"bytes"
 	"fmt"
@@ -12,6 +11,7 @@ import (
 	"github.com/stretchr/testify/assert"
 
 	"github.com/osbuild/image-builder-cli/pkg/progress"
+	"github.com/osbuild/images/pkg/osbuild"
 )
 
 func makeFakeOsbuild(t *testing.T, content string) string {
@@ -22,10 +22,7 @@ func makeFakeOsbuild(t *testing.T, content string) string {
 }
 
 func TestRunOSBuildWithProgressErrorReporting(t *testing.T) {
-	restore := progress.MockOsStderr(io.Discard)
-	defer restore()
-
-	restore = progress.MockOsbuildCmd(makeFakeOsbuild(t, `
+	restore := progress.MockOsbuildCmd(makeFakeOsbuild(t, `
 >&3 echo '{"message": "osbuild-stage-message"}'
 
 echo osbuild-stdout-output
@@ -34,9 +31,13 @@ exit 112
 `))
 	defer restore()
 
+	opts := &osbuild.OSBuildOptions{
+		Stderr: io.Discard,
+	}
+
 	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, opts)
 	assert.EqualError(t, err, `error running osbuild: exit status 112
 BuildLog:
 osbuild-stage-message
@@ -93,20 +94,14 @@ sleep 0.1
 `))
 	defer restore()
 
-	var fakeStdout, fakeStderr bytes.Buffer
-	restore = progress.MockOsStdout(&fakeStdout)
-	defer restore()
-	restore = progress.MockOsStderr(&fakeStderr)
-	defer restore()
-
 	pbar, err := progress.New("term")
 	assert.NoError(t, err)
 
-	var buildLog bytes.Buffer
-	opts := &progress.OSBuildOptions{
+	var buildLog, stdout bytes.Buffer
+	err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), nil, &osbuild.OSBuildOptions{
+		Stdout:  &stdout,
 		BuildLog: &buildLog,
-	}
-	err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), nil, opts)
+	})
 	assert.NoError(t, err)
 	expectedOutput := `osbuild-stdout-output
 osbuild-stderr-output
@@ -122,20 +117,14 @@ echo osbuild-stdout-output
 `))
 	defer restore()
 
-	var fakeStdout, fakeStderr bytes.Buffer
-	restore = progress.MockOsStdout(&fakeStdout)
-	defer restore()
-	restore = progress.MockOsStderr(&fakeStderr)
-	defer restore()
-
 	pbar, err := progress.New("verbose")
 	assert.NoError(t, err)
 
-	var buildLog bytes.Buffer
-	opts := &progress.OSBuildOptions{
+	var buildLog, stdout bytes.Buffer
+	err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), nil, &osbuild.OSBuildOptions{
+		Stdout:   &stdout,
 		BuildLog: &buildLog,
-	}
-	err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), nil, opts)
+	})
 	assert.NoError(t, err)
 	expectedOutput := `osbuild-stdout-output
 osbuild-stderr-output
@@ -151,7 +140,7 @@ func TestRunOSBuildCacheMaxSize(t *testing.T) {
 	pbar, err := progress.New("debug")
 	assert.NoError(t, err)
 
-	osbuildOpts := &progress.OSBuildOptions{
+	osbuildOpts := &osbuild.OSBuildOptions{
 		CacheMaxSize: 77,
 	}
 	err = progress.RunOSBuild(pbar, []byte(`{"fake":"manifest"}`), nil, osbuildOpts)

pkg/progress/export_test.go

@@ -2,6 +2,8 @@ package progress
 
 import (
 	"io"
+
+	"github.com/osbuild/images/pkg/osbuild"
 )
 
 type (
@@ -11,17 +13,9 @@ type (
 )
 
 var (
-	NewSyncedWriter = newSyncedWriter
+	OSStderr        = osStderr
 )
 
-func MockOsStdout(w io.Writer) (restore func()) {
-	saved := osStdout
-	osStdout = func() io.Writer { return w }
-	return func() {
-		osStdout = saved
-	}
-}
-
 func MockOsStderr(w io.Writer) (restore func()) {
 	saved := osStderr
 	osStderr = func() io.Writer { return w }
@@ -39,9 +33,9 @@ func MockIsattyIsTerminal(fn func(uintptr) bool) (restore func()) {
 }
 
 func MockOsbuildCmd(s string) (restore func()) {
-	saved := osbuildCmd
-	osbuildCmd = s
+	saved := osbuild.OSBuildCmd
+	osbuild.OSBuildCmd = s
 	return func() {
-		osbuildCmd = saved
+		osbuild.OSBuildCmd = saved
 	}
 }

pkg/progress/progress.go

@@ -27,9 +27,6 @@ var (
 // Used for testing, this must be a function (instead of the usual
 // "var osStderr = os.Stderr" so that higher level libraries can test
 // this code by replacing "os.Stderr", e.g. testutil.CaptureStdio()
-var osStdout = func() io.Writer {
-	return os.Stdout
-}
 var osStderr = func() io.Writer {
 	return os.Stderr
 }