Merge pull request #3908 from gabriel-samfira/handle-platform-file-paths

Handle file paths base on target platform
2026-08-09 09:11:45 +00:00 · 2023-07-10 13:57:23 -07:00
parent b8294e93bb f1657ecc14
commit 595bfa218f
8 changed files with 123 additions and 77 deletions
--- a/client/llb/fileop.go
+++ b/client/llb/fileop.go
@@ -145,6 +145,7 @@ func Mkdir(p string, m os.FileMode, opt ...MkdirOption) *FileAction {
 	for _, o := range opt {
 		o.SetMkdirOption(&mi)
 	}
+
 	return &FileAction{
 		action: &fileActionMkdir{
 			file: p,
@@ -447,7 +448,6 @@ func Copy(input CopyInput, src, dest string, opts ...CopyOption) *FileAction {
 	for _, o := range opts {
 		o.SetCopyOption(&mi)
 	}
-
 	return &FileAction{
 		action: &fileActionCopy{
 			state: state,
@@ -523,22 +523,19 @@ func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base

 func (a *fileActionCopy) sourcePath(ctx context.Context) (string, error) {
 	p := path.Clean(a.src)
+	dir := "/"
+	var err error
 	if !path.IsAbs(p) {
 		if a.state != nil {
-			dir, err := a.state.GetDir(ctx)
-			if err != nil {
-				return "", err
-			}
-			p = path.Join("/", dir, p)
+			dir, err = a.state.GetDir(ctx)
 		} else if a.fas != nil {
-			dir, err := a.fas.state.GetDir(ctx)
-			if err != nil {
-				return "", err
-			}
-			p = path.Join("/", dir, p)
+			dir, err = a.fas.state.GetDir(ctx)
+		}
+		if err != nil {
+			return "", err
 		}
 	}
-	return p, nil
+	return path.Join(dir, p), nil
 }

 func (a *fileActionCopy) addCaps(f *FileOp) {
--- a/client/llb/meta.go
+++ b/client/llb/meta.go
@@ -10,6 +10,7 @@ import (
 	"github.com/google/shlex"
 	"github.com/moby/buildkit/solver/pb"
 	ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
+	"github.com/pkg/errors"
 )

 type contextKeyT string
@@ -78,7 +79,7 @@ func dirf(value string, replace bool, v ...interface{}) StateOption {
 			if !path.IsAbs(value) {
 				prev, err := getDir(s)(ctx, c)
 				if err != nil {
-					return nil, err
+					return nil, errors.Wrap(err, "getting dir from state")
 				}
 				if prev == "" {
 					prev = "/"
--- a/client/llb/meta_test.go
+++ b/client/llb/meta_test.go
@@ -4,24 +4,25 @@ import (
 	"context"
 	"testing"

+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )

 func TestRelativeWd(t *testing.T) {
 	st := Scratch().Dir("foo")
-	require.Equal(t, getDirHelper(t, st), "/foo")
+	assert.Equal(t, getDirHelper(t, st), "/foo")

 	st = st.Dir("bar")
-	require.Equal(t, getDirHelper(t, st), "/foo/bar")
+	assert.Equal(t, getDirHelper(t, st), "/foo/bar")

 	st = st.Dir("..")
-	require.Equal(t, getDirHelper(t, st), "/foo")
+	assert.Equal(t, getDirHelper(t, st), "/foo")

 	st = st.Dir("/baz")
-	require.Equal(t, getDirHelper(t, st), "/baz")
+	assert.Equal(t, getDirHelper(t, st), "/baz")

 	st = st.Dir("../../..")
-	require.Equal(t, getDirHelper(t, st), "/")
+	assert.Equal(t, getDirHelper(t, st), "/")
 }

 func getDirHelper(t *testing.T, s State) string {
--- a/frontend/dockerfile/dockerfile2llb/convert.go
+++ b/frontend/dockerfile/dockerfile2llb/convert.go
@@ -360,6 +360,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 			if d.stage.BaseName == emptyImageName {
 				d.state = llb.Scratch()
 				d.image = emptyImage(platformOpt.targetPlatform)
+				d.platform = &platformOpt.targetPlatform
 				continue
 			}
 			func(i int, d *dispatchState) {
@@ -486,11 +487,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS

 		// make sure that PATH is always set
 		if _, ok := shell.BuildEnvs(d.image.Config.Env)["PATH"]; !ok {
-			var pathOS string
-			if d.platform != nil {
-				pathOS = d.platform.OS
-			}
-			d.image.Config.Env = append(d.image.Config.Env, "PATH="+system.DefaultPathEnv(pathOS))
+			d.image.Config.Env = append(d.image.Config.Env, "PATH="+system.DefaultPathEnv(d.platform.OS))
 		}

 		// initialize base metadata from image conf
@@ -899,6 +896,7 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE
 				st := llb.Scratch().Dir(sourcePath).File(
 					llb.Mkfile(f, 0755, []byte(data)),
 					dockerui.WithInternalName("preparing inline document"),
+					llb.Platform(*d.platform),
 				)

 				mount := llb.AddMount(destPath, st, llb.SourcePath(sourcePath), llb.Readonly)
@@ -1002,12 +1000,20 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE
 }

 func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bool, opt *dispatchOpt) error {
-	d.state = d.state.Dir(c.Path)
-	wd := c.Path
-	if !path.IsAbs(c.Path) {
-		wd = path.Join("/", d.image.Config.WorkingDir, wd)
+	wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, d.platform.OS)
+	if err != nil {
+		return errors.Wrap(err, "normalizing workdir")
 	}
+
+	// NormalizeWorkdir returns paths with platform specific separators. For Windows
+	// this will be of the form: \some\path, which is needed later when we pass it to
+	// HCS.
 	d.image.Config.WorkingDir = wd
+
+	// From this point forward, we can use UNIX style paths.
+	wd = system.ToSlash(wd, d.platform.OS)
+	d.state = d.state.Dir(wd)
+
 	if commit {
 		withLayer := false
 		if wd != "/" {
@@ -1026,6 +1032,7 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo
 			d.state = d.state.File(llb.Mkdir(wd, 0755, mkdirOpt...),
 				llb.WithCustomName(prefixCommand(d, uppercaseCmd(processCmdEnv(opt.shlex, c.String(), env)), d.prefixPlatform, &platform, env)),
 				location(opt.sourceMap, c.Location()),
+				llb.Platform(*d.platform),
 			)
 			withLayer = true
 		}
@@ -1035,11 +1042,11 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo
 }

 func dispatchCopy(d *dispatchState, cfg copyConfig) error {
-	pp, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath)
+	dest, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath, *d.platform)
 	if err != nil {
 		return err
 	}
-	dest := path.Join("/", pp)
+
 	if cfg.params.DestPath == "." || cfg.params.DestPath == "" || cfg.params.DestPath[len(cfg.params.DestPath)-1] == filepath.Separator {
 		dest += string(filepath.Separator)
 	}
@@ -1137,6 +1144,11 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
 				a = a.Copy(st, f, dest, opts...)
 			}
 		} else {
+			src, err = system.NormalizePath("/", src, d.platform.OS, false)
+			if err != nil {
+				return errors.Wrap(err, "removing drive letter")
+			}
+
 			opts := append([]llb.CopyOption{&llb.CopyInfo{
 				Mode:                mode,
 				FollowSymlinks:      true,
@@ -1148,9 +1160,9 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
 			}}, copyOpt...)

 			if a == nil {
-				a = llb.Copy(cfg.source, filepath.Join("/", src), dest, opts...)
+				a = llb.Copy(cfg.source, src, dest, opts...)
 			} else {
-				a = a.Copy(cfg.source, filepath.Join("/", src), dest, opts...)
+				a = a.Copy(cfg.source, src, dest, opts...)
 			}
 		}
 	}
@@ -1159,10 +1171,14 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
 		commitMessage.WriteString(" <<" + src.Path)

 		data := src.Data
-		f := src.Path
+		f, err := system.CheckSystemDriveAndRemoveDriveLetter(src.Path, d.platform.OS)
+		if err != nil {
+			return errors.Wrap(err, "removing drive letter")
+		}
 		st := llb.Scratch().File(
 			llb.Mkfile(f, 0644, []byte(data)),
 			dockerui.WithInternalName("preparing inline document"),
+			llb.Platform(*d.platform),
 		)

 		opts := append([]llb.CopyOption{&llb.CopyInfo{
@@ -1171,9 +1187,9 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
 		}}, copyOpt...)

 		if a == nil {
-			a = llb.Copy(st, f, dest, opts...)
+			a = llb.Copy(st, system.ToSlash(f, d.platform.OS), dest, opts...)
 		} else {
-			a = a.Copy(st, f, dest, opts...)
+			a = a.Copy(st, filepath.ToSlash(f), dest, opts...)
 		}
 	}

@@ -1204,7 +1220,9 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error {
 		d.cmdIndex-- // prefixCommand increases it
 		pgName := prefixCommand(d, name, d.prefixPlatform, &platform, env)

-		var copyOpts []llb.ConstraintsOpt
+		copyOpts := []llb.ConstraintsOpt{
+			llb.Platform(*d.platform),
+		}
 		copy(copyOpts, fileOpt)
 		copyOpts = append(copyOpts, llb.ProgressGroup(pgID, pgName, true))

@@ -1397,15 +1415,24 @@ func dispatchArg(d *dispatchState, c *instructions.ArgCommand, metaArgs []instru
 	return commitToHistory(&d.image, "ARG "+strings.Join(commitStrs, " "), false, nil, d.epoch)
 }

-func pathRelativeToWorkingDir(s llb.State, p string) (string, error) {
-	if path.IsAbs(p) {
-		return p, nil
-	}
-	dir, err := s.GetDir(context.TODO())
+func pathRelativeToWorkingDir(s llb.State, p string, platform ocispecs.Platform) (string, error) {
+	dir, err := s.GetDir(context.TODO(), llb.Platform(platform))
 	if err != nil {
 		return "", err
 	}
-	return path.Join(dir, p), nil
+
+	if len(p) == 0 {
+		return dir, nil
+	}
+	p, err = system.CheckSystemDriveAndRemoveDriveLetter(p, platform.OS)
+	if err != nil {
+		return "", errors.Wrap(err, "removing drive letter")
+	}
+
+	if system.IsAbs(p, platform.OS) {
+		return system.NormalizePath("/", p, platform.OS, false)
+	}
+	return system.NormalizePath(dir, p, platform.OS, false)
 }

 func addEnv(env []string, k, v string) []string {
--- a/frontend/gateway/container/container.go
+++ b/frontend/gateway/container/container.go
@@ -12,6 +12,7 @@ import (

 	"github.com/moby/buildkit/session/secrets"
 	"github.com/moby/buildkit/util/bklog"
+	"github.com/moby/buildkit/util/system"

 	"github.com/moby/buildkit/cache"
 	"github.com/moby/buildkit/executor"
@@ -92,7 +93,7 @@ func NewContainer(ctx context.Context, w worker.Worker, sm *session.Manager, g s
 			cm = refs[m.Input].Worker.CacheManager()
 		}
 		return cm.New(ctx, ref, g)
-	})
+	}, platform.OS)
 	if err != nil {
 		for i := len(p.Actives) - 1; i >= 0; i-- { // call in LIFO order
 			p.Actives[i].Ref.Release(context.TODO())
@@ -142,7 +143,7 @@ type MountMutableRef struct {

 type MakeMutable func(m *opspb.Mount, ref cache.ImmutableRef) (cache.MutableRef, error)

-func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manager, g session.Group, cwd string, mnts []*opspb.Mount, refs []*worker.WorkerRef, makeMutable MakeMutable) (p PreparedMounts, err error) {
+func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manager, g session.Group, cwd string, mnts []*opspb.Mount, refs []*worker.WorkerRef, makeMutable MakeMutable, platform string) (p PreparedMounts, err error) {
 	// loop over all mounts, fill in mounts, root and outputs
 	for i, m := range mnts {
 		var (
@@ -265,7 +266,7 @@ func PrepareMounts(ctx context.Context, mm *mounts.MountManager, cm cache.Manage
 		} else {
 			mws := MountWithSession(mountable, g)
 			dest := m.Dest
-			if !filepath.IsAbs(filepath.Clean(dest)) {
+			if !system.IsAbs(filepath.Clean(dest), platform) {
 				dest = filepath.Join("/", cwd, dest)
 			}
 			mws.Dest = dest
--- a/solver/llbsolver/file/backend.go
+++ b/solver/llbsolver/file/backend.go
@@ -5,6 +5,7 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"time"

@@ -13,6 +14,7 @@ import (
 	"github.com/moby/buildkit/snapshot"
 	"github.com/moby/buildkit/solver/llbsolver/ops/fileoptypes"
 	"github.com/moby/buildkit/solver/pb"
+	"github.com/moby/buildkit/util/system"
 	"github.com/pkg/errors"
 	copy "github.com/tonistiigi/fsutil/copy"
 )
@@ -66,7 +68,7 @@ func mapUserToChowner(user *copy.User, idmap *idtools.IdentityMapping) (copy.Cho
 }

 func mkdir(ctx context.Context, d string, action pb.FileActionMkDir, user *copy.User, idmap *idtools.IdentityMapping) error {
-	p, err := fs.RootPath(d, filepath.Join("/", action.Path))
+	p, err := fs.RootPath(d, action.Path)
 	if err != nil {
 		return err
 	}
@@ -126,7 +128,10 @@ func mkfile(ctx context.Context, d string, action pb.FileActionMkFile, user *cop

 func rm(ctx context.Context, d string, action pb.FileActionRm) error {
 	if action.AllowWildcard {
-		src := cleanPath(action.Path)
+		src, err := cleanPath(action.Path)
+		if err != nil {
+			return errors.Wrap(err, "cleaning path")
+		}
 		m, err := copy.ResolveWildcards(d, src, false)
 		if err != nil {
 			return err
@@ -167,9 +172,14 @@ func rmPath(root, src string, allowNotFound bool) error {
 }

 func docopy(ctx context.Context, src, dest string, action pb.FileActionCopy, u *copy.User, idmap *idtools.IdentityMapping) error {
-	srcPath := cleanPath(action.Src)
-	destPath := cleanPath(action.Dest)
-
+	srcPath, err := cleanPath(action.Src)
+	if err != nil {
+		return errors.Wrap(err, "cleaning source path")
+	}
+	destPath, err := cleanPath(action.Dest)
+	if err != nil {
+		return errors.Wrap(err, "cleaning path")
+	}
 	if !action.CreateDestPath {
 		p, err := fs.RootPath(dest, filepath.Join("/", action.Dest))
 		if err != nil {
@@ -244,19 +254,6 @@ func docopy(ctx context.Context, src, dest string, action pb.FileActionCopy, u *
 	return nil
 }

-func cleanPath(s string) string {
-	s2 := filepath.Join("/", s)
-	if strings.HasSuffix(s, "/.") {
-		if s2 != "/" {
-			s2 += "/"
-		}
-		s2 += "."
-	} else if strings.HasSuffix(s, "/") && s2 != "/" {
-		s2 += "/"
-	}
-	return s2
-}
-
 type Backend struct {
 }

@@ -349,3 +346,21 @@ func (fb *Backend) Copy(ctx context.Context, m1, m2, user, group fileoptypes.Mou

 	return docopy(ctx, src, dest, action, u, mnt2.m.IdentityMapping())
 }
+
+func cleanPath(s string) (string, error) {
+	s, err := system.CheckSystemDriveAndRemoveDriveLetter(s, runtime.GOOS)
+	if err != nil {
+		return "", errors.Wrap(err, "removing drive letter")
+	}
+	s = filepath.FromSlash(s)
+	s2 := filepath.Join("/", s)
+	if strings.HasSuffix(s, string(filepath.Separator)+".") {
+		if s2 != string(filepath.Separator) {
+			s2 += string(filepath.Separator)
+		}
+		s2 += "."
+	} else if strings.HasSuffix(s, string(filepath.Separator)) && s2 != string(filepath.Separator) {
+		s2 += string(filepath.Separator)
+	}
+	return s2, nil
+}
--- a/solver/llbsolver/ops/exec.go
+++ b/solver/llbsolver/ops/exec.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"os"
 	"path"
+	"runtime"
 	"sort"
 	"strings"

@@ -260,10 +261,14 @@ func (e *ExecOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu
 		}
 	}

+	platformOS := runtime.GOOS
+	if e.platform != nil {
+		platformOS = e.platform.OS
+	}
 	p, err := container.PrepareMounts(ctx, e.mm, e.cm, g, e.op.Meta.Cwd, e.op.Mounts, refs, func(m *pb.Mount, ref cache.ImmutableRef) (cache.MutableRef, error) {
 		desc := fmt.Sprintf("mount %s from exec %s", m.Dest, strings.Join(e.op.Meta.Args, " "))
 		return e.cm.New(ctx, ref, g, cache.WithDescription(desc))
-	})
+	}, platformOS)
 	defer func() {
 		if err != nil {
 			execInputs := make([]solver.Result, len(e.op.Mounts))
--- a/util/system/path.go
+++ b/util/system/path.go
@@ -37,8 +37,8 @@ func NormalizePath(parent, newPath, inputOS string, keepSlash bool) (string, err
 		inputOS = "linux"
 	}

-	newPath = toSlash(newPath, inputOS)
-	parent = toSlash(parent, inputOS)
+	newPath = ToSlash(newPath, inputOS)
+	parent = ToSlash(parent, inputOS)
 	origPath := newPath

 	if parent == "" {
@@ -82,18 +82,17 @@ func NormalizePath(parent, newPath, inputOS string, keepSlash bool) (string, err
 		}
 	}

-	return toSlash(newPath, inputOS), nil
+	return ToSlash(newPath, inputOS), nil
 }

-func toSlash(inputPath, inputOS string) string {
-	separator := "/"
-	if inputOS == "windows" {
-		separator = "\\"
+func ToSlash(inputPath, inputOS string) string {
+	if inputOS != "windows" {
+		return inputPath
 	}
-	return strings.Replace(inputPath, separator, "/", -1)
+	return strings.Replace(inputPath, "\\", "/", -1)
 }

-func fromSlash(inputPath, inputOS string) string {
+func FromSlash(inputPath, inputOS string) string {
 	separator := "/"
 	if inputOS == "windows" {
 		separator = "\\"
@@ -119,7 +118,7 @@ func NormalizeWorkdir(current, wd string, inputOS string) (string, error) {

 	// Make sure we use the platform specific path separator. HCS does not like forward
 	// slashes in CWD.
-	return fromSlash(wd, inputOS), nil
+	return FromSlash(wd, inputOS), nil
 }

 // IsAbs returns a boolean value indicating whether or not the path
@@ -142,7 +141,7 @@ func IsAbs(pth, inputOS string) bool {
 	if err != nil {
 		return false
 	}
-	cleanedPath = toSlash(cleanedPath, inputOS)
+	cleanedPath = ToSlash(cleanedPath, inputOS)
 	// We stripped any potential drive letter and converted any backslashes to
 	// forward slashes. We can safely use path.IsAbs() for both Windows and Linux.
 	return path.IsAbs(cleanedPath)
@@ -189,14 +188,14 @@ func CheckSystemDriveAndRemoveDriveLetter(path string, inputOS string) (string,
 	}

 	// UNC paths should error out
-	if len(path) >= 2 && toSlash(path[:2], inputOS) == "//" {
+	if len(path) >= 2 && ToSlash(path[:2], inputOS) == "//" {
 		return "", errors.Errorf("UNC paths are not supported")
 	}

 	parts := strings.SplitN(path, ":", 2)
 	// Path does not have a drive letter. Just return it.
 	if len(parts) < 2 {
-		return toSlash(filepath.Clean(path), inputOS), nil
+		return ToSlash(filepath.Clean(path), inputOS), nil
 	}

 	// We expect all paths to be in C:
@@ -221,5 +220,5 @@ func CheckSystemDriveAndRemoveDriveLetter(path string, inputOS string) (string,
 	//
 	// We must return the second element of the split path, as is, without attempting to convert
 	// it to an absolute path. We have no knowledge of the CWD; that is treated elsewhere.
-	return toSlash(filepath.Clean(parts[1]), inputOS), nil
+	return ToSlash(filepath.Clean(parts[1]), inputOS), nil
 }