From fe7b4d8fcd737b9162540149acc427d51d1ddfb9 Mon Sep 17 00:00:00 2001 From: John Howard Date: Tue, 20 Jun 2017 15:08:58 -0700 Subject: [PATCH] LCOW: Set correct default shell for platform in builder Signed-off-by: John Howard --- builder/dockerfile/builder_unix.go | 4 +++- builder/dockerfile/builder_windows.go | 7 ++++++- builder/dockerfile/dispatchers.go | 10 +++++----- builder/dockerfile/dispatchers_test.go | 2 +- builder/dockerfile/internals.go | 16 ++++++++-------- builder/dockerfile/internals_test.go | 5 +++-- 6 files changed, 26 insertions(+), 18 deletions(-) diff --git a/builder/dockerfile/builder_unix.go b/builder/dockerfile/builder_unix.go index 76a7ce74f9..5ea63da824 100644 --- a/builder/dockerfile/builder_unix.go +++ b/builder/dockerfile/builder_unix.go @@ -2,4 +2,6 @@ package dockerfile -var defaultShell = []string{"/bin/sh", "-c"} +func defaultShellForPlatform(platform string) []string { + return []string{"/bin/sh", "-c"} +} diff --git a/builder/dockerfile/builder_windows.go b/builder/dockerfile/builder_windows.go index 37e9fbcf4b..7bfef3238b 100644 --- a/builder/dockerfile/builder_windows.go +++ b/builder/dockerfile/builder_windows.go @@ -1,3 +1,8 @@ package dockerfile -var defaultShell = []string{"cmd", "/S", "/C"} +func defaultShellForPlatform(platform string) []string { + if platform == "linux" { + return []string{"/bin/sh", "-c"} + } + return []string{"cmd", "/S", "/C"} +} diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 0da362321a..9b1c52e7ef 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -399,7 +399,7 @@ func workdir(req dispatchRequest) error { } comment := "WORKDIR " + runConfig.WorkingDir - runConfigWithCommentCmd := copyRunConfig(runConfig, withCmdCommentString(comment)) + runConfigWithCommentCmd := copyRunConfig(runConfig, withCmdCommentString(comment, req.builder.platform)) containerID, err := req.builder.probeAndCreate(req.state, runConfigWithCommentCmd) if err != nil || containerID == "" { return err @@ -417,7 +417,7 @@ func workdir(req dispatchRequest) error { // the current SHELL which defaults to 'sh -c' under linux or 'cmd /S /C' under // Windows, in the event there is only one argument The difference in processing: // -// RUN echo hi # sh -c echo hi (Linux) +// RUN echo hi # sh -c echo hi (Linux and LCOW) // RUN echo hi # cmd /S /C echo hi (Windows) // RUN [ "echo", "hi" ] # echo hi // @@ -433,7 +433,7 @@ func run(req dispatchRequest) error { stateRunConfig := req.state.runConfig args := handleJSONArgs(req.args, req.attributes) if !req.attributes["json"] { - args = append(getShell(stateRunConfig), args...) + args = append(getShell(stateRunConfig, req.builder.platform), args...) } cmdFromArgs := strslice.StrSlice(args) buildArgs := req.builder.buildArgs.FilterAllowed(stateRunConfig.Env) @@ -518,7 +518,7 @@ func cmd(req dispatchRequest) error { runConfig := req.state.runConfig cmdSlice := handleJSONArgs(req.args, req.attributes) if !req.attributes["json"] { - cmdSlice = append(getShell(runConfig), cmdSlice...) + cmdSlice = append(getShell(runConfig, req.builder.platform), cmdSlice...) } runConfig.Cmd = strslice.StrSlice(cmdSlice) @@ -670,7 +670,7 @@ func entrypoint(req dispatchRequest) error { runConfig.Entrypoint = nil default: // ENTRYPOINT echo hi - runConfig.Entrypoint = strslice.StrSlice(append(getShell(runConfig), parsed[0])) + runConfig.Entrypoint = strslice.StrSlice(append(getShell(runConfig, req.builder.platform), parsed[0])) } // when setting the entrypoint if a CMD was not explicitly set then diff --git a/builder/dockerfile/dispatchers_test.go b/builder/dockerfile/dispatchers_test.go index 66cd1fb8fd..b3672fce1b 100644 --- a/builder/dockerfile/dispatchers_test.go +++ b/builder/dockerfile/dispatchers_test.go @@ -474,7 +474,7 @@ func TestRunWithBuildArgs(t *testing.T) { runConfig := &container.Config{} origCmd := strslice.StrSlice([]string{"cmd", "in", "from", "image"}) - cmdWithShell := strslice.StrSlice(append(getShell(runConfig), "echo foo")) + cmdWithShell := strslice.StrSlice(append(getShell(runConfig, runtime.GOOS), "echo foo")) envVars := []string{"|1", "one=two"} cachedCmd := strslice.StrSlice(append(envVars, cmdWithShell...)) diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 42d78ec96c..b781810c33 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -25,7 +25,7 @@ func (b *Builder) commit(dispatchState *dispatchState, comment string) error { return errors.New("Please provide a source image with `from` prior to commit") } - runConfigWithCommentCmd := copyRunConfig(dispatchState.runConfig, withCmdComment(comment)) + runConfigWithCommentCmd := copyRunConfig(dispatchState.runConfig, withCmdComment(comment, b.platform)) hit, err := b.probeCache(dispatchState, runConfigWithCommentCmd) if err != nil || hit { return err @@ -110,7 +110,7 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error // TODO: should this have been using origPaths instead of srcHash in the comment? runConfigWithCommentCmd := copyRunConfig( state.runConfig, - withCmdCommentString(fmt.Sprintf("%s %s in %s ", inst.cmdName, srcHash, inst.dest))) + withCmdCommentString(fmt.Sprintf("%s %s in %s ", inst.cmdName, srcHash, inst.dest), b.platform)) hit, err := b.probeCache(state, runConfigWithCommentCmd) if err != nil || hit { return err @@ -190,9 +190,9 @@ func withCmd(cmd []string) runConfigModifier { // withCmdComment sets Cmd to a nop comment string. See withCmdCommentString for // why there are two almost identical versions of this. -func withCmdComment(comment string) runConfigModifier { +func withCmdComment(comment string, platform string) runConfigModifier { return func(runConfig *container.Config) { - runConfig.Cmd = append(getShell(runConfig), "#(nop) ", comment) + runConfig.Cmd = append(getShell(runConfig, platform), "#(nop) ", comment) } } @@ -200,9 +200,9 @@ func withCmdComment(comment string) runConfigModifier { // A few instructions (workdir, copy, add) used a nop comment that is a single arg // where as all the other instructions used a two arg comment string. This // function implements the single arg version. -func withCmdCommentString(comment string) runConfigModifier { +func withCmdCommentString(comment string, platform string) runConfigModifier { return func(runConfig *container.Config) { - runConfig.Cmd = append(getShell(runConfig), "#(nop) "+comment) + runConfig.Cmd = append(getShell(runConfig, platform), "#(nop) "+comment) } } @@ -229,9 +229,9 @@ func withEntrypointOverride(cmd []string, entrypoint []string) runConfigModifier // getShell is a helper function which gets the right shell for prefixing the // shell-form of RUN, ENTRYPOINT and CMD instructions -func getShell(c *container.Config) []string { +func getShell(c *container.Config, platform string) []string { if 0 == len(c.Shell) { - return append([]string{}, defaultShell[:]...) + return append([]string{}, defaultShellForPlatform(platform)[:]...) } return append([]string{}, c.Shell[:]...) } diff --git a/builder/dockerfile/internals_test.go b/builder/dockerfile/internals_test.go index 4cba2c9842..8073cc6713 100644 --- a/builder/dockerfile/internals_test.go +++ b/builder/dockerfile/internals_test.go @@ -2,6 +2,7 @@ package dockerfile import ( "fmt" + "runtime" "testing" "github.com/docker/docker/api/types" @@ -97,9 +98,9 @@ func TestCopyRunConfig(t *testing.T) { }, { doc: "Set the command to a comment", - modifiers: []runConfigModifier{withCmdComment("comment")}, + modifiers: []runConfigModifier{withCmdComment("comment", runtime.GOOS)}, expected: &container.Config{ - Cmd: append(defaultShell, "#(nop) ", "comment"), + Cmd: append(defaultShellForPlatform(runtime.GOOS), "#(nop) ", "comment"), Env: defaultEnv, }, },