pkg/system: deprecate DefaultPathEnv, move to oci

This patch:

- Deprecates pkg/system.DefaultPathEnv
- Moves the implementation inside oci
- Adds TODOs to align the default in the Builder with the one used elsewhere

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

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/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")

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

@@ -1,20 +1,5 @@
 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
-}
-
 // CheckSystemDriveAndRemoveDriveLetter verifies that a path, if it includes a drive letter,
 // is the system drive.
 // On Linux: this is a no-op.

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
+}

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...)