Add support for ENV of the form: ENV name=value ...

still supports the old form: ENV name value

Also, fixed an issue with the parser where it would ignore lines
at the end of the Dockerfile that ended with \

Closes #2333

Signed-off-by: Doug Davis <dug@us.ibm.com>

builder/dispatchers.go

@@ -31,21 +31,39 @@ func nullDispatch(b *Builder, args []string, attributes map[string]bool, origina
 // in the dockerfile available from the next statement on via ${foo}.
 //
 func env(b *Builder, args []string, attributes map[string]bool, original string) error {
-	if len(args) != 2 {
-		return fmt.Errorf("ENV accepts two arguments")
+	if len(args) == 0 {
+		return fmt.Errorf("ENV is missing arguments")
+	}
+
+	if len(args)%2 != 0 {
+		// should never get here, but just in case
+		return fmt.Errorf("Bad input to ENV, too many args")
 	}
 
-	fullEnv := fmt.Sprintf("%s=%s", args[0], args[1])
+	commitStr := "ENV"
 
-	for i, envVar := range b.Config.Env {
-		envParts := strings.SplitN(envVar, "=", 2)
-		if args[0] == envParts[0] {
-			b.Config.Env[i] = fullEnv
-			return b.commit("", b.Config.Cmd, fmt.Sprintf("ENV %s", fullEnv))
+	for j := 0; j < len(args); j++ {
+		// name  ==> args[j]
+		// value ==> args[j+1]
+		newVar := args[j] + "=" + args[j+1] + ""
+		commitStr += " " + newVar
+
+		gotOne := false
+		for i, envVar := range b.Config.Env {
+			envParts := strings.SplitN(envVar, "=", 2)
+			if envParts[0] == args[j] {
+				b.Config.Env[i] = newVar
+				gotOne = true
+				break
+			}
+		}
+		if !gotOne {
+			b.Config.Env = append(b.Config.Env, newVar)
 		}
+		j++
 	}
-	b.Config.Env = append(b.Config.Env, fullEnv)
-	return b.commit("", b.Config.Cmd, fmt.Sprintf("ENV %s", fullEnv))
+
+	return b.commit("", b.Config.Cmd, commitStr)
 }
 
 // MAINTAINER some text <maybe@an.email.address>

builder/parser/line_parsers.go

@@ -12,6 +12,7 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
+	"unicode"
 )
 
 var (
@@ -41,17 +42,139 @@ func parseSubCommand(rest string) (*Node, map[string]bool, error) {
 // parse environment like statements. Note that this does *not* handle
 // variable interpolation, which will be handled in the evaluator.
 func parseEnv(rest string) (*Node, map[string]bool, error) {
-	node := &Node{}
-	rootnode := node
-	strs := TOKEN_WHITESPACE.Split(rest, 2)
+	// This is kind of tricky because we need to support the old
+	// variant:   ENV name value
+	// as well as the new one:    ENV name=value ...
+	// The trigger to know which one is being used will be whether we hit
+	// a space or = first.  space ==> old, "=" ==> new
+
+	const (
+		inSpaces = iota // looking for start of a word
+		inWord
+		inQuote
+	)
+
+	words := []string{}
+	phase := inSpaces
+	word := ""
+	quote := '\000'
+	blankOK := false
+	var ch rune
+
+	for pos := 0; pos <= len(rest); pos++ {
+		if pos != len(rest) {
+			ch = rune(rest[pos])
+		}
+
+		if phase == inSpaces { // Looking for start of word
+			if pos == len(rest) { // end of input
+				break
+			}
+			if unicode.IsSpace(ch) { // skip spaces
+				continue
+			}
+			phase = inWord // found it, fall thru
+		}
+		if (phase == inWord || phase == inQuote) && (pos == len(rest)) {
+			if blankOK || len(word) > 0 {
+				words = append(words, word)
+			}
+			break
+		}
+		if phase == inWord {
+			if unicode.IsSpace(ch) {
+				phase = inSpaces
+				if blankOK || len(word) > 0 {
+					words = append(words, word)
+
+					// Look for = and if no there assume
+					// we're doing the old stuff and
+					// just read the rest of the line
+					if !strings.Contains(word, "=") {
+						word = strings.TrimSpace(rest[pos:])
+						words = append(words, word)
+						break
+					}
+				}
+				word = ""
+				blankOK = false
+				continue
+			}
+			if ch == '\'' || ch == '"' {
+				quote = ch
+				blankOK = true
+				phase = inQuote
+				continue
+			}
+			if ch == '\\' {
+				if pos+1 == len(rest) {
+					continue // just skip \ at end
+				}
+				pos++
+				ch = rune(rest[pos])
+			}
+			word += string(ch)
+			continue
+		}
+		if phase == inQuote {
+			if ch == quote {
+				phase = inWord
+				continue
+			}
+			if ch == '\\' {
+				if pos+1 == len(rest) {
+					phase = inWord
+					continue // just skip \ at end
+				}
+				pos++
+				ch = rune(rest[pos])
+			}
+			word += string(ch)
+		}
+	}
 
-	if len(strs) < 2 {
-		return nil, nil, fmt.Errorf("ENV must have two arguments")
+	if len(words) == 0 {
+		return nil, nil, fmt.Errorf("ENV must have some arguments")
 	}
 
-	node.Value = strs[0]
-	node.Next = &Node{}
-	node.Next.Value = strs[1]
+	// Old format (ENV name value)
+	var rootnode *Node
+
+	if !strings.Contains(words[0], "=") {
+		node := &Node{}
+		rootnode = node
+		strs := TOKEN_WHITESPACE.Split(rest, 2)
+
+		if len(strs) < 2 {
+			return nil, nil, fmt.Errorf("ENV must have two arguments")
+		}
+
+		node.Value = strs[0]
+		node.Next = &Node{}
+		node.Next.Value = strs[1]
+	} else {
+		var prevNode *Node
+		for i, word := range words {
+			if !strings.Contains(word, "=") {
+				return nil, nil, fmt.Errorf("Syntax error - can't find = in %q. Must be of the form: name=value", word)
+			}
+			parts := strings.SplitN(word, "=", 2)
+
+			name := &Node{}
+			value := &Node{}
+
+			name.Next = value
+			name.Value = parts[0]
+			value.Value = parts[1]
+
+			if i == 0 {
+				rootnode = name
+			} else {
+				prevNode.Next = name
+			}
+			prevNode = value
+		}
+	}
 
 	return rootnode, nil, nil
 }

builder/parser/parser.go

@@ -125,6 +125,12 @@ func Parse(rwc io.Reader) (*Node, error) {
 					break
 				}
 			}
+			if child == nil && line != "" {
+				line, child, err = parseLine(line)
+				if err != nil {
+					return nil, err
+				}
+			}
 		}
 
 		if child != nil {

builder/parser/testfiles-negative/env_equals_env/Dockerfile → builder/parser/testfiles-negative/env_no_value/Dockerfile

@@ -1,3 +1,3 @@
 FROM busybox
 
-ENV PATH=PATH
+ENV PATH

builder/parser/testfiles/env/Dockerfile

@@ -0,0 +1,15 @@
+FROM ubuntu
+ENV name value
+ENV name=value
+ENV name=value name2=value2
+ENV name="value value1"
+ENV name=value\ value2
+ENV name="value'quote space'value2"
+ENV name='value"double quote"value2'
+ENV name=value\ value2 name2=value2\ value3
+ENV name=value \
+    name1=value1 \
+    name2="value2a \
+           value2b" \
+    name3="value3a\n\"value3b\"" \
+	name4="value4a\\nvalue4b" \

builder/parser/testfiles/env/result

@@ -0,0 +1,10 @@
+(from "ubuntu")
+(env "name" "value")
+(env "name" "value")
+(env "name" "value" "name2" "value2")
+(env "name" "value value1")
+(env "name" "value value2")
+(env "name" "value'quote space'value2")
+(env "name" "value\"double quote\"value2")
+(env "name" "value value2" "name2" "value2 value3")
+(env "name" "value" "name1" "value1" "name2" "value2a            value2b" "name3" "value3an\"value3b\"" "name4" "value4a\\nvalue4b")

docs/sources/reference/builder.md

@@ -337,11 +337,36 @@ expose ports to the host, at runtime,
 ## ENV
 
     ENV <key> <value>
+    ENV <key>=<value> ...
 
 The `ENV` instruction sets the environment variable `<key>` to the value
 `<value>`. This value will be passed to all future `RUN` instructions. This is
 functionally equivalent to prefixing the command with `<key>=<value>`
 
+The `ENV` instruction has two forms. The first form, `ENV <key> <value>`,
+will set a single variable to a value. The entire string after the first
+space will be treated as the `<value>` - including characters such as 
+spaces and quotes.
+
+The second form, `ENV <key>=<value> ...`, allows for multiple variables to 
+be set at one time. Notice that the second form uses the equals sign (=) 
+in the syntax, while the first form does not. Like command line parsing, 
+quotes and backslashes can be used to include spaces within values.
+
+For example:
+
+    ENV myName="John Doe" myDog=Rex\ The\ Dog \
+        myCat=fluffy
+
+and
+
+    ENV myName John Doe
+    ENV myDog Rex The Dog
+    ENV myCat fluffy
+
+will yield the same net results in the final container, but the first form 
+does it all in one layer.
+
 The environment variables set using `ENV` will persist when a container is run
 from the resulting image. You can view the values using `docker inspect`, and
 change them using `docker run --env <key>=<value>`.

integration-cli/docker_cli_build_test.go

@@ -2951,6 +2951,46 @@ RUN    [ "$(cat $TO)" = "hello" ]
 	logDone("build - environment variables usage")
 }
 
+func TestBuildEnvUsage2(t *testing.T) {
+	name := "testbuildenvusage2"
+	defer deleteImages(name)
+	dockerfile := `FROM busybox
+ENV    abc=def
+RUN    [ "$abc" = "def" ]
+ENV    def="hello world"
+RUN    [ "$def" = "hello world" ]
+ENV    def=hello\ world
+RUN    [ "$def" = "hello world" ]
+ENV    v1=abc v2="hi there"
+RUN    [ "$v1" = "abc" ]
+RUN    [ "$v2" = "hi there" ]
+ENV    v3='boogie nights' v4="with'quotes too"
+RUN    [ "$v3" = "boogie nights" ]
+RUN    [ "$v4" = "with'quotes too" ]
+ENV    abc=zzz FROM=hello/docker/world
+ENV    abc=zzz TO=/docker/world/hello
+ADD    $FROM $TO
+RUN    [ "$(cat $TO)" = "hello" ]
+ENV    abc "zzz"
+RUN    [ $abc = \"zzz\" ]
+ENV    abc 'yyy'
+RUN    [ $abc = \'yyy\' ]
+ENV    abc=
+RUN    [ "$abc" = "" ]
+`
+	ctx, err := fakeContext(dockerfile, map[string]string{
+		"hello/docker/world": "hello",
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = buildImageFromContext(name, ctx, true)
+	if err != nil {
+		t.Fatal(err)
+	}
+	logDone("build - environment variables usage2")
+}
+
 func TestBuildAddScript(t *testing.T) {
 	name := "testbuildaddscript"
 	defer deleteImages(name)