Skip to content

Commit 9f3e5ee

Browse files
thaJeztahthaJeztah
committed
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 <[email protected]>
1 parent 73261bb commit 9f3e5ee

7 files changed

Lines changed: 45 additions & 22 deletions

File tree

‎builder/dockerfile/dispatchers_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414
"github.com/docker/docker/api/types/strslice"
1515
"github.com/docker/docker/builder"
1616
"github.com/docker/docker/image"
17-
"github.com/docker/docker/pkg/system"
17+
"github.com/docker/docker/oci"
1818
"github.com/docker/go-connections/nat"
1919
"github.com/moby/buildkit/frontend/dockerfile/instructions"
2020
"github.com/moby/buildkit/frontend/dockerfile/parser"
@@ -128,7 +128,8 @@ func TestFromScratch(t *testing.T) {
128128
assert.NilError(t, err)
129129
assert.Check(t, sb.state.hasFromImage())
130130
assert.Check(t, is.Equal("", sb.state.imageID))
131-
expected := "PATH=" + system.DefaultPathEnv(runtime.GOOS)
131+
// TODO(thaJeztah): use github.com/moby/buildkit/util/system.DefaultPathEnv() once https://github.com/moby/buildkit/pull/3158 is resolved.
132+
expected := "PATH=" + oci.DefaultPathEnv(runtime.GOOS)
132133
assert.Check(t, is.DeepEqual([]string{expected}, sb.state.runConfig.Env))
133134
}
134135

‎builder/dockerfile/evaluator.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/docker/docker/api/types/container"
2929
"github.com/docker/docker/builder"
3030
"github.com/docker/docker/errdefs"
31+
"github.com/docker/docker/oci"
3132
"github.com/docker/docker/pkg/system"
3233
"github.com/docker/docker/runconfig/opts"
3334
"github.com/moby/buildkit/frontend/dockerfile/instructions"
@@ -236,7 +237,8 @@ func (s *dispatchState) beginStage(stageName string, image builder.Image) error
236237
// Add the default PATH to runConfig.ENV if one exists for the operating system and there
237238
// is no PATH set. Note that Windows containers on Windows won't have one as it's set by HCS
238239
func (s *dispatchState) setDefaultPath() {
239-
defaultPath := system.DefaultPathEnv(s.operatingSystem)
240+
// TODO(thaJeztah): use github.com/moby/buildkit/util/system.DefaultPathEnv() once https://github.com/moby/buildkit/pull/3158 is resolved.
241+
defaultPath := oci.DefaultPathEnv(s.operatingSystem)
240242
if defaultPath == "" {
241243
return
242244
}

‎container/container.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ import (
2828
"github.com/docker/docker/image"
2929
"github.com/docker/docker/layer"
3030
libcontainerdtypes "github.com/docker/docker/libcontainerd/types"
31+
"github.com/docker/docker/oci"
3132
"github.com/docker/docker/pkg/containerfs"
3233
"github.com/docker/docker/pkg/idtools"
3334
"github.com/docker/docker/pkg/ioutils"
34-
"github.com/docker/docker/pkg/system"
3535
"github.com/docker/docker/restartmanager"
3636
"github.com/docker/docker/volume"
3737
volumemounts "github.com/docker/docker/volume/mounts"
@@ -737,7 +737,7 @@ func (container *Container) CreateDaemonEnvironment(tty bool, linkedEnv []string
737737

738738
env := make([]string, 0, envSize)
739739
if runtime.GOOS != "windows" {
740-
env = append(env, "PATH="+system.DefaultPathEnv(ctrOS))
740+
env = append(env, "PATH="+oci.DefaultPathEnv(ctrOS))
741741
env = append(env, "HOSTNAME="+container.Config.Hostname)
742742
if tty {
743743
env = append(env, "TERM=xterm")

‎oci/defaults.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,24 @@ import (
99

1010
func iPtr(i int64) *int64 { return &i }
1111

12+
const defaultUnixPathEnv = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
13+
14+
// DefaultPathEnv is unix style list of directories to search for
15+
// executables. Each directory is separated from the next by a colon
16+
// ':' character .
17+
// For Windows containers, an empty string is returned as the default
18+
// path will be set by the container, and Docker has no context of what the
19+
// default path should be.
20+
//
21+
// TODO(thaJeztah) align Windows default with BuildKit; see https://github.com/moby/buildkit/pull/1747
22+
// TODO(thaJeztah) use defaults from containerd (but align it with BuildKit; see https://github.com/moby/buildkit/pull/1747)
23+
func DefaultPathEnv(os string) string {
24+
if os == "windows" {
25+
return ""
26+
}
27+
return defaultUnixPathEnv
28+
}
29+
1230
// DefaultSpec returns the default spec used by docker for the current Platform
1331
func DefaultSpec() specs.Spec {
1432
if runtime.GOOS == "windows" {

‎pkg/system/path.go‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,5 @@
11
package system // import "github.com/docker/docker/pkg/system"
22

3-
const defaultUnixPathEnv = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
4-
5-
// DefaultPathEnv is unix style list of directories to search for
6-
// executables. Each directory is separated from the next by a colon
7-
// ':' character .
8-
// For Windows containers, an empty string is returned as the default
9-
// path will be set by the container, and Docker has no context of what the
10-
// default path should be.
11-
func DefaultPathEnv(os string) string {
12-
if os == "windows" {
13-
return ""
14-
}
15-
return defaultUnixPathEnv
16-
}
17-
183
// CheckSystemDriveAndRemoveDriveLetter verifies that a path, if it includes a drive letter,
194
// is the system drive.
205
// On Linux: this is a no-op.

‎pkg/system/path_deprecated.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package system
2+
3+
const defaultUnixPathEnv = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
4+
5+
// DefaultPathEnv is unix style list of directories to search for
6+
// executables. Each directory is separated from the next by a colon
7+
// ':' character .
8+
// For Windows containers, an empty string is returned as the default
9+
// path will be set by the container, and Docker has no context of what the
10+
// default path should be.
11+
//
12+
// Deprecated: use oci.DefaultPathEnv
13+
func DefaultPathEnv(os string) string {
14+
if os == "windows" {
15+
return ""
16+
}
17+
return defaultUnixPathEnv
18+
}

‎plugin/v2/plugin_linux.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88

99
"github.com/docker/docker/api/types"
1010
"github.com/docker/docker/oci"
11-
"github.com/docker/docker/pkg/system"
1211
specs "github.com/opencontainers/runtime-spec/specs-go"
1312
"github.com/pkg/errors"
1413
)
@@ -114,7 +113,7 @@ func (p *Plugin) InitSpec(execRoot string) (*specs.Spec, error) {
114113
}
115114

116115
envs := make([]string, 1, len(p.PluginObj.Settings.Env)+1)
117-
envs[0] = "PATH=" + system.DefaultPathEnv(runtime.GOOS)
116+
envs[0] = "PATH=" + oci.DefaultPathEnv(runtime.GOOS)
118117
envs = append(envs, p.PluginObj.Settings.Env...)
119118

120119
args := append(p.PluginObj.Config.Entrypoint, p.PluginObj.Settings.Args...)

0 commit comments

Comments
 (0)