diff --git a/cmd/image-builder/build.go b/cmd/image-builder/build.go index 9358edad..f66e870a 100644 --- a/cmd/image-builder/build.go +++ b/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, } diff --git a/go.mod b/go.mod index 407eea37..1c998c41 100644 --- a/go.mod +++ b/go.mod @@ -12,7 +12,7 @@ require ( github.com/gobwas/glob v0.2.3 github.com/mattn/go-isatty v0.0.20 github.com/osbuild/blueprint v1.16.0 - github.com/osbuild/images v0.206.0 + github.com/osbuild/images v0.209.0 github.com/spf13/cobra v1.9.1 github.com/stretchr/testify v1.11.1 golang.org/x/sys v0.35.0 @@ -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 diff --git a/go.sum b/go.sum index cd534f90..0527f461 100644 --- a/go.sum +++ b/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,8 +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/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= diff --git a/pkg/progress/command.go b/pkg/progress/command.go index d6340a97..8bacbbed 100644 --- a/pkg/progress/command.go +++ b/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,67 +35,15 @@ 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) @@ -116,24 +51,23 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, exports []string, o 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 + } else { + opts.Stdout = &stdio } - - 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,11 +120,15 @@ func runOSBuildWithProgress(pb ProgressBar, manifest []byte, exports []string, o // external build log if st.Message != "" { tracesMsgs = append(tracesMsgs, st.Message) + mu.Lock() fmt.Fprintln(buildLog, st.Message) + mu.Unlock() } if st.Trace != "" { tracesMsgs = append(tracesMsgs, st.Trace) + mu.Lock() fmt.Fprintln(buildLog, st.Trace) + mu.Unlock() } } diff --git a/pkg/progress/command_test.go b/pkg/progress/command_test.go index 9d4cf4fb..bf9cdf1b 100644 --- a/pkg/progress/command_test.go +++ b/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) diff --git a/pkg/progress/export_test.go b/pkg/progress/export_test.go index 26d0c57c..b66a1c87 100644 --- a/pkg/progress/export_test.go +++ b/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 } } diff --git a/pkg/progress/progress.go b/pkg/progress/progress.go index c96eaac3..66b4c97d 100644 --- a/pkg/progress/progress.go +++ b/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 } diff --git a/pkg/progress/syncwriter.go b/pkg/progress/syncwriter.go deleted file mode 100644 index f9ca783a..00000000 --- a/pkg/progress/syncwriter.go +++ /dev/null @@ -1,22 +0,0 @@ -package progress - -import ( - "io" - "sync" -) - -type syncedWriter struct { - mu *sync.Mutex - w io.Writer -} - -func newSyncedWriter(mu *sync.Mutex, w io.Writer) io.Writer { - return &syncedWriter{mu: mu, w: w} -} - -func (sw *syncedWriter) Write(p []byte) (n int, err error) { - sw.mu.Lock() - defer sw.mu.Unlock() - - return sw.w.Write(p) -} diff --git a/pkg/progress/syncwriter_test.go b/pkg/progress/syncwriter_test.go deleted file mode 100644 index bf2daf1c..00000000 --- a/pkg/progress/syncwriter_test.go +++ /dev/null @@ -1,44 +0,0 @@ -package progress_test - -import ( - "bufio" - "bytes" - "fmt" - "strings" - "sync" - "testing" - "time" - - "github.com/stretchr/testify/assert" - - "github.com/osbuild/image-builder-cli/pkg/progress" -) - -func TestSyncWriter(t *testing.T) { - var mu sync.Mutex - var buf bytes.Buffer - var wg sync.WaitGroup - - for id := 0; id < 100; id++ { - wg.Add(1) - w := progress.NewSyncedWriter(&mu, &buf) - go func(id int) { - defer wg.Done() - for i := 0; i < 500; i++ { - fmt.Fprintln(w, strings.Repeat(fmt.Sprintf("%v", id%10), 60)) - time.Sleep(10 * time.Nanosecond) - } - }(id) - } - wg.Wait() - - scanner := bufio.NewScanner(&buf) - for { - if !scanner.Scan() { - break - } - line := scanner.Text() - assert.True(t, len(line) == 60, fmt.Sprintf("len %v: line: %v", len(line), line)) - } - assert.NoError(t, scanner.Err()) -}