Only reset the cache exporter configuration attribute if the inline

cache was configured.

Signed-off-by: a-palchikov <[email protected]>

client/client_test.go

@@ -143,7 +143,6 @@ func TestIntegration(t *testing.T) {
 		testTarExporterSymlink,
 		testMultipleRegistryCacheImportExport,
 		testMultipleExporters,
-		testPreventsMultipleExportWithSameExporter,
 		testSourceMap,
 		testSourceMapFromRef,
 		testLazyImagePush,
@@ -2185,32 +2184,6 @@ func testUser(t *testing.T, sb integration.Sandbox) {
 	checkAllReleasable(t, c, sb, true)
 }
 
-func testPreventsMultipleExportWithSameExporter(t *testing.T, sb integration.Sandbox) {
-	requiresLinux(t)
-
-	c, err := New(sb.Context(), sb.Address())
-	require.NoError(t, err)
-	defer c.Close()
-
-	def, err := llb.Scratch().File(llb.Mkfile("foo.txt", 0o755, nil)).Marshal(sb.Context())
-	require.NoError(t, err)
-
-	dummyOutput := func(map[string]string) (io.WriteCloser, error) { return nil, fmt.Errorf("not implemented") }
-	_, err = c.Solve(sb.Context(), def, SolveOpt{
-		Exports: []ExportEntry{
-			{
-				Type:   ExporterOCI,
-				Output: dummyOutput,
-			},
-			{
-				Type:   ExporterOCI,
-				Output: dummyOutput,
-			},
-		},
-	}, nil)
-	require.Errorf(t, err, "using multiple ExporterOCI is not supported")
-}
-
 func testMultipleExporters(t *testing.T, sb integration.Sandbox) {
 	requiresLinux(t)
 
@@ -2221,12 +2194,17 @@ func testMultipleExporters(t *testing.T, sb integration.Sandbox) {
 	def, err := llb.Scratch().File(llb.Mkfile("foo.txt", 0o755, nil)).Marshal(context.TODO())
 	require.NoError(t, err)
 
-	destDir := t.TempDir()
+	destDir, destDir2 := t.TempDir(), t.TempDir()
 	out := filepath.Join(destDir, "out.tar")
 	outW, err := os.Create(out)
 	require.NoError(t, err)
 	defer outW.Close()
 
+	out2 := filepath.Join(destDir, "out2.tar")
+	outW2, err := os.Create(out2)
+	require.NoError(t, err)
+	defer outW2.Close()
+
 	registry, err := sb.NewRegistry()
 	if errors.Is(err, integration.ErrRequirements) {
 		t.Skip(err.Error())
@@ -2243,15 +2221,25 @@ func testMultipleExporters(t *testing.T, sb integration.Sandbox) {
 
 	resp, err := c.Solve(sb.Context(), def, SolveOpt{
 		Exports: []ExportEntry{
+			// Ensure that multiple local exporter destinations are written properly
 			{
 				Type:      ExporterLocal,
 				OutputDir: destDir,
 			},
-			// TODO(dima): this won't work just yet
-			//{
-			//	Type:   ExporterTar,
-			//	Output: fixedWriteCloser(outW),
-			//},
+			{
+				Type:      ExporterLocal,
+				OutputDir: destDir2,
+			},
+			// Ensure that multiple instances of the same exporter are possible
+			{
+				Type:   ExporterTar,
+				Output: fixedWriteCloser(outW),
+			},
+			{
+				Type:   ExporterTar,
+				Output: fixedWriteCloser(outW2),
+			},
+
 			{
 				Type: imageExporter,
 				Attrs: map[string]string{
@@ -2262,8 +2250,10 @@ func testMultipleExporters(t *testing.T, sb integration.Sandbox) {
 	}, nil)
 	require.NoError(t, err)
 	require.Equal(t, resp.ExporterResponse["image.name"], target1+","+target2)
-	// require.FileExists(t, filepath.Join(destDir, "out.tar"))
+	require.FileExists(t, filepath.Join(destDir, "out.tar"))
+	require.FileExists(t, filepath.Join(destDir, "out2.tar"))
 	require.FileExists(t, filepath.Join(destDir, "foo.txt"))
+	require.FileExists(t, filepath.Join(destDir2, "foo.txt"))
 }
 
 func testOCIExporter(t *testing.T, sb integration.Sandbox) {

client/solve.go

@@ -80,7 +80,7 @@ func (c *Client) Solve(ctx context.Context, def *llb.Definition, opt SolveOpt, s
 
 type runGatewayCB func(ref string, s *session.Session, opts map[string]string) error
 
-func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runGatewayCB, opt SolveOpt, statusChan chan *SolveStatus) (result *SolveResponse, err error) {
+func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runGatewayCB, opt SolveOpt, statusChan chan *SolveStatus) (*SolveResponse, error) {
 	if def != nil && runGateway != nil {
 		return nil, errors.New("invalid with def and cb")
 	}
@@ -211,7 +211,6 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
 			s.Allow(filesync.NewFSSyncTarget(exporterConfig.outputs))
 		}
 
-
 		eg.Go(func() error {
 			sd := c.sessionDialer
 			if sd == nil {
@@ -229,6 +228,7 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
 	}
 
 	solveCtx, cancelSolve := context.WithCancel(ctx)
+	var res *SolveResponse
 	eg.Go(func() error {
 		ctx := solveCtx
 		defer cancelSolve()
@@ -272,7 +272,7 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
 		}
 		if localExporter {
 			// Add a single instance of the local exporter
-			// since it's replicated  entirely in the client
+			// since it's replicated entirely on the client
 			exports = append(exports, &controlapi.Exporter{
 				Name: ExporterLocal,
 			})
@@ -292,8 +292,7 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
 		if err != nil {
 			return errors.Wrap(err, "failed to solve")
 		}
-
-		result = &SolveResponse{
+		res = &SolveResponse{
 			ExporterResponse: resp.ExporterResponse,
 		}
 		return nil
@@ -391,7 +390,7 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
 	}
 	// Update index.json of exported cache content store
 	// FIXME(AkihiroSuda): dedupe const definition of cache/remotecache.ExporterResponseManifestDesc = "cache.manifest"
-	if manifestDescJSON := result.ExporterResponse["cache.manifest"]; manifestDescJSON != "" {
+	if manifestDescJSON := res.ExporterResponse["cache.manifest"]; manifestDescJSON != "" {
 		var manifestDesc ocispecs.Descriptor
 		if err = json.Unmarshal([]byte(manifestDescJSON), &manifestDesc); err != nil {
 			return nil, err
@@ -402,7 +401,7 @@ func (c *Client) solve(ctx context.Context, def *llb.Definition, runGateway runG
 			}
 		}
 	}
-	return result, nil
+	return res, nil
 }
 
 func prepareSyncedDirs(def *llb.Definition, localDirs map[string]string) ([]filesync.SyncedDir, error) {
@@ -476,7 +475,7 @@ func parseCacheOptions(ctx context.Context, isGateway bool, opt SolveOpt) (*cach
 			if csDir == "" {
 				return nil, errors.New("local cache exporter requires dest")
 			}
-			if err := os.MkdirAll(csDir, 0o755); err != nil {
+			if err := os.MkdirAll(csDir, 0755); err != nil {
 				return nil, err
 			}
 			cs, err := contentlocal.NewStore(csDir)

cmd/buildctl/build/exportcache.go

@@ -22,7 +22,7 @@ func parseExportCacheCSV(s string) (client.CacheOptionsEntry, error) {
 	for _, field := range fields {
 		parts := strings.SplitN(field, "=", 2)
 		if len(parts) != 2 {
-			return ex, errors.Errorf("invalid value %q", field)
+			return ex, errors.Errorf("invalid value %s", field)
 		}
 		key := strings.ToLower(parts[0])
 		value := parts[1]
@@ -63,7 +63,7 @@ func ParseExportCache(exportCaches []string) ([]client.CacheOptionsEntry, error)
 		} else {
 			ex, err := parseExportCacheCSV(exportCache)
 			if err != nil {
-				return nil, errors.Wrap(err, "parsing export-cache csv")
+				return nil, err
 			}
 			exports = append(exports, ex)
 		}

cmd/buildctl/build/importcache.go

@@ -22,7 +22,7 @@ func parseImportCacheCSV(s string) (client.CacheOptionsEntry, error) {
 	for _, field := range fields {
 		parts := strings.SplitN(field, "=", 2)
 		if len(parts) != 2 {
-			return im, errors.Errorf("invalid value %q", field)
+			return im, errors.Errorf("invalid value %s", field)
 		}
 		key := strings.ToLower(parts[0])
 		value := parts[1]
@@ -57,7 +57,7 @@ func ParseImportCache(importCaches []string) ([]client.CacheOptionsEntry, error)
 		} else {
 			im, err := parseImportCacheCSV(importCache)
 			if err != nil {
-				return nil, errors.Wrap(err, "parsing import-cache csv")
+				return nil, err
 			}
 			imports = append(imports, im)
 		}

cmd/buildctl/build/output.go

@@ -55,12 +55,12 @@ func parseOutputCSV(s string) (client.ExportEntry, error) {
 // ParseOutput parses --output
 func ParseOutput(exports []string) ([]client.ExportEntry, error) {
 	var entries []client.ExportEntry
-	for _, exp := range exports {
-		opts, err := parseOutputCSV(exp)
+	for _, s := range exports {
+		e, err := parseOutputCSV(s)
 		if err != nil {
 			return nil, err
 		}
-		entries = append(entries, opts)
+		entries = append(entries, e)
 	}
 	return entries, nil
 }

control/control.go

@@ -336,7 +336,6 @@ func (c *Controller) Solve(ctx context.Context, req *controlapi.SolveRequest) (*
 	if err != nil {
 		return nil, err
 	}
-
 	return &controlapi.SolveResponse{
 		ExporterResponse: resp.ExporterResponse,
 	}, nil

exporter/oci/export.go

@@ -136,8 +136,6 @@ func (e *imageExporterInstance) ExportImage(ctx context.Context, src exporter.So
 		return nil, err
 	}
 	defer func() {
-		// TODO(dima): is it safe to remove the descriptor while branching
-		// off into streamer?
 		e.opt.ImageWriter.ContentStore().Delete(context.TODO(), desc.Digest)
 	}()
 
@@ -235,7 +233,7 @@ func (e *imageExporterInstance) ExportImage(ctx context.Context, src exporter.So
 	if err := archiveexporter.Export(ctx, mprovider, w, expOpts...); err != nil {
 		w.Close()
 		if grpcerrors.Code(err) == codes.AlreadyExists {
-			return nil, report(nil)
+			return resp, report(nil)
 		}
 		return nil, report(err)
 	}
@@ -251,7 +249,7 @@ func normalizedNames(name string) ([]string, error) {
 		return nil, nil
 	}
 	names := strings.Split(name, ",")
-	tagNames := make([]string, len(names))
+	var tagNames = make([]string, len(names))
 	for i, name := range names {
 		parsed, err := reference.ParseNormalizedNamed(name)
 		if err != nil {

session/filesync/diffcopy.go

@@ -156,7 +156,6 @@ func writeTargetFile(ds grpc.ServerStream, fs map[string]FileOutputFunc, opts me
 			return errors.WithStack(err)
 		}
 		if wc == nil {
-			// TODO(dima): factor this out
 			md := map[string]string{}
 			for k, v := range opts {
 				if strings.HasPrefix(k, keyExporterMetaPrefix) {

solver/llbsolver/solver.go

@@ -204,7 +204,7 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
 		inp, err := result.ConvertResult(cached, func(res solver.CachedResult) (cache.ImmutableRef, error) {
 			workerRef, ok := res.Sys().(*worker.WorkerRef)
 			if !ok {
-				return nil, errors.Errorf("invalid reference: %T", res.Sys())
+				return nil, errors.Errorf("invalid reference: %T", r.Sys())
 			}
 			return workerRef.ImmutableRef, nil
 		})
@@ -213,7 +213,9 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
 		}
 
 		cacheExporter := exp.CacheExporter
-		exp.CacheExporter = nil
+		if _, ok := asInlineCache(cacheExporter); ok {
+			exp.CacheExporter = nil
+		}
 		exporterResponse, err = s.runExporters(ctx, exp.Exporters, cacheExporter, cached, inp, j)
 		if err != nil {
 			return nil, err
@@ -344,7 +346,7 @@ func (s *Solver) runExporters(ctx context.Context, exporters []exporter.Exporter
 
 func exportInlineCache(ctx context.Context, compression compression.Config, e remotecache.Exporter, cached *result.Result[solver.CachedResult], job *solver.Job, sessionID string) (*exptypes.InlineCache, error) {
 	result := &exptypes.InlineCache{}
-	if err := inBuilderContext(ctx, job, "preparing layers for inline cache", j.SessionID+"-cache-inline", func(ctx context.Context, _ session.Group) error {
+	if err := inBuilderContext(ctx, job, "preparing layers for inline cache", job.SessionID+"-cache-inline", func(ctx context.Context, _ session.Group) error {
 		if cached.Ref != nil {
 			dtic, err := inlineCache(ctx, e, cached.Ref, compression, session.NewGroup(sessionID))
 			if err != nil {
@@ -451,7 +453,6 @@ func defaultResolver(wc *worker.Controller) ResolveWorkerFunc {
 		return wc.GetDefault()
 	}
 }
-
 func allWorkers(wc *worker.Controller) func(func(w worker.Worker) error) error {
 	return func(f func(worker.Worker) error) error {
 		all, err := wc.List()