builder: Fix handling of ENV references that reference themselves, plus tests.

Docker-DCO-1.1-Signed-off-by: Erik Hollensbe <github@hollensbe.org> (github: erikh)

builder/builder.go

@@ -2,7 +2,6 @@ package builder
 
 import (
 	"github.com/docker/docker/builder/evaluator"
-	"github.com/docker/docker/nat"
 	"github.com/docker/docker/runconfig"
 )
 
@@ -10,25 +9,9 @@ import (
 func NewBuilder(opts *evaluator.BuildOpts) *evaluator.BuildFile {
 	return &evaluator.BuildFile{
 		Dockerfile:    nil,
-		Env:           evaluator.EnvMap{},
-		Config:        initRunConfig(),
+		Config:        &runconfig.Config{},
 		Options:       opts,
 		TmpContainers: evaluator.UniqueMap{},
 		TmpImages:     evaluator.UniqueMap{},
 	}
 }
-
-func initRunConfig() *runconfig.Config {
-	return &runconfig.Config{
-		PortSpecs: []string{},
-		// FIXME(erikh) this should be a type that lives in runconfig
-		ExposedPorts: map[nat.Port]struct{}{},
-		Env:          []string{},
-		Cmd:          []string{},
-
-		// FIXME(erikh) this should also be a type in runconfig
-		Volumes:    map[string]struct{}{},
-		Entrypoint: []string{"/bin/sh", "-c"},
-		OnBuild:    []string{},
-	}
-}

builder/evaluator/dispatchers.go

@@ -13,12 +13,12 @@ import (
 	"strings"
 
 	"github.com/docker/docker/nat"
+	"github.com/docker/docker/pkg/log"
 	"github.com/docker/docker/runconfig"
-	"github.com/docker/docker/utils"
 )
 
 // dispatch with no layer / parsing. This is effectively not a command.
-func nullDispatch(b *BuildFile, args []string) error {
+func nullDispatch(b *BuildFile, args []string, attributes map[string]bool) error {
 	return nil
 }
 
@@ -27,24 +27,28 @@ func nullDispatch(b *BuildFile, args []string) error {
 // Sets the environment variable foo to bar, also makes interpolation
 // in the dockerfile available from the next statement on via ${foo}.
 //
-func env(b *BuildFile, args []string) error {
+func env(b *BuildFile, args []string, attributes map[string]bool) error {
 	if len(args) != 2 {
 		return fmt.Errorf("ENV accepts two arguments")
 	}
 
-	// the duplication here is intended to ease the replaceEnv() call's env
-	// handling. This routine gets much shorter with the denormalization here.
-	key := args[0]
-	b.Env[key] = args[1]
-	b.Config.Env = append(b.Config.Env, strings.Join([]string{key, b.Env[key]}, "="))
+	fullEnv := fmt.Sprintf("%s=%s", args[0], args[1])
 
-	return b.commit("", b.Config.Cmd, fmt.Sprintf("ENV %s=%s", key, b.Env[key]))
+	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))
+		}
+	}
+	b.Config.Env = append(b.Config.Env, fullEnv)
+	return b.commit("", b.Config.Cmd, fmt.Sprintf("ENV %s", fullEnv))
 }
 
 // MAINTAINER some text <maybe@an.email.address>
 //
 // Sets the maintainer metadata.
-func maintainer(b *BuildFile, args []string) error {
+func maintainer(b *BuildFile, args []string, attributes map[string]bool) error {
 	if len(args) != 1 {
 		return fmt.Errorf("MAINTAINER requires only one argument")
 	}
@@ -58,7 +62,7 @@ func maintainer(b *BuildFile, args []string) error {
 // Add the file 'foo' to '/path'. Tarball and Remote URL (git, http) handling
 // exist here. If you do not wish to have this automatic handling, use COPY.
 //
-func add(b *BuildFile, args []string) error {
+func add(b *BuildFile, args []string, attributes map[string]bool) error {
 	if len(args) != 2 {
 		return fmt.Errorf("ADD requires two arguments")
 	}
@@ -70,7 +74,7 @@ func add(b *BuildFile, args []string) error {
 //
 // Same as 'ADD' but without the tar and remote url handling.
 //
-func dispatchCopy(b *BuildFile, args []string) error {
+func dispatchCopy(b *BuildFile, args []string, attributes map[string]bool) error {
 	if len(args) != 2 {
 		return fmt.Errorf("COPY requires two arguments")
 	}
@@ -82,7 +86,7 @@ func dispatchCopy(b *BuildFile, args []string) error {
 //
 // This sets the image the dockerfile will build on top of.
 //
-func from(b *BuildFile, args []string) error {
+func from(b *BuildFile, args []string, attributes map[string]bool) error {
 	if len(args) != 1 {
 		return fmt.Errorf("FROM requires one argument")
 	}
@@ -114,7 +118,7 @@ func from(b *BuildFile, args []string) error {
 // special cases. search for 'OnBuild' in internals.go for additional special
 // cases.
 //
-func onbuild(b *BuildFile, args []string) error {
+func onbuild(b *BuildFile, args []string, attributes map[string]bool) error {
 	triggerInstruction := strings.ToUpper(strings.TrimSpace(args[0]))
 	switch triggerInstruction {
 	case "ONBUILD":
@@ -133,7 +137,7 @@ func onbuild(b *BuildFile, args []string) error {
 //
 // Set the working directory for future RUN/CMD/etc statements.
 //
-func workdir(b *BuildFile, args []string) error {
+func workdir(b *BuildFile, args []string, attributes map[string]bool) error {
 	if len(args) != 1 {
 		return fmt.Errorf("WORKDIR requires exactly one argument")
 	}
@@ -161,10 +165,8 @@ func workdir(b *BuildFile, args []string) error {
 // RUN echo hi          # sh -c echo hi
 // RUN [ "echo", "hi" ] # echo hi
 //
-func run(b *BuildFile, args []string) error {
-	if len(args) == 1 { // literal string command, not an exec array
-		args = append([]string{"/bin/sh", "-c"}, args[0])
-	}
+func run(b *BuildFile, args []string, attributes map[string]bool) error {
+	args = handleJsonArgs(args, attributes)
 
 	if b.image == "" {
 		return fmt.Errorf("Please provide a source image with `from` prior to run")
@@ -182,7 +184,7 @@ func run(b *BuildFile, args []string) error {
 
 	defer func(cmd []string) { b.Config.Cmd = cmd }(cmd)
 
-	utils.Debugf("Command to be executed: %v", b.Config.Cmd)
+	log.Debugf("Command to be executed: %v", b.Config.Cmd)
 
 	hit, err := b.probeCache()
 	if err != nil {
@@ -196,6 +198,7 @@ func run(b *BuildFile, args []string) error {
 	if err != nil {
 		return err
 	}
+
 	// Ensure that we keep the container mounted until the commit
 	// to avoid unmounting and then mounting directly again
 	c.Mount()
@@ -217,12 +220,9 @@ func run(b *BuildFile, args []string) error {
 // Set the default command to run in the container (which may be empty).
 // Argument handling is the same as RUN.
 //
-func cmd(b *BuildFile, args []string) error {
-	if len(args) < 2 {
-		args = append([]string{"/bin/sh", "-c"}, args...)
-	}
+func cmd(b *BuildFile, args []string, attributes map[string]bool) error {
+	b.Config.Cmd = handleJsonArgs(args, attributes)
 
-	b.Config.Cmd = args
 	if err := b.commit("", b.Config.Cmd, fmt.Sprintf("CMD %v", cmd)); err != nil {
 		return err
 	}
@@ -239,14 +239,15 @@ func cmd(b *BuildFile, args []string) error {
 // Handles command processing similar to CMD and RUN, only b.Config.Entrypoint
 // is initialized at NewBuilder time instead of through argument parsing.
 //
-func entrypoint(b *BuildFile, args []string) error {
-	b.Config.Entrypoint = args
+func entrypoint(b *BuildFile, args []string, attributes map[string]bool) error {
+	b.Config.Entrypoint = handleJsonArgs(args, attributes)
 
 	// if there is no cmd in current Dockerfile - cleanup cmd
 	if !b.cmdSet {
 		b.Config.Cmd = nil
 	}
-	if err := b.commit("", b.Config.Cmd, fmt.Sprintf("ENTRYPOINT %v", entrypoint)); err != nil {
+
+	if err := b.commit("", b.Config.Cmd, fmt.Sprintf("ENTRYPOINT %v", b.Config.Entrypoint)); err != nil {
 		return err
 	}
 	return nil
@@ -257,7 +258,7 @@ func entrypoint(b *BuildFile, args []string) error {
 // Expose ports for links and port mappings. This all ends up in
 // b.Config.ExposedPorts for runconfig.
 //
-func expose(b *BuildFile, args []string) error {
+func expose(b *BuildFile, args []string, attributes map[string]bool) error {
 	portsTab := args
 
 	if b.Config.ExposedPorts == nil {
@@ -284,7 +285,7 @@ func expose(b *BuildFile, args []string) error {
 // Set the user to 'foo' for future commands and when running the
 // ENTRYPOINT/CMD at container run time.
 //
-func user(b *BuildFile, args []string) error {
+func user(b *BuildFile, args []string, attributes map[string]bool) error {
 	if len(args) != 1 {
 		return fmt.Errorf("USER requires exactly one argument")
 	}
@@ -298,7 +299,7 @@ func user(b *BuildFile, args []string) error {
 // Expose the volume /foo for use. Will also accept the JSON form, but either
 // way requires exactly one argument.
 //
-func volume(b *BuildFile, args []string) error {
+func volume(b *BuildFile, args []string, attributes map[string]bool) error {
 	if len(args) != 1 {
 		return fmt.Errorf("Volume cannot be empty")
 	}
@@ -318,6 +319,6 @@ func volume(b *BuildFile, args []string) error {
 }
 
 // INSERT is no longer accepted, but we still parse it.
-func insert(b *BuildFile, args []string) error {
+func insert(b *BuildFile, args []string, attributes map[string]bool) error {
 	return fmt.Errorf("INSERT has been deprecated. Please use ADD instead")
 }

builder/evaluator/evaluator.go

@@ -38,17 +38,16 @@ import (
 	"github.com/docker/docker/utils"
 )
 
-type EnvMap map[string]string
 type UniqueMap map[string]struct{}
 
 var (
 	ErrDockerfileEmpty = errors.New("Dockerfile cannot be empty")
 )
 
-var evaluateTable map[string]func(*BuildFile, []string) error
+var evaluateTable map[string]func(*BuildFile, []string, map[string]bool) error
 
 func init() {
-	evaluateTable = map[string]func(*BuildFile, []string) error{
+	evaluateTable = map[string]func(*BuildFile, []string, map[string]bool) error{
 		"env":            env,
 		"maintainer":     maintainer,
 		"add":            add,
@@ -71,7 +70,6 @@ func init() {
 // processing as it evaluates the parsing result.
 type BuildFile struct {
 	Dockerfile *parser.Node      // the syntax tree of the dockerfile
-	Env        EnvMap            // map of environment variables
 	Config     *runconfig.Config // runconfig for cmd, run, entrypoint etc.
 	Options    *BuildOpts        // see below
 
@@ -152,7 +150,9 @@ func (b *BuildFile) Run(context io.Reader) (string, error) {
 				b.clearTmp(b.TmpContainers)
 			}
 			return "", err
-		} else if b.Options.Remove {
+		}
+		fmt.Fprintf(b.Options.OutStream, " ---> %s\n", utils.TruncateID(b.image))
+		if b.Options.Remove {
 			b.clearTmp(b.TmpContainers)
 		}
 	}
@@ -181,25 +181,29 @@ func (b *BuildFile) Run(context io.Reader) (string, error) {
 // features.
 func (b *BuildFile) dispatch(stepN int, ast *parser.Node) error {
 	cmd := ast.Value
+	attrs := ast.Attributes
 	strs := []string{}
+	msg := fmt.Sprintf("Step %d : %s", stepN, strings.ToUpper(cmd))
 
 	if cmd == "onbuild" {
 		fmt.Fprintf(b.Options.OutStream, "%#v\n", ast.Next.Children[0].Value)
 		ast = ast.Next.Children[0]
-		strs = append(strs, ast.Value)
+		strs = append(strs, b.replaceEnv(ast.Value))
+		msg += " " + ast.Value
 	}
 
 	for ast.Next != nil {
 		ast = ast.Next
-		strs = append(strs, replaceEnv(b, ast.Value))
+		strs = append(strs, b.replaceEnv(ast.Value))
+		msg += " " + ast.Value
 	}
 
-	fmt.Fprintf(b.Options.OutStream, "Step %d : %s %s\n", stepN, strings.ToUpper(cmd), strings.Join(strs, " "))
+	fmt.Fprintf(b.Options.OutStream, "%s\n", msg)
 
 	// XXX yes, we skip any cmds that are not valid; the parser should have
 	// picked these out already.
 	if f, ok := evaluateTable[cmd]; ok {
-		return f(b, strs)
+		return f(b, strs, attrs)
 	}
 
 	return nil

builder/evaluator/internals.go

@@ -21,12 +21,12 @@ import (
 	"github.com/docker/docker/archive"
 	"github.com/docker/docker/daemon"
 	imagepkg "github.com/docker/docker/image"
+	"github.com/docker/docker/pkg/log"
 	"github.com/docker/docker/pkg/parsers"
 	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/pkg/tarsum"
 	"github.com/docker/docker/registry"
-	"github.com/docker/docker/runconfig"
 	"github.com/docker/docker/utils"
 )
 
@@ -299,13 +299,15 @@ func (b *BuildFile) pullImage(name string) (*imagepkg.Image, error) {
 
 func (b *BuildFile) processImageFrom(img *imagepkg.Image) error {
 	b.image = img.ID
-	b.Config = &runconfig.Config{}
+
 	if img.Config != nil {
 		b.Config = img.Config
 	}
+
 	if b.Config.Env == nil || len(b.Config.Env) == 0 {
 		b.Config.Env = append(b.Config.Env, "PATH="+daemon.DefaultPathEnv)
 	}
+
 	// Process ONBUILD triggers if they exist
 	if nTriggers := len(b.Config.OnBuild); nTriggers != 0 {
 		fmt.Fprintf(b.Options.ErrStream, "# Executing %d build triggers\n", nTriggers)
@@ -332,7 +334,7 @@ func (b *BuildFile) processImageFrom(img *imagepkg.Image) error {
 		// in this function.
 
 		if f, ok := evaluateTable[strings.ToLower(stepInstruction)]; ok {
-			if err := f(b, splitStep[1:]); err != nil {
+			if err := f(b, splitStep[1:], nil); err != nil {
 				return err
 			}
 		} else {
@@ -354,11 +356,11 @@ func (b *BuildFile) probeCache() (bool, error) {
 			return false, err
 		} else if cache != nil {
 			fmt.Fprintf(b.Options.OutStream, " ---> Using cache\n")
-			utils.Debugf("[BUILDER] Use cached version")
+			log.Debugf("[BUILDER] Use cached version")
 			b.image = cache.ID
 			return true, nil
 		} else {
-			utils.Debugf("[BUILDER] Cache miss")
+			log.Debugf("[BUILDER] Cache miss")
 		}
 	}
 	return false, nil
@@ -423,19 +425,17 @@ func (b *BuildFile) run(c *daemon.Container) error {
 
 func (b *BuildFile) checkPathForAddition(orig string) error {
 	origPath := path.Join(b.contextPath, orig)
-	if p, err := filepath.EvalSymlinks(origPath); err != nil {
+	origPath, err := filepath.EvalSymlinks(origPath)
+	if err != nil {
 		if os.IsNotExist(err) {
 			return fmt.Errorf("%s: no such file or directory", orig)
 		}
 		return err
-	} else {
-		origPath = p
 	}
 	if !strings.HasPrefix(origPath, b.contextPath) {
 		return fmt.Errorf("Forbidden path outside the build context: %s (%s)", orig, origPath)
 	}
-	_, err := os.Stat(origPath)
-	if err != nil {
+	if _, err := os.Stat(origPath); err != nil {
 		if os.IsNotExist(err) {
 			return fmt.Errorf("%s: no such file or directory", orig)
 		}
@@ -499,7 +499,7 @@ func (b *BuildFile) addContext(container *daemon.Container, orig, dest string, d
 		if err := archive.UntarPath(origPath, tarDest); err == nil {
 			return nil
 		} else if err != io.EOF {
-			utils.Debugf("Couldn't untar %s to %s: %s", origPath, tarDest, err)
+			log.Debugf("Couldn't untar %s to %s: %s", origPath, tarDest, err)
 		}
 	}
 

builder/evaluator/support.go

@@ -10,17 +10,37 @@ var (
 )
 
 // handle environment replacement. Used in dispatcher.
-func replaceEnv(b *BuildFile, str string) string {
+func (b *BuildFile) replaceEnv(str string) string {
 	for _, match := range TOKEN_ENV_INTERPOLATION.FindAllString(str, -1) {
 		match = match[strings.Index(match, "$"):]
 		matchKey := strings.Trim(match, "${}")
 
-		for envKey, envValue := range b.Env {
-			if matchKey == envKey {
-				str = strings.Replace(str, match, envValue, -1)
+		for _, keyval := range b.Config.Env {
+			tmp := strings.SplitN(keyval, "=", 2)
+			if tmp[0] == matchKey {
+				str = strings.Replace(str, match, tmp[1], -1)
 			}
 		}
 	}
 
 	return str
 }
+
+func (b *BuildFile) FindEnvKey(key string) int {
+	for k, envVar := range b.Config.Env {
+		envParts := strings.SplitN(envVar, "=", 2)
+		if key == envParts[0] {
+			return k
+		}
+	}
+	return -1
+}
+
+func handleJsonArgs(args []string, attributes map[string]bool) []string {
+	if attributes != nil && attributes["json"] {
+		return args
+	}
+
+	// literal string command, not an exec array
+	return append([]string{"/bin/sh", "-c", strings.Join(args, " ")})
+}

builder/parser/line_parsers.go

@@ -14,13 +14,13 @@ import (
 )
 
 var (
-	dockerFileErrJSONNesting = errors.New("You may not nest arrays in Dockerfile statements.")
+	errDockerfileJSONNesting = errors.New("You may not nest arrays in Dockerfile statements.")
 )
 
 // ignore the current argument. This will still leave a command parsed, but
 // will not incorporate the arguments into the ast.
-func parseIgnore(rest string) (*Node, error) {
-	return &Node{}, nil
+func parseIgnore(rest string) (*Node, map[string]bool, error) {
+	return &Node{}, nil, nil
 }
 
 // used for onbuild. Could potentially be used for anything that represents a
@@ -28,18 +28,18 @@ func parseIgnore(rest string) (*Node, error) {
 //
 // ONBUILD RUN foo bar -> (onbuild (run foo bar))
 //
-func parseSubCommand(rest string) (*Node, error) {
+func parseSubCommand(rest string) (*Node, map[string]bool, error) {
 	_, child, err := parseLine(rest)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
-	return &Node{Children: []*Node{child}}, nil
+	return &Node{Children: []*Node{child}}, nil, nil
 }
 
 // parse environment like statements. Note that this does *not* handle
 // variable interpolation, which will be handled in the evaluator.
-func parseEnv(rest string) (*Node, error) {
+func parseEnv(rest string) (*Node, map[string]bool, error) {
 	node := &Node{}
 	rootnode := node
 	strs := TOKEN_WHITESPACE.Split(rest, 2)
@@ -47,12 +47,12 @@ func parseEnv(rest string) (*Node, error) {
 	node.Next = &Node{}
 	node.Next.Value = strs[1]
 
-	return rootnode, nil
+	return rootnode, nil, nil
 }
 
 // parses a whitespace-delimited set of arguments. The result is effectively a
 // linked list of string arguments.
-func parseStringsWhitespaceDelimited(rest string) (*Node, error) {
+func parseStringsWhitespaceDelimited(rest string) (*Node, map[string]bool, error) {
 	node := &Node{}
 	rootnode := node
 	prevnode := node
@@ -68,16 +68,18 @@ func parseStringsWhitespaceDelimited(rest string) (*Node, error) {
 	// chain.
 	prevnode.Next = nil
 
-	return rootnode, nil
+	return rootnode, nil, nil
 }
 
 // parsestring just wraps the string in quotes and returns a working node.
-func parseString(rest string) (*Node, error) {
-	return &Node{rest, nil, nil}, nil
+func parseString(rest string) (*Node, map[string]bool, error) {
+	n := &Node{}
+	n.Value = rest
+	return n, nil, nil
 }
 
 // parseJSON converts JSON arrays to an AST.
-func parseJSON(rest string) (*Node, error) {
+func parseJSON(rest string) (*Node, map[string]bool, error) {
 	var (
 		myJson   []interface{}
 		next     = &Node{}
@@ -86,7 +88,7 @@ func parseJSON(rest string) (*Node, error) {
 	)
 
 	if err := json.Unmarshal([]byte(rest), &myJson); err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
 	for _, str := range myJson {
@@ -95,7 +97,7 @@ func parseJSON(rest string) (*Node, error) {
 		case float64:
 			str = strconv.FormatFloat(str.(float64), 'G', -1, 64)
 		default:
-			return nil, dockerFileErrJSONNesting
+			return nil, nil, errDockerfileJSONNesting
 		}
 		next.Value = str.(string)
 		next.Next = &Node{}
@@ -105,26 +107,27 @@ func parseJSON(rest string) (*Node, error) {
 
 	prevnode.Next = nil
 
-	return orignext, nil
+	return orignext, map[string]bool{"json": true}, nil
 }
 
 // parseMaybeJSON determines if the argument appears to be a JSON array. If
 // so, passes to parseJSON; if not, quotes the result and returns a single
 // node.
-func parseMaybeJSON(rest string) (*Node, error) {
+func parseMaybeJSON(rest string) (*Node, map[string]bool, error) {
 	rest = strings.TrimSpace(rest)
 
 	if strings.HasPrefix(rest, "[") {
-		node, err := parseJSON(rest)
+		node, attrs, err := parseJSON(rest)
 
 		if err == nil {
-			return node, nil
-		} else if err == dockerFileErrJSONNesting {
-			return nil, err
+			return node, attrs, nil
+		}
+		if err == errDockerfileJSONNesting {
+			return nil, nil, err
 		}
 	}
 
 	node := &Node{}
 	node.Value = rest
-	return node, nil
+	return node, nil, nil
 }

builder/parser/parser.go

@@ -21,13 +21,14 @@ import (
 // works a little more effectively than a "proper" parse tree for our needs.
 //
 type Node struct {
-	Value    string  // actual content
-	Next     *Node   // the next item in the current sexp
-	Children []*Node // the children of this sexp
+	Value      string          // actual content
+	Next       *Node           // the next item in the current sexp
+	Children   []*Node         // the children of this sexp
+	Attributes map[string]bool // special attributes for this node
 }
 
 var (
-	dispatch                map[string]func(string) (*Node, error)
+	dispatch                map[string]func(string) (*Node, map[string]bool, error)
 	TOKEN_WHITESPACE        = regexp.MustCompile(`[\t\v\f\r ]+`)
 	TOKEN_LINE_CONTINUATION = regexp.MustCompile(`\\$`)
 	TOKEN_COMMENT           = regexp.MustCompile(`^#.*$`)
@@ -40,7 +41,7 @@ func init() {
 	// reformulating the arguments according to the rules in the parser
 	// functions. Errors are propogated up by Parse() and the resulting AST can
 	// be incorporated directly into the existing AST as a next.
-	dispatch = map[string]func(string) (*Node, error){
+	dispatch = map[string]func(string) (*Node, map[string]bool, error){
 		"user":           parseString,
 		"onbuild":        parseSubCommand,
 		"workdir":        parseString,
@@ -75,12 +76,13 @@ func parseLine(line string) (string, *Node, error) {
 	node := &Node{}
 	node.Value = cmd
 
-	sexp, err := fullDispatch(cmd, args)
+	sexp, attrs, err := fullDispatch(cmd, args)
 	if err != nil {
 		return "", nil, err
 	}
 
 	node.Next = sexp
+	node.Attributes = attrs
 
 	return "", node, nil
 }

builder/parser/utils.go

@@ -51,17 +51,17 @@ func (node *Node) Dump() string {
 
 // performs the dispatch based on the two primal strings, cmd and args. Please
 // look at the dispatch table in parser.go to see how these dispatchers work.
-func fullDispatch(cmd, args string) (*Node, error) {
+func fullDispatch(cmd, args string) (*Node, map[string]bool, error) {
 	if _, ok := dispatch[cmd]; !ok {
-		return nil, fmt.Errorf("'%s' is not a valid dockerfile command", cmd)
+		return nil, nil, fmt.Errorf("'%s' is not a valid dockerfile command", cmd)
 	}
 
-	sexp, err := dispatch[cmd](args)
+	sexp, attrs, err := dispatch[cmd](args)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 
-	return sexp, nil
+	return sexp, attrs, nil
 }
 
 // splitCommand takes a single line of text and parses out the cmd and args,

integration-cli/docker_cli_build_test.go

@@ -685,10 +685,11 @@ func TestBuildRelativeWorkdir(t *testing.T) {
 
 func TestBuildEnv(t *testing.T) {
 	name := "testbuildenv"
-	expected := "[PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin PORT=2375]"
+	expected := "[PATH=/test:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin PORT=2375]"
 	defer deleteImages(name)
 	_, err := buildImage(name,
 		`FROM busybox
+		ENV PATH /test:$PATH
         ENV PORT 2375
 		RUN [ $(env | grep PORT) = 'PORT=2375' ]`,
 		true)
@@ -1708,6 +1709,9 @@ func TestBuildEnvUsage(t *testing.T) {
 	name := "testbuildenvusage"
 	defer deleteImages(name)
 	dockerfile := `FROM busybox
+ENV    PATH $HOME/bin:$PATH
+ENV    PATH /tmp:$PATH
+RUN    [ "$PATH" = "/tmp:$HOME/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" ]
 ENV    FOO /foo/baz
 ENV    BAR /bar
 ENV    BAZ $BAR
@@ -1717,7 +1721,8 @@ RUN    [ "$FOOPATH" = "$PATH:/foo/baz" ]
 ENV	   FROM hello/docker/world
 ENV    TO /docker/world/hello
 ADD    $FROM $TO
-RUN    [ "$(cat $TO)" = "hello" ]`
+RUN    [ "$(cat $TO)" = "hello" ]
+`
 	ctx, err := fakeContext(dockerfile, map[string]string{
 		"hello/docker/world": "hello",
 	})