Merge pull request #4583 from leandrosansilva/refactoring/do_not_use_…

…deprecated_local_dirs_in_tests Refactoring: Remove usage of deprecated LocalDirs field in SolveOpts
moby · Feb 7, 2024 · 7c0d261 · 7c0d261
2 parents db3aaa3 + 704268a
commit 7c0d261
Show file tree

Hide file tree

Showing 22 changed files with 351 additions and 249 deletions.
diff --git a/client/build_test.go b/client/build_test.go
@@ -31,6 +31,7 @@ import (
 	digest "github.com/opencontainers/go-digest"
 	"github.com/pkg/errors"
 	"github.com/stretchr/testify/require"
+	"github.com/tonistiigi/fsutil"
 	"golang.org/x/crypto/ssh/agent"
 	"google.golang.org/grpc/codes"
 )
@@ -691,9 +692,9 @@ func testClientGatewayContainerMounts(t *testing.T, sb integration.Sandbox) {
 	require.NoError(t, err)
 	defer c.Close()
 
-	tmpdir := t.TempDir()
+	tmpdir := integration.Tmpdir(t)
 
-	err = os.WriteFile(filepath.Join(tmpdir, "local-file"), []byte("local"), 0644)
+	err = os.WriteFile(filepath.Join(tmpdir.Name, "local-file"), []byte("local"), 0644)
 	require.NoError(t, err)
 
 	a := agent.NewKeyring()
@@ -834,7 +835,7 @@ func testClientGatewayContainerMounts(t *testing.T, sb integration.Sandbox) {
 	}
 
 	_, err = c.Build(ctx, SolveOpt{
-		LocalDirs: map[string]string{
+		LocalMounts: map[string]fsutil.FS{
 			"mylocal": tmpdir,
 		},
 		Session: []session.Attachable{

diff --git a/client/client_test.go b/client/client_test.go
@@ -69,6 +69,7 @@ import (
 	"github.com/pkg/errors"
 	"github.com/spdx/tools-golang/spdx"
 	"github.com/stretchr/testify/require"
+	"github.com/tonistiigi/fsutil"
 	"golang.org/x/crypto/ssh/agent"
 	"golang.org/x/sync/errgroup"
 	"google.golang.org/grpc"
@@ -210,6 +211,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){
 	testSnapshotWithMultipleBlobs,
 	testExportLocalNoPlatformSplit,
 	testExportLocalNoPlatformSplitOverwrite,
+	testSolverOptLocalDirsStillWorks,
 }
 
 func TestIntegration(t *testing.T) {
@@ -272,9 +274,9 @@ func testCacheExportCacheKeyLoop(t *testing.T, sb integration.Sandbox) {
 	require.NoError(t, err)
 	defer c.Close()
 
-	tmpdir := t.TempDir()
+	tmpdir := integration.Tmpdir(t)
 
-	err = os.WriteFile(filepath.Join(tmpdir, "foo"), []byte("foodata"), 0600)
+	err = os.WriteFile(filepath.Join(tmpdir.Name, "foo"), []byte("foodata"), 0600)
 	require.NoError(t, err)
 
 	for _, mode := range []bool{false, true} {
@@ -295,11 +297,11 @@ func testCacheExportCacheKeyLoop(t *testing.T, sb integration.Sandbox) {
 						{
 							Type: "local",
 							Attrs: map[string]string{
-								"dest": filepath.Join(tmpdir, "cache"),
+								"dest": filepath.Join(tmpdir.Name, "cache"),
 							},
 						},
 					},
-					LocalDirs: map[string]string{
+					LocalMounts: map[string]fsutil.FS{
 						"mylocal": tmpdir,
 					},
 				}, nil)
@@ -1515,7 +1517,7 @@ func testLocalSymlinkEscape(t *testing.T, sb integration.Sandbox) {
 	require.NoError(t, err)
 
 	_, err = c.Solve(sb.Context(), def, SolveOpt{
-		LocalDirs: map[string]string{
+		LocalMounts: map[string]fsutil.FS{
 			"mylocal": dir,
 		},
 	}, nil)
@@ -1554,6 +1556,49 @@ func testRelativeWorkDir(t *testing.T, sb integration.Sandbox) {
 	require.Equal(t, []byte("/test1/test2\n"), dt)
 }
 
+// TODO: remove this test once `client.SolveOpt.LocalDirs`, now marked as deprecated, is removed.
+// For more context on this test, please check:
+// https://github.com/moby/buildkit/pull/4583#pullrequestreview-1847043452
+func testSolverOptLocalDirsStillWorks(t *testing.T, sb integration.Sandbox) {
+	integration.SkipOnPlatform(t, "windows")
+
+	c, err := New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	out := llb.Image("docker.io/library/busybox:latest").
+		File(llb.Copy(llb.Local("mylocal"), "input.txt", "input.txt")).
+		Run(llb.Shlex(`sh -c "/bin/rev < input.txt > /out/output.txt"`)).
+		AddMount(`/out`, llb.Scratch())
+
+	def, err := out.Marshal(sb.Context())
+	require.NoError(t, err)
+
+	srcDir := integration.Tmpdir(t,
+		fstest.CreateFile("input.txt", []byte("Hello World"), 0600),
+	)
+
+	destDir := integration.Tmpdir(t)
+
+	_, err = c.Solve(sb.Context(), def, SolveOpt{
+		LocalDirs: map[string]string{
+			"mylocal": srcDir.Name,
+		},
+		Exports: []ExportEntry{
+			{
+				Type:      ExporterLocal,
+				OutputDir: destDir.Name,
+			},
+		},
+	}, nil)
+
+	require.NoError(t, err)
+
+	dt, err := os.ReadFile(filepath.Join(destDir.Name, "output.txt"))
+	require.NoError(t, err)
+	require.Equal(t, []byte("dlroW olleH"), dt)
+}
+
 func testFileOpMkdirMkfile(t *testing.T, sb integration.Sandbox) {
 	requiresLinux(t)
 	c, err := New(sb.Context(), sb.Address())
@@ -1624,7 +1669,7 @@ func testFileOpCopyRm(t *testing.T, sb integration.Sandbox) {
 				OutputDir: destDir,
 			},
 		},
-		LocalDirs: map[string]string{
+		LocalMounts: map[string]fsutil.FS{
 			"mylocal":  dir,
 			"mylocal2": dir2,
 		},
@@ -1751,7 +1796,7 @@ func testFileOpCopyIncludeExclude(t *testing.T, sb integration.Sandbox) {
 				OutputDir: destDir,
 			},
 		},
-		LocalDirs: map[string]string{
+		LocalMounts: map[string]fsutil.FS{
 			"mylocal": dir,
 		},
 	}, nil)
@@ -1772,7 +1817,7 @@ func testFileOpCopyIncludeExclude(t *testing.T, sb integration.Sandbox) {
 	// Create additional file which doesn't match the include pattern, and make
 	// sure this doesn't invalidate the cache.
 
-	err = fstest.Apply(fstest.CreateFile("unmatchedfile", []byte("data1"), 0600)).Apply(dir)
+	err = fstest.Apply(fstest.CreateFile("unmatchedfile", []byte("data1"), 0600)).Apply(dir.Name)
 	require.NoError(t, err)
 
 	st = llb.Scratch().File(
@@ -1798,7 +1843,7 @@ func testFileOpCopyIncludeExclude(t *testing.T, sb integration.Sandbox) {
 				OutputDir: destDir,
 			},
 		},
-		LocalDirs: map[string]string{
+		LocalMounts: map[string]fsutil.FS{
 			"mylocal": dir,
 		},
 	}, nil)
@@ -1861,7 +1906,7 @@ func testLocalSourceWithDiffer(t *testing.T, sb integration.Sandbox, d llb.DiffT
 
 	tv := syscall.NsecToTimespec(time.Now().UnixNano())
 
-	err = syscall.UtimesNano(filepath.Join(dir, "foo"), []syscall.Timespec{tv, tv})
+	err = syscall.UtimesNano(filepath.Join(dir.Name, "foo"), []syscall.Timespec{tv, tv})
 	require.NoError(t, err)
 
 	st := llb.Local("mylocal"+string(d), llb.Differ(d, false))
@@ -1878,7 +1923,7 @@ func testLocalSourceWithDiffer(t *testing.T, sb integration.Sandbox, d llb.DiffT
 				OutputDir: destDir,
 			},
 		},
-		LocalDirs: map[string]string{
+		LocalMounts: map[string]fsutil.FS{
 			"mylocal" + string(d): dir,
 		},
 	}, nil)
@@ -1888,10 +1933,10 @@ func testLocalSourceWithDiffer(t *testing.T, sb integration.Sandbox, d llb.DiffT
 	require.NoError(t, err)
 	require.Equal(t, []byte("foo"), dt)
 
-	err = os.WriteFile(filepath.Join(dir, "foo"), []byte("bar"), 0600)
+	err = os.WriteFile(filepath.Join(dir.Name, "foo"), []byte("bar"), 0600)
 	require.NoError(t, err)
 
-	err = syscall.UtimesNano(filepath.Join(dir, "foo"), []syscall.Timespec{tv, tv})
+	err = syscall.UtimesNano(filepath.Join(dir.Name, "foo"), []syscall.Timespec{tv, tv})
 	require.NoError(t, err)
 
 	_, err = c.Solve(context.TODO(), def, SolveOpt{
@@ -1901,7 +1946,7 @@ func testLocalSourceWithDiffer(t *testing.T, sb integration.Sandbox, d llb.DiffT
 				OutputDir: destDir,
 			},
 		},
-		LocalDirs: map[string]string{
+		LocalMounts: map[string]fsutil.FS{
 			"mylocal" + string(d): dir,
 		},
 	}, nil)
@@ -2211,7 +2256,7 @@ func testFileOpRmWildcard(t *testing.T, sb integration.Sandbox) {
 				OutputDir: destDir,
 			},
 		},
-		LocalDirs: map[string]string{
+		LocalMounts: map[string]fsutil.FS{
 			"mylocal": dir,
 		},
 	}, nil)
@@ -7658,7 +7703,7 @@ func testParallelLocalBuilds(t *testing.T, sb integration.Sandbox) {
 							OutputDir: destDir,
 						},
 					},
-					LocalDirs: map[string]string{
+					LocalMounts: map[string]fsutil.FS{
 						"source": srcDir,
 					},
 				}, nil)
@@ -9713,7 +9758,7 @@ func ensureFileContents(t *testing.T, path, expectedContents string) {
 
 func makeSSHAgentSock(t *testing.T, agent agent.Agent) (p string, err error) {
 	tmpDir := integration.Tmpdir(t)
-	sockPath := filepath.Join(tmpDir, "ssh_auth_sock")
+	sockPath := filepath.Join(tmpDir.Name, "ssh_auth_sock")
 
 	l, err := net.Listen("unix", sockPath)
 	if err != nil {

diff --git a/cmd/buildctl/build.go b/cmd/buildctl/build.go
@@ -250,7 +250,7 @@ func buildAction(clicontext *cli.Context) error {
 
 	solveOpt := client.SolveOpt{
 		Exports: exports,
-		// LocalDirs is set later
+		// LocalMounts is set later
 		Frontend: clicontext.String("frontend"),
 		// FrontendAttrs is set later
 		// OCILayouts is set later
@@ -267,7 +267,7 @@ func buildAction(clicontext *cli.Context) error {
 		return errors.Wrap(err, "invalid opt")
 	}
 
-	solveOpt.LocalDirs, err = build.ParseLocal(clicontext.StringSlice("local"))
+	solveOpt.LocalMounts, err = build.ParseLocal(clicontext.StringSlice("local"))
 	if err != nil {
 		return errors.Wrap(err, "invalid local")
 	}

diff --git a/cmd/buildctl/build/local.go b/cmd/buildctl/build/local.go
@@ -1,6 +1,25 @@
 package build
 
+import (
+	"github.com/pkg/errors"
+	"github.com/tonistiigi/fsutil"
+)
+
 // ParseLocal parses --local
-func ParseLocal(locals []string) (map[string]string, error) {
-	return attrMap(locals)
+func ParseLocal(locals []string) (map[string]fsutil.FS, error) {
+	localDirs, err := attrMap(locals)
+	if err != nil {
+		return nil, errors.WithStack(err)
+	}
+
+	mounts := make(map[string]fsutil.FS, len(localDirs))
+
+	for k, v := range localDirs {
+		mounts[k], err = fsutil.NewFS(v)
+		if err != nil {
+			return nil, errors.WithStack(err)
+		}
+	}
+
+	return mounts, nil
 }
diff --git a/examples/build-using-dockerfile/main.go b/examples/build-using-dockerfile/main.go
@@ -16,6 +16,7 @@ import (
 	"github.com/moby/buildkit/util/progress/progressui"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+	"github.com/tonistiigi/fsutil"
 	"github.com/urfave/cli"
 	"golang.org/x/sync/errgroup"
 )
@@ -136,9 +137,15 @@ func newSolveOpt(clicontext *cli.Context, w io.WriteCloser) (*client.SolveOpt, e
 	if file == "" {
 		file = filepath.Join(buildCtx, "Dockerfile")
 	}
-	localDirs := map[string]string{
-		"context":    buildCtx,
-		"dockerfile": filepath.Dir(file),
+
+	cxtLocalMount, err := fsutil.NewFS(buildCtx)
+	if err != nil {
+		return nil, errors.New("invalid buildCtx local mount dir")
+	}
+
+	dockerfileLocalMount, err := fsutil.NewFS(filepath.Dir(file))
+	if err != nil {
+		return nil, errors.New("invalid dockerfile local mount dir")
 	}
 
 	frontend := "dockerfile.v0" // TODO: use gateway
@@ -173,7 +180,10 @@ func newSolveOpt(clicontext *cli.Context, w io.WriteCloser) (*client.SolveOpt, e
 				},
 			},
 		},
-		LocalDirs:     localDirs,
+		LocalMounts: map[string]fsutil.FS{
+			"context":    cxtLocalMount,
+			"dockerfile": dockerfileLocalMount,
+		},
 		Frontend:      frontend,
 		FrontendAttrs: frontendAttrs,
 	}, nil

diff --git a/frontend/dockerfile/dockerfile_addchecksum_test.go b/frontend/dockerfile/dockerfile_addchecksum_test.go
@@ -12,6 +12,7 @@ import (
 	"github.com/moby/buildkit/util/testutil/integration"
 	digest "github.com/opencontainers/go-digest"
 	"github.com/stretchr/testify/require"
+	"github.com/tonistiigi/fsutil"
 )
 
 var addChecksumTests = integration.TestFuncs(
@@ -50,7 +51,7 @@ ADD --checksum=%s %s /tmp/foo
 			fstest.CreateFile("Dockerfile", dockerfile, 0600),
 		)
 		_, err := f.Solve(sb.Context(), c, client.SolveOpt{
-			LocalDirs: map[string]string{
+			LocalMounts: map[string]fsutil.FS{
 				dockerui.DefaultLocalNameDockerfile: dir,
 				dockerui.DefaultLocalNameContext:    dir,
 			},
@@ -69,7 +70,7 @@ ADD --checksum=${DIGEST} ${LINK} /tmp/foo
 			fstest.CreateFile("Dockerfile", dockerfile, 0600),
 		)
 		_, err := f.Solve(sb.Context(), c, client.SolveOpt{
-			LocalDirs: map[string]string{
+			LocalMounts: map[string]fsutil.FS{
 				dockerui.DefaultLocalNameDockerfile: dir,
 				dockerui.DefaultLocalNameContext:    dir,
 			},
@@ -86,7 +87,7 @@ ADD --checksum=%s %s /tmp/foo
 			fstest.CreateFile("Dockerfile", dockerfile, 0600),
 		)
 		_, err := f.Solve(sb.Context(), c, client.SolveOpt{
-			LocalDirs: map[string]string{
+			LocalMounts: map[string]fsutil.FS{
 				dockerui.DefaultLocalNameDockerfile: dir,
 				dockerui.DefaultLocalNameContext:    dir,
 			},
@@ -103,7 +104,7 @@ ADD --checksum=md5:7e55db001d319a94b0b713529a756623 %s /tmp/foo
 			fstest.CreateFile("Dockerfile", dockerfile, 0600),
 		)
 		_, err := f.Solve(sb.Context(), c, client.SolveOpt{
-			LocalDirs: map[string]string{
+			LocalMounts: map[string]fsutil.FS{
 				dockerui.DefaultLocalNameDockerfile: dir,
 				dockerui.DefaultLocalNameContext:    dir,
 			},
@@ -120,7 +121,7 @@ ADD --checksum=unknown:%s %s /tmp/foo
 			fstest.CreateFile("Dockerfile", dockerfile, 0600),
 		)
 		_, err := f.Solve(sb.Context(), c, client.SolveOpt{
-			LocalDirs: map[string]string{
+			LocalMounts: map[string]fsutil.FS{
 				dockerui.DefaultLocalNameDockerfile: dir,
 				dockerui.DefaultLocalNameContext:    dir,
 			},
@@ -137,7 +138,7 @@ ADD --checksum=%s %s /tmp/foo
 			fstest.CreateFile("Dockerfile", dockerfile, 0600),
 		)
 		_, err := f.Solve(sb.Context(), c, client.SolveOpt{
-			LocalDirs: map[string]string{
+			LocalMounts: map[string]fsutil.FS{
 				dockerui.DefaultLocalNameDockerfile: dir,
 				dockerui.DefaultLocalNameContext:    dir,
 			},
@@ -156,7 +157,7 @@ ADD --checksum=%s foo /tmp/foo
 			fstest.CreateFile("Dockerfile", dockerfile, 0600),
 		)
 		_, err := f.Solve(sb.Context(), c, client.SolveOpt{
-			LocalDirs: map[string]string{
+			LocalMounts: map[string]fsutil.FS{
 				dockerui.DefaultLocalNameDockerfile: dir,
 				dockerui.DefaultLocalNameContext:    dir,
 			},