Merge pull request #44275 from thaJeztah/move_pkg_system_funcs

pkg/system: move some functions to a new home

builder/dockerfile/dispatchers_test.go

@@ -14,7 +14,7 @@ import (
 	"github.com/docker/docker/api/types/strslice"
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/image"
-	"github.com/docker/docker/pkg/system"
+	"github.com/docker/docker/oci"
 	"github.com/docker/go-connections/nat"
 	"github.com/moby/buildkit/frontend/dockerfile/instructions"
 	"github.com/moby/buildkit/frontend/dockerfile/parser"
@@ -128,7 +128,8 @@ func TestFromScratch(t *testing.T) {
 	assert.NilError(t, err)
 	assert.Check(t, sb.state.hasFromImage())
 	assert.Check(t, is.Equal("", sb.state.imageID))
-	expected := "PATH=" + system.DefaultPathEnv(runtime.GOOS)
+	// TODO(thaJeztah): use github.com/moby/buildkit/util/system.DefaultPathEnv() once https://github.com/moby/buildkit/pull/3158 is resolved.
+	expected := "PATH=" + oci.DefaultPathEnv(runtime.GOOS)
 	assert.Check(t, is.DeepEqual([]string{expected}, sb.state.runConfig.Env))
 }
 

builder/dockerfile/evaluator.go

@@ -28,6 +28,7 @@ import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/errdefs"
+	"github.com/docker/docker/oci"
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/runconfig/opts"
 	"github.com/moby/buildkit/frontend/dockerfile/instructions"
@@ -236,7 +237,8 @@ func (s *dispatchState) beginStage(stageName string, image builder.Image) error
 // Add the default PATH to runConfig.ENV if one exists for the operating system and there
 // is no PATH set. Note that Windows containers on Windows won't have one as it's set by HCS
 func (s *dispatchState) setDefaultPath() {
-	defaultPath := system.DefaultPathEnv(s.operatingSystem)
+	// TODO(thaJeztah): use github.com/moby/buildkit/util/system.DefaultPathEnv() once https://github.com/moby/buildkit/pull/3158 is resolved.
+	defaultPath := oci.DefaultPathEnv(s.operatingSystem)
 	if defaultPath == "" {
 		return
 	}

container/archive_windows.go

@@ -6,7 +6,6 @@ import (
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/pkg/archive"
-	"github.com/docker/docker/pkg/system"
 	"github.com/pkg/errors"
 )
 
@@ -18,8 +17,8 @@ func (container *Container) ResolvePath(path string) (resolvedPath, absPath stri
 	if container.BaseFS == "" {
 		return "", "", errors.New("ResolvePath: BaseFS of container " + container.ID + " is unexpectedly empty")
 	}
-	// Check if a drive letter supplied, it must be the system drive.
-	path, err = system.CheckSystemDriveAndRemoveDriveLetter(path)
+	// Check if a drive letter supplied, it must be the system drive. No-op except on Windows
+	path, err = archive.CheckSystemDriveAndRemoveDriveLetter(path)
 	if err != nil {
 		return "", "", err
 	}

container/container.go

@@ -28,10 +28,10 @@ import (
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
 	libcontainerdtypes "github.com/docker/docker/libcontainerd/types"
+	"github.com/docker/docker/oci"
 	"github.com/docker/docker/pkg/containerfs"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/ioutils"
-	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/restartmanager"
 	"github.com/docker/docker/volume"
 	volumemounts "github.com/docker/docker/volume/mounts"
@@ -737,7 +737,7 @@ func (container *Container) CreateDaemonEnvironment(tty bool, linkedEnv []string
 
 	env := make([]string, 0, envSize)
 	if runtime.GOOS != "windows" {
-		env = append(env, "PATH="+system.DefaultPathEnv(ctrOS))
+		env = append(env, "PATH="+oci.DefaultPathEnv(ctrOS))
 		env = append(env, "HOSTNAME="+container.Config.Hostname)
 		if tty {
 			env = append(env, "TERM=xterm")

daemon/archive_windows.go

@@ -14,7 +14,6 @@ import (
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/chrootarchive"
 	"github.com/docker/docker/pkg/ioutils"
-	"github.com/docker/docker/pkg/system"
 )
 
 // containerStatPath stats the filesystem resource at the specified path in this
@@ -172,7 +171,7 @@ func (daemon *Daemon) containerExtractToDir(container *container.Container, path
 	path = filepath.FromSlash(path)
 
 	// Check if a drive letter supplied, it must be the system drive. No-op except on Windows
-	path, err = system.CheckSystemDriveAndRemoveDriveLetter(path)
+	path, err = archive.CheckSystemDriveAndRemoveDriveLetter(path)
 	if err != nil {
 		return err
 	}

integration-cli/docker_cli_build_test.go

@@ -19,7 +19,6 @@ import (
 	"github.com/docker/docker/integration-cli/cli"
 	"github.com/docker/docker/integration-cli/cli/build"
 	"github.com/docker/docker/pkg/archive"
-	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/testutil"
 	"github.com/docker/docker/testutil/fakecontext"
 	"github.com/docker/docker/testutil/fakegit"
@@ -3583,7 +3582,7 @@ func (s *DockerCLIBuildSuite) TestBuildSymlinkBreakout(c *testing.T) {
 	assert.NilError(c, err)
 
 	// See https://github.com/moby/moby/pull/37770 for reason for next line.
-	tmpdir, err = system.GetLongPathName(tmpdir)
+	tmpdir, err = getLongPathName(tmpdir)
 	assert.NilError(c, err)
 
 	defer os.RemoveAll(tmpdir)

integration-cli/utils_unix_test.go

@@ -0,0 +1,11 @@
+//go:build !windows
+// +build !windows
+
+package main
+
+// getLongPathName converts Windows short pathnames to full pathnames.
+// For example C:\Users\ADMIN~1 --> C:\Users\Administrator.
+// It is a no-op on non-Windows platforms
+func getLongPathName(path string) (string, error) {
+	return path, nil
+}

integration-cli/utils_windows_test.go

@@ -0,0 +1,27 @@
+package main
+
+import "golang.org/x/sys/windows"
+
+// getLongPathName converts Windows short pathnames to full pathnames.
+// For example C:\Users\ADMIN~1 --> C:\Users\Administrator.
+// It is a no-op on non-Windows platforms
+func getLongPathName(path string) (string, error) {
+	// See https://groups.google.com/forum/#!topic/golang-dev/1tufzkruoTg
+	p, err := windows.UTF16FromString(path)
+	if err != nil {
+		return "", err
+	}
+	b := p // GetLongPathName says we can reuse buffer
+	n, err := windows.GetLongPathName(&p[0], &b[0], uint32(len(b)))
+	if err != nil {
+		return "", err
+	}
+	if n > uint32(len(b)) {
+		b = make([]uint16, n)
+		_, err = windows.GetLongPathName(&p[0], &b[0], uint32(len(b)))
+		if err != nil {
+			return "", err
+		}
+	}
+	return windows.UTF16ToString(b), nil
+}

oci/defaults.go

@@ -9,6 +9,24 @@ import (
 
 func iPtr(i int64) *int64 { return &i }
 
+const defaultUnixPathEnv = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+
+// DefaultPathEnv is unix style list of directories to search for
+// executables. Each directory is separated from the next by a colon
+// ':' character .
+// For Windows containers, an empty string is returned as the default
+// path will be set by the container, and Docker has no context of what the
+// default path should be.
+//
+// TODO(thaJeztah) align Windows default with BuildKit; see https://github.com/moby/buildkit/pull/1747
+// TODO(thaJeztah) use defaults from containerd (but align it with BuildKit; see https://github.com/moby/buildkit/pull/1747)
+func DefaultPathEnv(os string) string {
+	if os == "windows" {
+		return ""
+	}
+	return defaultUnixPathEnv
+}
+
 // DefaultSpec returns the default spec used by docker for the current Platform
 func DefaultSpec() specs.Spec {
 	if runtime.GOOS == "windows" {

pkg/system/path.go → pkg/archive/path.go

@@ -1,19 +1,4 @@
-package system // import "github.com/docker/docker/pkg/system"
-
-const defaultUnixPathEnv = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
-
-// DefaultPathEnv is unix style list of directories to search for
-// executables. Each directory is separated from the next by a colon
-// ':' character .
-// For Windows containers, an empty string is returned as the default
-// path will be set by the container, and Docker has no context of what the
-// default path should be.
-func DefaultPathEnv(os string) string {
-	if os == "windows" {
-		return ""
-	}
-	return defaultUnixPathEnv
-}
+package archive
 
 // CheckSystemDriveAndRemoveDriveLetter verifies that a path, if it includes a drive letter,
 // is the system drive.

pkg/archive/path_unix.go

@@ -0,0 +1,10 @@
+//go:build !windows
+// +build !windows
+
+package archive
+
+// checkSystemDriveAndRemoveDriveLetter is the non-Windows implementation
+// of CheckSystemDriveAndRemoveDriveLetter
+func checkSystemDriveAndRemoveDriveLetter(path string) (string, error) {
+	return path, nil
+}

pkg/archive/path_windows.go

@@ -0,0 +1,22 @@
+package archive
+
+import (
+	"fmt"
+	"path/filepath"
+	"strings"
+)
+
+// checkSystemDriveAndRemoveDriveLetter is the Windows implementation
+// of CheckSystemDriveAndRemoveDriveLetter
+func checkSystemDriveAndRemoveDriveLetter(path string) (string, error) {
+	if len(path) == 2 && string(path[1]) == ":" {
+		return "", fmt.Errorf("no relative path specified in %q", path)
+	}
+	if !filepath.IsAbs(path) || len(path) < 2 {
+		return filepath.FromSlash(path), nil
+	}
+	if string(path[1]) == ":" && !strings.EqualFold(string(path[0]), "c") {
+		return "", fmt.Errorf("the specified path is not on the system drive (C:)")
+	}
+	return filepath.FromSlash(path[2:]), nil
+}

pkg/system/path_windows_test.go → pkg/archive/path_windows_test.go

@@ -1,7 +1,4 @@
-//go:build windows
-// +build windows
-
-package system // import "github.com/docker/docker/pkg/system"
+package archive
 
 import (
 	"testing"
@@ -11,7 +8,7 @@ import (
 func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) {
 	// Fails if not C drive.
 	_, err := CheckSystemDriveAndRemoveDriveLetter(`d:\`)
-	if err == nil || err.Error() != "The specified path is not on the system drive (C:)" {
+	if err == nil || err.Error() != "the specified path is not on the system drive (C:)" {
 		t.Fatalf("Expected error for d:")
 	}
 
@@ -68,7 +65,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) {
 	if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:`); err == nil {
 		t.Fatalf("c: should fail")
 	}
-	if err.Error() != `No relative path specified in "c:"` {
+	if err.Error() != `no relative path specified in "c:"` {
 		t.Fatalf(path, err)
 	}
 
@@ -76,7 +73,7 @@ func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) {
 	if path, err = CheckSystemDriveAndRemoveDriveLetter(`d:`); err == nil {
 		t.Fatalf("c: should fail")
 	}
-	if err.Error() != `No relative path specified in "d:"` {
+	if err.Error() != `no relative path specified in "d:"` {
 		t.Fatalf(path, err)
 	}
 }

pkg/system/path_deprecated.go

@@ -0,0 +1,18 @@
+package system
+
+const defaultUnixPathEnv = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
+
+// DefaultPathEnv is unix style list of directories to search for
+// executables. Each directory is separated from the next by a colon
+// ':' character .
+// For Windows containers, an empty string is returned as the default
+// path will be set by the container, and Docker has no context of what the
+// default path should be.
+//
+// Deprecated: use oci.DefaultPathEnv
+func DefaultPathEnv(os string) string {
+	if os == "windows" {
+		return ""
+	}
+	return defaultUnixPathEnv
+}

pkg/system/path_unix.go

@@ -1,17 +0,0 @@
-//go:build !windows
-// +build !windows
-
-package system // import "github.com/docker/docker/pkg/system"
-
-// GetLongPathName converts Windows short pathnames to full pathnames.
-// For example C:\Users\ADMIN~1 --> C:\Users\Administrator.
-// It is a no-op on non-Windows platforms
-func GetLongPathName(path string) (string, error) {
-	return path, nil
-}
-
-// checkSystemDriveAndRemoveDriveLetter is the non-Windows implementation
-// of CheckSystemDriveAndRemoveDriveLetter
-func checkSystemDriveAndRemoveDriveLetter(path string) (string, error) {
-	return path, nil
-}

pkg/system/path_windows.go

@@ -1,48 +0,0 @@
-package system // import "github.com/docker/docker/pkg/system"
-
-import (
-	"fmt"
-	"path/filepath"
-	"strings"
-
-	"golang.org/x/sys/windows"
-)
-
-// GetLongPathName converts Windows short pathnames to full pathnames.
-// For example C:\Users\ADMIN~1 --> C:\Users\Administrator.
-// It is a no-op on non-Windows platforms
-func GetLongPathName(path string) (string, error) {
-	// See https://groups.google.com/forum/#!topic/golang-dev/1tufzkruoTg
-	p, err := windows.UTF16FromString(path)
-	if err != nil {
-		return "", err
-	}
-	b := p // GetLongPathName says we can reuse buffer
-	n, err := windows.GetLongPathName(&p[0], &b[0], uint32(len(b)))
-	if err != nil {
-		return "", err
-	}
-	if n > uint32(len(b)) {
-		b = make([]uint16, n)
-		_, err = windows.GetLongPathName(&p[0], &b[0], uint32(len(b)))
-		if err != nil {
-			return "", err
-		}
-	}
-	return windows.UTF16ToString(b), nil
-}
-
-// checkSystemDriveAndRemoveDriveLetter is the Windows implementation
-// of CheckSystemDriveAndRemoveDriveLetter
-func checkSystemDriveAndRemoveDriveLetter(path string) (string, error) {
-	if len(path) == 2 && string(path[1]) == ":" {
-		return "", fmt.Errorf("No relative path specified in %q", path)
-	}
-	if !filepath.IsAbs(path) || len(path) < 2 {
-		return filepath.FromSlash(path), nil
-	}
-	if string(path[1]) == ":" && !strings.EqualFold(string(path[0]), "c") {
-		return "", fmt.Errorf("The specified path is not on the system drive (C:)")
-	}
-	return filepath.FromSlash(path[2:]), nil
-}

plugin/v2/plugin_linux.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/oci"
-	"github.com/docker/docker/pkg/system"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
 )
@@ -114,7 +113,7 @@ func (p *Plugin) InitSpec(execRoot string) (*specs.Spec, error) {
 	}
 
 	envs := make([]string, 1, len(p.PluginObj.Settings.Env)+1)
-	envs[0] = "PATH=" + system.DefaultPathEnv(runtime.GOOS)
+	envs[0] = "PATH=" + oci.DefaultPathEnv(runtime.GOOS)
 	envs = append(envs, p.PluginObj.Settings.Env...)
 
 	args := append(p.PluginObj.Config.Entrypoint, p.PluginObj.Settings.Args...)