From 686a84b428f794c51128ad4722ad136c81d0d7fd Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Fri, 26 May 2023 12:48:56 +0000 Subject: [PATCH 01/12] Handle file paths base on target platform This change properly handles paths on different platforms. In short, this change checks the target platform we're building an image for and applies normalization steps to make sure the file paths are valid. This makes buildkit properly handle paths on both *nix systems and on Windows. Signed-off-by: Gabriel Adrian Samfira --- client/llb/fileop.go | 91 ++++++++++--------- client/llb/meta.go | 17 ++-- client/llb/meta_test.go | 11 ++- frontend/dockerfile/dockerfile2llb/convert.go | 57 +++++++++--- frontend/gateway/container/container.go | 7 +- solver/llbsolver/file/backend.go | 54 +++++++---- solver/llbsolver/ops/exec.go | 6 +- 7 files changed, 154 insertions(+), 89 deletions(-) diff --git a/client/llb/fileop.go b/client/llb/fileop.go index fb7a80a05..8573d12ca 100644 --- a/client/llb/fileop.go +++ b/client/llb/fileop.go @@ -4,12 +4,13 @@ import ( "context" _ "crypto/sha256" // for opencontainers/go-digest "os" - "path" + "path/filepath" "strconv" "strings" "time" "github.com/moby/buildkit/solver/pb" + "github.com/moby/buildkit/util/system" digest "github.com/opencontainers/go-digest" "github.com/pkg/errors" ) @@ -54,7 +55,7 @@ type CopyInput interface { } type subAction interface { - toProtoAction(context.Context, string, pb.InputIndex) (pb.IsFileAction, error) + toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) } type capAdder interface { @@ -160,10 +161,14 @@ type fileActionMkdir struct { info MkdirInfo } -func (a *fileActionMkdir) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) { +func (a *fileActionMkdir) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) { + normalizedPath, err := system.NormalizePath(parent, a.file, platform, false) + if err != nil { + return nil, errors.Wrap(err, "normalizing path") + } return &pb.FileAction_Mkdir{ Mkdir: &pb.FileActionMkDir{ - Path: normalizePath(parent, a.file, false), + Path: normalizedPath, Mode: int32(a.mode & 0777), MakeParents: a.info.MakeParents, Owner: a.info.ChownOpt.marshal(base), @@ -334,10 +339,14 @@ type fileActionMkfile struct { info MkfileInfo } -func (a *fileActionMkfile) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) { +func (a *fileActionMkfile) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) { + normalizedPath, err := system.NormalizePath(parent, a.file, platform, false) + if err != nil { + return nil, errors.Wrap(err, "normalizing path") + } return &pb.FileAction_Mkfile{ Mkfile: &pb.FileActionMkFile{ - Path: normalizePath(parent, a.file, false), + Path: normalizedPath, Mode: int32(a.mode & 0777), Data: a.dt, Owner: a.info.ChownOpt.marshal(base), @@ -402,10 +411,14 @@ type fileActionRm struct { info RmInfo } -func (a *fileActionRm) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) { +func (a *fileActionRm) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) { + normalizedPath, err := system.NormalizePath(parent, a.file, platform, false) + if err != nil { + return nil, errors.Wrap(err, "normalizing path") + } return &pb.FileAction_Rm{ Rm: &pb.FileActionRm{ - Path: normalizePath(parent, a.file, false), + Path: normalizedPath, AllowNotFound: a.info.AllowNotFound, AllowWildcard: a.info.AllowWildcard, }, @@ -492,14 +505,18 @@ type fileActionCopy struct { info CopyInfo } -func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) { - src, err := a.sourcePath(ctx) +func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) { + src, err := a.sourcePath(ctx, platform) if err != nil { return nil, err } + normalizedPath, err := system.NormalizePath(parent, a.dest, platform, false) + if err != nil { + return nil, errors.Wrap(err, "normalizing path") + } c := &pb.FileActionCopy{ Src: src, - Dest: normalizePath(parent, a.dest, true), + Dest: normalizedPath, Owner: a.info.ChownOpt.marshal(base), IncludePatterns: a.info.IncludePatterns, ExcludePatterns: a.info.ExcludePatterns, @@ -521,21 +538,22 @@ func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base }, nil } -func (a *fileActionCopy) sourcePath(ctx context.Context) (string, error) { - p := path.Clean(a.src) - if !path.IsAbs(p) { +func (a *fileActionCopy) sourcePath(ctx context.Context, platform string) (string, error) { + p := filepath.Clean(a.src) + if !system.IsAbs(p, platform) { + var dir string + var err error 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 + } + p, err = system.NormalizePath(dir, p, platform, false) + if err != nil { + return "", errors.Wrap(err, "normalizing source path") } } return p, nil @@ -753,7 +771,11 @@ func (f *FileOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, [] } } - action, err := st.action.toProtoAction(ctx, parent, st.base) + var platform string + if f.constraints.Platform != nil { + platform = f.constraints.Platform.OS + } + action, err := st.action.toProtoAction(ctx, parent, st.base, platform) if err != nil { return "", nil, nil, nil, err } @@ -774,25 +796,6 @@ func (f *FileOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, [] return f.Load() } -func normalizePath(parent, p string, keepSlash bool) string { - origPath := p - p = path.Clean(p) - if !path.IsAbs(p) { - p = path.Join("/", parent, p) - } - if keepSlash { - if strings.HasSuffix(origPath, "/") && !strings.HasSuffix(p, "/") { - p += "/" - } else if strings.HasSuffix(origPath, "/.") { - if p != "/" { - p += "/" - } - p += "." - } - } - return p -} - func (f *FileOp) Output() Output { return f.output } diff --git a/client/llb/meta.go b/client/llb/meta.go index f4e67efe5..7e307d6bc 100644 --- a/client/llb/meta.go +++ b/client/llb/meta.go @@ -4,12 +4,13 @@ import ( "context" "fmt" "net" - "path" "github.com/containerd/containerd/platforms" "github.com/google/shlex" "github.com/moby/buildkit/solver/pb" + "github.com/moby/buildkit/util/system" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/pkg/errors" ) type contextKeyT string @@ -75,15 +76,19 @@ func dirf(value string, replace bool, v ...interface{}) StateOption { } return func(s State) State { return s.withValue(keyDir, func(ctx context.Context, c *Constraints) (interface{}, error) { - if !path.IsAbs(value) { + var platform string + if c != nil && c.Platform != nil { + platform = c.Platform.OS + } + if !system.IsAbs(value, platform) { prev, err := getDir(s)(ctx, c) if err != nil { - return nil, err + return nil, errors.Wrap(err, "getting dir from state") } - if prev == "" { - prev = "/" + value, err = system.NormalizePath(prev, value, platform, false) + if err != nil { + return nil, errors.Wrap(err, "normalizing path") } - value = path.Join(prev, value) } return value, nil }) diff --git a/client/llb/meta_test.go b/client/llb/meta_test.go index 3c7112d2d..ef67d89e2 100644 --- 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 { diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 3deba448d..973e49cd1 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -994,11 +994,16 @@ 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) + var platformOS string + if d != nil && d.platform != nil { + platformOS = d.platform.OS } + wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, platformOS) + if err != nil { + return errors.Wrap(err, "normalizing workdir") + } + + d.state = d.state.Dir(wd) d.image.Config.WorkingDir = wd if commit { withLayer := false @@ -1027,11 +1032,16 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo } func dispatchCopy(d *dispatchState, cfg copyConfig) error { - pp, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath) + var platformOS string + if d.platform != nil { + platformOS = d.platform.OS + } + pp, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath, platformOS) if err != nil { return err } - dest := path.Join("/", pp) + dest := filepath.Join("/", pp) + if cfg.params.DestPath == "." || cfg.params.DestPath == "" || cfg.params.DestPath[len(cfg.params.DestPath)-1] == filepath.Separator { dest += string(filepath.Separator) } @@ -1135,6 +1145,11 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { a = a.Copy(st, f, dest, opts...) } } else { + src, err = system.CheckSystemDriveAndRemoveDriveLetter(src, platformOS) + if err != nil { + return errors.Wrap(err, "removing drive letter") + } + opts := append([]llb.CopyOption{&llb.CopyInfo{ Mode: mode, FollowSymlinks: true, @@ -1157,7 +1172,10 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { commitMessage.WriteString(" <<" + src.Path) data := src.Data - f := src.Path + f, err := system.CheckSystemDriveAndRemoveDriveLetter(src.Path, platformOS) + if err != nil { + return errors.Wrap(err, "removing drive letter") + } st := llb.Scratch().File( llb.Mkfile(f, 0664, []byte(data)), dockerui.WithInternalName("preparing inline document"), @@ -1168,6 +1186,11 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { CreateDestPath: true, }}, copyOpt...) + dest, err = system.CheckSystemDriveAndRemoveDriveLetter(dest, platformOS) + if err != nil { + return errors.Wrap(err, "removing drive letter") + } + if a == nil { a = llb.Copy(st, f, dest, opts...) } else { @@ -1394,15 +1417,25 @@ 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 - } +func pathRelativeToWorkingDir(s llb.State, p, platform string) (string, error) { dir, err := s.GetDir(context.TODO()) if err != nil { return "", err } - return path.Join(dir, p), nil + + if len(p) == 0 { + return dir, nil + } + + p, err = system.CheckSystemDriveAndRemoveDriveLetter(p, platform) + if err != nil { + return "", errors.Wrap(err, "remving drive letter") + } + + if system.IsAbs(p, platform) { + return p, nil + } + return filepath.Join(dir, p), nil } func addEnv(env []string, k, v string) []string { diff --git a/frontend/gateway/container/container.go b/frontend/gateway/container/container.go index 6555fd6de..af6476e7f 100644 --- 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 diff --git a/solver/llbsolver/file/backend.go b/solver/llbsolver/file/backend.go index 974c2e04e..06c6b076b 100644 --- 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,11 @@ 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)) + actionPath, err := system.NormalizePath("/", action.Path, runtime.GOOS, false) + if err != nil { + return errors.Wrap(err, "removing drive letter") + } + p, err := fs.RootPath(d, filepath.FromSlash(actionPath)) if err != nil { return err } @@ -126,7 +132,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 +176,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 +258,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 +350,20 @@ 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") + } + s2 := filepath.Join("/", s) + if strings.HasSuffix(filepath.FromSlash(s), string(filepath.Separator)+".") { + if s2 != string(filepath.Separator) { + s2 += string(filepath.Separator) + } + s2 += "." + } else if strings.HasSuffix(filepath.FromSlash(s), string(filepath.Separator)) && s2 != string(filepath.Separator) { + s2 += string(filepath.Separator) + } + return s2, nil +} diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index 48556b60a..ea4c624ba 100644 --- a/solver/llbsolver/ops/exec.go +++ b/solver/llbsolver/ops/exec.go @@ -260,10 +260,14 @@ func (e *ExecOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu } } + var platformOS string + 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)) From 236d00b59ab7798359245e1d8c85767d57a88cee Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Wed, 31 May 2023 14:13:44 -0700 Subject: [PATCH 02/12] Use current OS as a default Signed-off-by: Gabriel Adrian Samfira --- client/llb/fileop.go | 4 +++- client/llb/meta.go | 3 ++- frontend/dockerfile/dockerfile2llb/convert.go | 5 +++-- solver/llbsolver/ops/exec.go | 3 ++- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/client/llb/fileop.go b/client/llb/fileop.go index 8573d12ca..5d599c51d 100644 --- a/client/llb/fileop.go +++ b/client/llb/fileop.go @@ -5,6 +5,7 @@ import ( _ "crypto/sha256" // for opencontainers/go-digest "os" "path/filepath" + "runtime" "strconv" "strings" "time" @@ -771,7 +772,8 @@ func (f *FileOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, [] } } - var platform string + // Assume that we're building an image for the same OS we're running on. + platform := runtime.GOOS if f.constraints.Platform != nil { platform = f.constraints.Platform.OS } diff --git a/client/llb/meta.go b/client/llb/meta.go index 7e307d6bc..f5077379e 100644 --- a/client/llb/meta.go +++ b/client/llb/meta.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "net" + "runtime" "github.com/containerd/containerd/platforms" "github.com/google/shlex" @@ -76,7 +77,7 @@ func dirf(value string, replace bool, v ...interface{}) StateOption { } return func(s State) State { return s.withValue(keyDir, func(ctx context.Context, c *Constraints) (interface{}, error) { - var platform string + platform := runtime.GOOS if c != nil && c.Platform != nil { platform = c.Platform.OS } diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 973e49cd1..f5ada16bd 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -10,6 +10,7 @@ import ( "os" "path" "path/filepath" + "runtime" "sort" "strconv" "strings" @@ -994,7 +995,7 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE } func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bool, opt *dispatchOpt) error { - var platformOS string + platformOS := runtime.GOOS if d != nil && d.platform != nil { platformOS = d.platform.OS } @@ -1032,7 +1033,7 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo } func dispatchCopy(d *dispatchState, cfg copyConfig) error { - var platformOS string + platformOS := runtime.GOOS if d.platform != nil { platformOS = d.platform.OS } diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index ea4c624ba..eee0dd39f 100644 --- a/solver/llbsolver/ops/exec.go +++ b/solver/llbsolver/ops/exec.go @@ -7,6 +7,7 @@ import ( "fmt" "os" "path" + "runtime" "sort" "strings" @@ -260,7 +261,7 @@ func (e *ExecOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu } } - var platformOS string + platformOS := runtime.GOOS if e.platform != nil { platformOS = e.platform.OS } From 7a503aef505a70ba7155dc6260a43dc9816a4ac3 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Thu, 1 Jun 2023 18:50:41 +0300 Subject: [PATCH 03/12] Rename variable, remove superfluous check Signed-off-by: Gabriel Adrian Samfira --- client/llb/fileop.go | 26 +++++++++---------- frontend/dockerfile/dockerfile2llb/convert.go | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/client/llb/fileop.go b/client/llb/fileop.go index 5d599c51d..2276f05d1 100644 --- a/client/llb/fileop.go +++ b/client/llb/fileop.go @@ -56,7 +56,7 @@ type CopyInput interface { } type subAction interface { - toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) + toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platformOS string) (pb.IsFileAction, error) } type capAdder interface { @@ -162,8 +162,8 @@ type fileActionMkdir struct { info MkdirInfo } -func (a *fileActionMkdir) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) { - normalizedPath, err := system.NormalizePath(parent, a.file, platform, false) +func (a *fileActionMkdir) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platformOS string) (pb.IsFileAction, error) { + normalizedPath, err := system.NormalizePath(parent, a.file, platformOS, false) if err != nil { return nil, errors.Wrap(err, "normalizing path") } @@ -340,8 +340,8 @@ type fileActionMkfile struct { info MkfileInfo } -func (a *fileActionMkfile) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) { - normalizedPath, err := system.NormalizePath(parent, a.file, platform, false) +func (a *fileActionMkfile) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platformOS string) (pb.IsFileAction, error) { + normalizedPath, err := system.NormalizePath(parent, a.file, platformOS, false) if err != nil { return nil, errors.Wrap(err, "normalizing path") } @@ -412,8 +412,8 @@ type fileActionRm struct { info RmInfo } -func (a *fileActionRm) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) { - normalizedPath, err := system.NormalizePath(parent, a.file, platform, false) +func (a *fileActionRm) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platformOS string) (pb.IsFileAction, error) { + normalizedPath, err := system.NormalizePath(parent, a.file, platformOS, false) if err != nil { return nil, errors.Wrap(err, "normalizing path") } @@ -506,12 +506,12 @@ type fileActionCopy struct { info CopyInfo } -func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platform string) (pb.IsFileAction, error) { - src, err := a.sourcePath(ctx, platform) +func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platformOS string) (pb.IsFileAction, error) { + src, err := a.sourcePath(ctx, platformOS) if err != nil { return nil, err } - normalizedPath, err := system.NormalizePath(parent, a.dest, platform, false) + normalizedPath, err := system.NormalizePath(parent, a.dest, platformOS, false) if err != nil { return nil, errors.Wrap(err, "normalizing path") } @@ -773,11 +773,11 @@ func (f *FileOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, [] } // Assume that we're building an image for the same OS we're running on. - platform := runtime.GOOS + platformOS := runtime.GOOS if f.constraints.Platform != nil { - platform = f.constraints.Platform.OS + platformOS = f.constraints.Platform.OS } - action, err := st.action.toProtoAction(ctx, parent, st.base, platform) + action, err := st.action.toProtoAction(ctx, parent, st.base, platformOS) if err != nil { return "", nil, nil, nil, err } diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index f5ada16bd..eb77790c6 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -996,7 +996,7 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bool, opt *dispatchOpt) error { platformOS := runtime.GOOS - if d != nil && d.platform != nil { + if d.platform != nil { platformOS = d.platform.OS } wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, platformOS) From b29ec0b04e4dde4f5a28c7c4d614c20cd03a64c4 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Fri, 2 Jun 2023 22:04:47 +0300 Subject: [PATCH 04/12] Remove nil pointer check and extra NormalizePath Signed-off-by: Gabriel Adrian Samfira --- frontend/dockerfile/dockerfile2llb/convert.go | 6 +----- solver/llbsolver/file/backend.go | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index eb77790c6..f5bb183e5 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -995,11 +995,7 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE } func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bool, opt *dispatchOpt) error { - platformOS := runtime.GOOS - if d.platform != nil { - platformOS = d.platform.OS - } - wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, platformOS) + wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, d.platform.OS) if err != nil { return errors.Wrap(err, "normalizing workdir") } diff --git a/solver/llbsolver/file/backend.go b/solver/llbsolver/file/backend.go index 06c6b076b..825341655 100644 --- a/solver/llbsolver/file/backend.go +++ b/solver/llbsolver/file/backend.go @@ -68,11 +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 { - actionPath, err := system.NormalizePath("/", action.Path, runtime.GOOS, false) - if err != nil { - return errors.Wrap(err, "removing drive letter") - } - p, err := fs.RootPath(d, filepath.FromSlash(actionPath)) + p, err := fs.RootPath(d, action.Path) if err != nil { return err } From fbbaa39c53b3f136374006b5342b6dea0f211dfe Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Fri, 2 Jun 2023 22:19:19 +0300 Subject: [PATCH 05/12] Add nil pointer check in dispatchWorkdir Signed-off-by: Gabriel Adrian Samfira --- frontend/dockerfile/dockerfile2llb/convert.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index f5bb183e5..eb77790c6 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -995,7 +995,11 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE } func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bool, opt *dispatchOpt) error { - wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, d.platform.OS) + platformOS := runtime.GOOS + if d.platform != nil { + platformOS = d.platform.OS + } + wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, platformOS) if err != nil { return errors.Wrap(err, "normalizing workdir") } From 051949167da5d9c1a569ddadb15e77185fddec1e Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Thu, 15 Jun 2023 13:27:47 -0700 Subject: [PATCH 06/12] Set default platform * Set default platform for scratch images * Set default platform constraint for FileOps if none is set via ConstraintOpt. * Explicitly set platform ConstraintOpt in dockerfile frontend based on dispatch state platform. Signed-off-by: Gabriel Adrian Samfira --- client/llb/fileop.go | 8 +--- client/llb/state.go | 5 +++ frontend/dockerfile/dockerfile2llb/convert.go | 43 ++++++++----------- 3 files changed, 24 insertions(+), 32 deletions(-) diff --git a/client/llb/fileop.go b/client/llb/fileop.go index 2276f05d1..a0ef9f62a 100644 --- a/client/llb/fileop.go +++ b/client/llb/fileop.go @@ -5,7 +5,6 @@ import ( _ "crypto/sha256" // for opencontainers/go-digest "os" "path/filepath" - "runtime" "strconv" "strings" "time" @@ -772,12 +771,7 @@ func (f *FileOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, [] } } - // Assume that we're building an image for the same OS we're running on. - platformOS := runtime.GOOS - if f.constraints.Platform != nil { - platformOS = f.constraints.Platform.OS - } - action, err := st.action.toProtoAction(ctx, parent, st.base, platformOS) + action, err := st.action.toProtoAction(ctx, parent, st.base, f.constraints.Platform.OS) if err != nil { return "", nil, nil, nil, err } diff --git a/client/llb/state.go b/client/llb/state.go index f15fad87a..078637c21 100644 --- a/client/llb/state.go +++ b/client/llb/state.go @@ -299,6 +299,11 @@ func (s State) File(a *FileAction, opts ...ConstraintsOpt) State { o.SetConstraintsOption(&c) } + // No platform was set by constraint options. Set the current OS and arch. + if c.Platform == nil { + platform := platforms.DefaultSpec() + c.Platform = &platform + } return s.WithOutput(NewFileOp(s, a, c).Output()) } diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index eb77790c6..05c6fa71b 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -10,7 +10,6 @@ import ( "os" "path" "path/filepath" - "runtime" "sort" "strconv" "strings" @@ -361,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) { @@ -479,11 +479,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 @@ -892,6 +888,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) @@ -995,11 +992,7 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE } func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bool, opt *dispatchOpt) error { - platformOS := runtime.GOOS - if d.platform != nil { - platformOS = d.platform.OS - } - wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, platformOS) + wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, d.platform.OS) if err != nil { return errors.Wrap(err, "normalizing workdir") } @@ -1024,6 +1017,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 } @@ -1033,11 +1027,7 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo } func dispatchCopy(d *dispatchState, cfg copyConfig) error { - platformOS := runtime.GOOS - if d.platform != nil { - platformOS = d.platform.OS - } - pp, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath, platformOS) + pp, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath, *d.platform) if err != nil { return err } @@ -1146,7 +1136,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { a = a.Copy(st, f, dest, opts...) } } else { - src, err = system.CheckSystemDriveAndRemoveDriveLetter(src, platformOS) + src, err = system.CheckSystemDriveAndRemoveDriveLetter(src, d.platform.OS) if err != nil { return errors.Wrap(err, "removing drive letter") } @@ -1173,13 +1163,14 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { commitMessage.WriteString(" <<" + src.Path) data := src.Data - f, err := system.CheckSystemDriveAndRemoveDriveLetter(src.Path, platformOS) + 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, 0664, []byte(data)), dockerui.WithInternalName("preparing inline document"), + llb.Platform(*d.platform), ) opts := append([]llb.CopyOption{&llb.CopyInfo{ @@ -1187,7 +1178,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { CreateDestPath: true, }}, copyOpt...) - dest, err = system.CheckSystemDriveAndRemoveDriveLetter(dest, platformOS) + dest, err = system.CheckSystemDriveAndRemoveDriveLetter(dest, d.platform.OS) if err != nil { return errors.Wrap(err, "removing drive letter") } @@ -1226,7 +1217,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)) @@ -1418,8 +1411,8 @@ 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, platform string) (string, error) { - 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 } @@ -1428,15 +1421,15 @@ func pathRelativeToWorkingDir(s llb.State, p, platform string) (string, error) { return dir, nil } - p, err = system.CheckSystemDriveAndRemoveDriveLetter(p, platform) + p, err = system.CheckSystemDriveAndRemoveDriveLetter(p, platform.OS) if err != nil { return "", errors.Wrap(err, "remving drive letter") } - if system.IsAbs(p, platform) { + if system.IsAbs(p, platform.OS) { return p, nil } - return filepath.Join(dir, p), nil + return system.NormalizePath(dir, p, platform.OS, false) } func addEnv(env []string, k, v string) []string { From edae0c68121406a71c0addecf9900b4fd8066670 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Tue, 20 Jun 2023 05:16:46 -0700 Subject: [PATCH 07/12] Ensure we use proper path separators In dockerfile2llb we ensure we use proper path separators before we send them to LLB. In LLB we default to linux/amd64 if no constraints are set by the caller. Signed-off-by: Gabriel Adrian Samfira --- client/llb/fileop.go | 17 ++++++------ client/llb/state.go | 6 +++-- frontend/dockerfile/dockerfile2llb/convert.go | 27 ++++++++++--------- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/client/llb/fileop.go b/client/llb/fileop.go index a0ef9f62a..a22a38b73 100644 --- a/client/llb/fileop.go +++ b/client/llb/fileop.go @@ -460,7 +460,6 @@ func Copy(input CopyInput, src, dest string, opts ...CopyOption) *FileAction { for _, o := range opts { o.SetCopyOption(&mi) } - return &FileAction{ action: &fileActionCopy{ state: state, @@ -539,10 +538,12 @@ func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base } func (a *fileActionCopy) sourcePath(ctx context.Context, platform string) (string, error) { - p := filepath.Clean(a.src) + // filepath.Clean() also does a filepath.FromSlash(). Explicitly convert back to UNIX path + // separators. + p := filepath.ToSlash(filepath.Clean(a.src)) + dir := "/" + var err error if !system.IsAbs(p, platform) { - var dir string - var err error if a.state != nil { dir, err = a.state.GetDir(ctx) } else if a.fas != nil { @@ -551,10 +552,10 @@ func (a *fileActionCopy) sourcePath(ctx context.Context, platform string) (strin if err != nil { return "", err } - p, err = system.NormalizePath(dir, p, platform, false) - if err != nil { - return "", errors.Wrap(err, "normalizing source path") - } + } + p, err = system.NormalizePath(dir, p, platform, false) + if err != nil { + return "", errors.Wrap(err, "normalizing source path") } return p, nil } diff --git a/client/llb/state.go b/client/llb/state.go index 078637c21..b4e2c3548 100644 --- a/client/llb/state.go +++ b/client/llb/state.go @@ -301,8 +301,10 @@ func (s State) File(a *FileAction, opts ...ConstraintsOpt) State { // No platform was set by constraint options. Set the current OS and arch. if c.Platform == nil { - platform := platforms.DefaultSpec() - c.Platform = &platform + c.Platform = &ocispecs.Platform{ + OS: "linux", + Architecture: "amd64", + } } return s.WithOutput(NewFileOp(s, a, c).Output()) } diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 05c6fa71b..37f9c258f 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -997,8 +997,15 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo return errors.Wrap(err, "normalizing workdir") } - d.state = d.state.Dir(wd) + // 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 = filepath.ToSlash(wd) + d.state = d.state.Dir(wd) + if commit { withLayer := false if wd != "/" { @@ -1027,11 +1034,10 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo } func dispatchCopy(d *dispatchState, cfg copyConfig) error { - pp, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath, *d.platform) + dest, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath, *d.platform) if err != nil { return err } - dest := filepath.Join("/", pp) if cfg.params.DestPath == "." || cfg.params.DestPath == "" || cfg.params.DestPath[len(cfg.params.DestPath)-1] == filepath.Separator { dest += string(filepath.Separator) @@ -1136,7 +1142,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { a = a.Copy(st, f, dest, opts...) } } else { - src, err = system.CheckSystemDriveAndRemoveDriveLetter(src, d.platform.OS) + src, err = system.NormalizePath("/", src, d.platform.OS, false) if err != nil { return errors.Wrap(err, "removing drive letter") } @@ -1152,9 +1158,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...) } } } @@ -1178,15 +1184,10 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { CreateDestPath: true, }}, copyOpt...) - dest, err = system.CheckSystemDriveAndRemoveDriveLetter(dest, d.platform.OS) - if err != nil { - return errors.Wrap(err, "removing drive letter") - } - if a == nil { - a = llb.Copy(st, f, dest, opts...) + a = llb.Copy(st, filepath.ToSlash(f), dest, opts...) } else { - a = a.Copy(st, f, dest, opts...) + a = a.Copy(st, filepath.ToSlash(f), dest, opts...) } } From d425c7cd3c0d95d64c66781d7d671cabcf07af81 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Tue, 20 Jun 2023 06:00:57 -0700 Subject: [PATCH 08/12] Default to linux in client Signed-off-by: Gabriel Adrian Samfira --- client/llb/meta.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/llb/meta.go b/client/llb/meta.go index f5077379e..ead4f1b16 100644 --- a/client/llb/meta.go +++ b/client/llb/meta.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "net" - "runtime" "github.com/containerd/containerd/platforms" "github.com/google/shlex" @@ -77,7 +76,7 @@ func dirf(value string, replace bool, v ...interface{}) StateOption { } return func(s State) State { return s.withValue(keyDir, func(ctx context.Context, c *Constraints) (interface{}, error) { - platform := runtime.GOOS + platform := "linux" if c != nil && c.Platform != nil { platform = c.Platform.OS } From 45b4617a88fe1b7811b310dedf0c8e68a8d6a9d8 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Tue, 27 Jun 2023 10:41:42 +0300 Subject: [PATCH 09/12] Remove Architecture Signed-off-by: Gabriel Adrian Samfira --- client/llb/state.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/llb/state.go b/client/llb/state.go index b4e2c3548..0b27ec352 100644 --- a/client/llb/state.go +++ b/client/llb/state.go @@ -302,8 +302,7 @@ func (s State) File(a *FileAction, opts ...ConstraintsOpt) State { // No platform was set by constraint options. Set the current OS and arch. if c.Platform == nil { c.Platform = &ocispecs.Platform{ - OS: "linux", - Architecture: "amd64", + OS: "linux", } } return s.WithOutput(NewFileOp(s, a, c).Output()) From 805b993c086b68fb936ec89a6284b92e059b6c66 Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Tue, 27 Jun 2023 12:53:02 -0700 Subject: [PATCH 10/12] Revert most changes to client/llb Signed-off-by: Gabriel Adrian Samfira --- client/llb/fileop.go | 70 ++++++++++++++++++++++---------------------- client/llb/meta.go | 14 ++++----- client/llb/state.go | 6 ---- 3 files changed, 40 insertions(+), 50 deletions(-) diff --git a/client/llb/fileop.go b/client/llb/fileop.go index a22a38b73..7b725fbd3 100644 --- a/client/llb/fileop.go +++ b/client/llb/fileop.go @@ -4,13 +4,13 @@ import ( "context" _ "crypto/sha256" // for opencontainers/go-digest "os" + "path" "path/filepath" "strconv" "strings" "time" "github.com/moby/buildkit/solver/pb" - "github.com/moby/buildkit/util/system" digest "github.com/opencontainers/go-digest" "github.com/pkg/errors" ) @@ -55,7 +55,7 @@ type CopyInput interface { } type subAction interface { - toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platformOS string) (pb.IsFileAction, error) + toProtoAction(context.Context, string, pb.InputIndex) (pb.IsFileAction, error) } type capAdder interface { @@ -146,6 +146,7 @@ func Mkdir(p string, m os.FileMode, opt ...MkdirOption) *FileAction { for _, o := range opt { o.SetMkdirOption(&mi) } + return &FileAction{ action: &fileActionMkdir{ file: p, @@ -161,14 +162,10 @@ type fileActionMkdir struct { info MkdirInfo } -func (a *fileActionMkdir) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platformOS string) (pb.IsFileAction, error) { - normalizedPath, err := system.NormalizePath(parent, a.file, platformOS, false) - if err != nil { - return nil, errors.Wrap(err, "normalizing path") - } +func (a *fileActionMkdir) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) { return &pb.FileAction_Mkdir{ Mkdir: &pb.FileActionMkDir{ - Path: normalizedPath, + Path: normalizePath(parent, a.file, false), Mode: int32(a.mode & 0777), MakeParents: a.info.MakeParents, Owner: a.info.ChownOpt.marshal(base), @@ -339,14 +336,10 @@ type fileActionMkfile struct { info MkfileInfo } -func (a *fileActionMkfile) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platformOS string) (pb.IsFileAction, error) { - normalizedPath, err := system.NormalizePath(parent, a.file, platformOS, false) - if err != nil { - return nil, errors.Wrap(err, "normalizing path") - } +func (a *fileActionMkfile) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) { return &pb.FileAction_Mkfile{ Mkfile: &pb.FileActionMkFile{ - Path: normalizedPath, + Path: normalizePath(parent, a.file, false), Mode: int32(a.mode & 0777), Data: a.dt, Owner: a.info.ChownOpt.marshal(base), @@ -411,14 +404,10 @@ type fileActionRm struct { info RmInfo } -func (a *fileActionRm) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platformOS string) (pb.IsFileAction, error) { - normalizedPath, err := system.NormalizePath(parent, a.file, platformOS, false) - if err != nil { - return nil, errors.Wrap(err, "normalizing path") - } +func (a *fileActionRm) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) { return &pb.FileAction_Rm{ Rm: &pb.FileActionRm{ - Path: normalizedPath, + Path: normalizePath(parent, a.file, false), AllowNotFound: a.info.AllowNotFound, AllowWildcard: a.info.AllowWildcard, }, @@ -504,18 +493,14 @@ type fileActionCopy struct { info CopyInfo } -func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base pb.InputIndex, platformOS string) (pb.IsFileAction, error) { - src, err := a.sourcePath(ctx, platformOS) +func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base pb.InputIndex) (pb.IsFileAction, error) { + src, err := a.sourcePath(ctx) if err != nil { return nil, err } - normalizedPath, err := system.NormalizePath(parent, a.dest, platformOS, false) - if err != nil { - return nil, errors.Wrap(err, "normalizing path") - } c := &pb.FileActionCopy{ Src: src, - Dest: normalizedPath, + Dest: normalizePath(parent, a.dest, true), Owner: a.info.ChownOpt.marshal(base), IncludePatterns: a.info.IncludePatterns, ExcludePatterns: a.info.ExcludePatterns, @@ -537,13 +522,13 @@ func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base }, nil } -func (a *fileActionCopy) sourcePath(ctx context.Context, platform string) (string, error) { +func (a *fileActionCopy) sourcePath(ctx context.Context) (string, error) { // filepath.Clean() also does a filepath.FromSlash(). Explicitly convert back to UNIX path // separators. p := filepath.ToSlash(filepath.Clean(a.src)) dir := "/" var err error - if !system.IsAbs(p, platform) { + if !path.IsAbs(p) { if a.state != nil { dir, err = a.state.GetDir(ctx) } else if a.fas != nil { @@ -553,11 +538,7 @@ func (a *fileActionCopy) sourcePath(ctx context.Context, platform string) (strin return "", err } } - p, err = system.NormalizePath(dir, p, platform, false) - if err != nil { - return "", errors.Wrap(err, "normalizing source path") - } - return p, nil + return path.Join(dir, p), nil } func (a *fileActionCopy) addCaps(f *FileOp) { @@ -772,7 +753,7 @@ func (f *FileOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, [] } } - action, err := st.action.toProtoAction(ctx, parent, st.base, f.constraints.Platform.OS) + action, err := st.action.toProtoAction(ctx, parent, st.base) if err != nil { return "", nil, nil, nil, err } @@ -793,6 +774,25 @@ func (f *FileOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, [] return f.Load() } +func normalizePath(parent, p string, keepSlash bool) string { + origPath := p + p = path.Clean(p) + if !path.IsAbs(p) { + p = path.Join("/", parent, p) + } + if keepSlash { + if strings.HasSuffix(origPath, "/") && !strings.HasSuffix(p, "/") { + p += "/" + } else if strings.HasSuffix(origPath, "/.") { + if p != "/" { + p += "/" + } + p += "." + } + } + return p +} + func (f *FileOp) Output() Output { return f.output } diff --git a/client/llb/meta.go b/client/llb/meta.go index ead4f1b16..ab1021bd6 100644 --- a/client/llb/meta.go +++ b/client/llb/meta.go @@ -4,11 +4,11 @@ import ( "context" "fmt" "net" + "path" "github.com/containerd/containerd/platforms" "github.com/google/shlex" "github.com/moby/buildkit/solver/pb" - "github.com/moby/buildkit/util/system" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" "github.com/pkg/errors" ) @@ -76,19 +76,15 @@ func dirf(value string, replace bool, v ...interface{}) StateOption { } return func(s State) State { return s.withValue(keyDir, func(ctx context.Context, c *Constraints) (interface{}, error) { - platform := "linux" - if c != nil && c.Platform != nil { - platform = c.Platform.OS - } - if !system.IsAbs(value, platform) { + if !path.IsAbs(value) { prev, err := getDir(s)(ctx, c) if err != nil { return nil, errors.Wrap(err, "getting dir from state") } - value, err = system.NormalizePath(prev, value, platform, false) - if err != nil { - return nil, errors.Wrap(err, "normalizing path") + if prev == "" { + prev = "/" } + value = path.Join(prev, value) } return value, nil }) diff --git a/client/llb/state.go b/client/llb/state.go index 0b27ec352..f15fad87a 100644 --- a/client/llb/state.go +++ b/client/llb/state.go @@ -299,12 +299,6 @@ func (s State) File(a *FileAction, opts ...ConstraintsOpt) State { o.SetConstraintsOption(&c) } - // No platform was set by constraint options. Set the current OS and arch. - if c.Platform == nil { - c.Platform = &ocispecs.Platform{ - OS: "linux", - } - } return s.WithOutput(NewFileOp(s, a, c).Output()) } From 69f2c06bb87c260b4c9215c11c9c78b57fdc387b Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Mon, 3 Jul 2023 14:07:39 +0000 Subject: [PATCH 11/12] Use system.ToSlash() instead of filepath.ToSlash() When running on Linux, filepath.ToSlash() is not suitable to convert paths containing Windows path delimiter to Unix delimiters. To enable this, we export system.ToSlash() that takes a platform flag, based on which we use the proper file path separator, regardless of the platform we run on. Signed-off-by: Gabriel Adrian Samfira --- frontend/dockerfile/dockerfile2llb/convert.go | 3 +-- util/system/path.go | 20 +++++++++---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 37f9c258f..70c3db095 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -1003,7 +1003,7 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo d.image.Config.WorkingDir = wd // From this point forward, we can use UNIX style paths. - wd = filepath.ToSlash(wd) + wd = system.ToSlash(wd, d.platform.OS) d.state = d.state.Dir(wd) if commit { @@ -1421,7 +1421,6 @@ func pathRelativeToWorkingDir(s llb.State, p string, platform ocispecs.Platform) if len(p) == 0 { return dir, nil } - p, err = system.CheckSystemDriveAndRemoveDriveLetter(p, platform.OS) if err != nil { return "", errors.Wrap(err, "remving drive letter") diff --git a/util/system/path.go b/util/system/path.go index 4466a4376..d6bf51f35 100644 --- 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,10 +82,10 @@ 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 { +func ToSlash(inputPath, inputOS string) string { separator := "/" if inputOS == "windows" { separator = "\\" @@ -93,7 +93,7 @@ func toSlash(inputPath, inputOS string) string { return strings.Replace(inputPath, separator, "/", -1) } -func fromSlash(inputPath, inputOS string) string { +func FromSlash(inputPath, inputOS string) string { separator := "/" if inputOS == "windows" { separator = "\\" @@ -119,7 +119,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 +142,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 +189,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 +221,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 } From f1657ecc1451cca4d749d20ccf0193a935c2523a Mon Sep 17 00:00:00 2001 From: Gabriel Adrian Samfira Date: Mon, 10 Jul 2023 22:51:33 +0300 Subject: [PATCH 12/12] Fix various nits Signed-off-by: Gabriel Adrian Samfira --- client/llb/fileop.go | 5 +---- frontend/dockerfile/dockerfile2llb/convert.go | 6 +++--- solver/llbsolver/file/backend.go | 5 +++-- util/system/path.go | 7 +++---- 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/client/llb/fileop.go b/client/llb/fileop.go index 7b725fbd3..7fc445c4c 100644 --- a/client/llb/fileop.go +++ b/client/llb/fileop.go @@ -5,7 +5,6 @@ import ( _ "crypto/sha256" // for opencontainers/go-digest "os" "path" - "path/filepath" "strconv" "strings" "time" @@ -523,9 +522,7 @@ func (a *fileActionCopy) toProtoAction(ctx context.Context, parent string, base } func (a *fileActionCopy) sourcePath(ctx context.Context) (string, error) { - // filepath.Clean() also does a filepath.FromSlash(). Explicitly convert back to UNIX path - // separators. - p := filepath.ToSlash(filepath.Clean(a.src)) + p := path.Clean(a.src) dir := "/" var err error if !path.IsAbs(p) { diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 70c3db095..0c5c46cdd 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -1185,7 +1185,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { }}, copyOpt...) if a == nil { - a = llb.Copy(st, filepath.ToSlash(f), dest, opts...) + a = llb.Copy(st, system.ToSlash(f, d.platform.OS), dest, opts...) } else { a = a.Copy(st, filepath.ToSlash(f), dest, opts...) } @@ -1423,11 +1423,11 @@ func pathRelativeToWorkingDir(s llb.State, p string, platform ocispecs.Platform) } p, err = system.CheckSystemDriveAndRemoveDriveLetter(p, platform.OS) if err != nil { - return "", errors.Wrap(err, "remving drive letter") + return "", errors.Wrap(err, "removing drive letter") } if system.IsAbs(p, platform.OS) { - return p, nil + return system.NormalizePath("/", p, platform.OS, false) } return system.NormalizePath(dir, p, platform.OS, false) } diff --git a/solver/llbsolver/file/backend.go b/solver/llbsolver/file/backend.go index 825341655..6212066cd 100644 --- a/solver/llbsolver/file/backend.go +++ b/solver/llbsolver/file/backend.go @@ -352,13 +352,14 @@ func cleanPath(s string) (string, error) { if err != nil { return "", errors.Wrap(err, "removing drive letter") } + s = filepath.FromSlash(s) s2 := filepath.Join("/", s) - if strings.HasSuffix(filepath.FromSlash(s), string(filepath.Separator)+".") { + if strings.HasSuffix(s, string(filepath.Separator)+".") { if s2 != string(filepath.Separator) { s2 += string(filepath.Separator) } s2 += "." - } else if strings.HasSuffix(filepath.FromSlash(s), string(filepath.Separator)) && s2 != string(filepath.Separator) { + } else if strings.HasSuffix(s, string(filepath.Separator)) && s2 != string(filepath.Separator) { s2 += string(filepath.Separator) } return s2, nil diff --git a/util/system/path.go b/util/system/path.go index d6bf51f35..94f9a826f 100644 --- a/util/system/path.go +++ b/util/system/path.go @@ -86,11 +86,10 @@ func NormalizePath(parent, newPath, inputOS string, keepSlash bool) (string, err } func ToSlash(inputPath, inputOS string) string { - separator := "/" - if inputOS == "windows" { - separator = "\\" + if inputOS != "windows" { + return inputPath } - return strings.Replace(inputPath, separator, "/", -1) + return strings.Replace(inputPath, "\\", "/", -1) } func FromSlash(inputPath, inputOS string) string {