Some refactoring of dispatch()

Remove runConfig from Builder and dispatchRequest. It is not only on
dispatchState.

Move dispatch state fields from Builder to dispatchState

Move stageName tracking to dispatchRequest.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

builder/dockerfile/buildargs.go

@@ -1,5 +1,10 @@
 package dockerfile
 
+import (
+	"fmt"
+	"github.com/docker/docker/runconfig/opts"
+)
+
 // builtinAllowedBuildArgs is list of built-in allowed build args
 // these args are considered transparent and are excluded from the image history.
 // Filtering from history is implemented in dispatchers.go
@@ -96,6 +101,19 @@ func (b *buildArgs) getAllFromMapping(source map[string]*string) map[string]stri
 	return m
 }
 
+// FilterAllowed returns all allowed args without the filtered args
+func (b *buildArgs) FilterAllowed(filter []string) []string {
+	envs := []string{}
+	configEnv := opts.ConvertKVStringsToMap(filter)
+
+	for key, val := range b.GetAllAllowed() {
+		if _, ok := configEnv[key]; !ok {
+			envs = append(envs, fmt.Sprintf("%s=%s", key, val))
+		}
+	}
+	return envs
+}
+
 func (b *buildArgs) getBuildArg(key string, mapping map[string]*string) (string, bool) {
 	defaultValue, exists := mapping[key]
 	// Return override from options if one is defined

builder/dockerfile/builder.go

@@ -95,20 +95,12 @@ type Builder struct {
 	source    builder.Source
 	clientCtx context.Context
 
-	runConfig     *container.Config // runconfig for cmd, run, entrypoint etc.
 	tmpContainers map[string]struct{}
 	imageContexts *imageContexts // helper for storing contexts from builds
 	disableCommit bool
 	cacheBusted   bool
 	buildArgs     *buildArgs
 	imageCache    builder.ImageCache
-
-	// TODO: these move to DispatchState
-	maintainer  string
-	cmdSet      bool
-	noBaseImage bool   // A flag to track the use of `scratch` as the base image
-	image       string // imageID
-	from        builder.Image
 }
 
 // newBuilder creates a new Dockerfile builder from an optional dockerfile and a Options.
@@ -124,7 +116,6 @@ func newBuilder(clientCtx context.Context, options builderOptions) *Builder {
 		Stderr:        options.ProgressWriter.StderrFormatter,
 		Output:        options.ProgressWriter.Output,
 		docker:        options.Backend,
-		runConfig:     new(container.Config),
 		tmpContainers: map[string]struct{}{},
 		buildArgs:     newBuildArgs(config.BuildArgs),
 	}
@@ -136,7 +127,6 @@ func (b *Builder) resetImageCache() {
 	if icb, ok := b.docker.(builder.ImageCacheBuilder); ok {
 		b.imageCache = icb.MakeImageCache(b.options.CacheFrom)
 	}
-	b.noBaseImage = false
 	b.cacheBusted = false
 }
 
@@ -154,59 +144,61 @@ func (b *Builder) build(source builder.Source, dockerfile *parser.Result) (*buil
 		return nil, err
 	}
 
-	imageID, err := b.dispatchDockerfileWithCancellation(dockerfile)
+	dispatchState, err := b.dispatchDockerfileWithCancellation(dockerfile)
 	if err != nil {
 		return nil, err
 	}
 
+	if b.options.Target != "" && !dispatchState.isCurrentStage(b.options.Target) {
+		return nil, errors.Errorf("failed to reach build target %s in Dockerfile", b.options.Target)
+	}
+
 	b.warnOnUnusedBuildArgs()
 
-	if imageID == "" {
+	if dispatchState.imageID == "" {
 		return nil, errors.New("No image was generated. Is your Dockerfile empty?")
 	}
-	return &builder.Result{ImageID: imageID, FromImage: b.from}, nil
+	return &builder.Result{ImageID: dispatchState.imageID, FromImage: dispatchState.baseImage}, nil
 }
 
-func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Result) (string, error) {
+func (b *Builder) dispatchDockerfileWithCancellation(dockerfile *parser.Result) (*dispatchState, error) {
 	shlex := NewShellLex(dockerfile.EscapeToken)
-
+	state := newDispatchState()
 	total := len(dockerfile.AST.Children)
-	var imageID string
+	var err error
 	for i, n := range dockerfile.AST.Children {
 		select {
 		case <-b.clientCtx.Done():
 			logrus.Debug("Builder: build cancelled!")
 			fmt.Fprint(b.Stdout, "Build cancelled")
-			return "", errors.New("Build cancelled")
+			return nil, errors.New("Build cancelled")
 		default:
 			// Not cancelled yet, keep going...
 		}
 
-		if command.From == n.Value && b.imageContexts.isCurrentTarget(b.options.Target) {
+		if n.Value == command.From && state.isCurrentStage(b.options.Target) {
 			break
 		}
 
-		if err := b.dispatch(i, total, n, shlex); err != nil {
+		opts := dispatchOptions{
+			state:   state,
+			stepMsg: formatStep(i, total),
+			node:    n,
+			shlex:   shlex,
+		}
+		if state, err = b.dispatch(opts); err != nil {
 			if b.options.ForceRemove {
 				b.clearTmp()
 			}
-			return "", err
+			return nil, err
 		}
 
-		// TODO: get this from dispatch
-		imageID = b.image
-
-		fmt.Fprintf(b.Stdout, " ---> %s\n", stringid.TruncateID(imageID))
+		fmt.Fprintf(b.Stdout, " ---> %s\n", stringid.TruncateID(state.imageID))
 		if b.options.Remove {
 			b.clearTmp()
 		}
 	}
-
-	if b.options.Target != "" && !b.imageContexts.isCurrentTarget(b.options.Target) {
-		return "", errors.Errorf("failed to reach build target %s in Dockerfile", b.options.Target)
-	}
-
-	return imageID, nil
+	return state, nil
 }
 
 func addNodesForLabelOption(dockerfile *parser.Node, labels map[string]string) {
@@ -227,12 +219,6 @@ func (b *Builder) warnOnUnusedBuildArgs() {
 	}
 }
 
-// hasFromImage returns true if the builder has processed a `FROM <image>` line
-// TODO: move to DispatchState
-func (b *Builder) hasFromImage() bool {
-	return b.image != "" || b.noBaseImage
-}
-
 // BuildFromConfig builds directly from `changes`, treating it as if it were the contents of a Dockerfile
 // It will:
 // - Call parse.Parse() to get an AST root for the concatenated Dockerfile entries.
@@ -249,31 +235,28 @@ func BuildFromConfig(config *container.Config, changes []string) (*container.Con
 
 	b := newBuilder(context.Background(), builderOptions{})
 
-	result, err := parser.Parse(bytes.NewBufferString(strings.Join(changes, "\n")))
+	dockerfile, err := parser.Parse(bytes.NewBufferString(strings.Join(changes, "\n")))
 	if err != nil {
 		return nil, err
 	}
 
 	// ensure that the commands are valid
-	for _, n := range result.AST.Children {
+	for _, n := range dockerfile.AST.Children {
 		if !validCommitCommands[n.Value] {
 			return nil, fmt.Errorf("%s is not a valid change command", n.Value)
 		}
 	}
 
-	b.runConfig = config
 	b.Stdout = ioutil.Discard
 	b.Stderr = ioutil.Discard
 	b.disableCommit = true
 
-	if err := checkDispatchDockerfile(result.AST); err != nil {
-		return nil, err
-	}
-
-	if err := dispatchFromDockerfile(b, result); err != nil {
+	if err := checkDispatchDockerfile(dockerfile.AST); err != nil {
 		return nil, err
 	}
-	return b.runConfig, nil
+	dispatchState := newDispatchState()
+	dispatchState.runConfig = config
+	return dispatchFromDockerfile(b, dockerfile, dispatchState)
 }
 
 func checkDispatchDockerfile(dockerfile *parser.Node) error {
@@ -285,15 +268,21 @@ func checkDispatchDockerfile(dockerfile *parser.Node) error {
 	return nil
 }
 
-func dispatchFromDockerfile(b *Builder, result *parser.Result) error {
+func dispatchFromDockerfile(b *Builder, result *parser.Result, dispatchState *dispatchState) (*container.Config, error) {
 	shlex := NewShellLex(result.EscapeToken)
 	ast := result.AST
 	total := len(ast.Children)
 
 	for i, n := range ast.Children {
-		if err := b.dispatch(i, total, n, shlex); err != nil {
-			return err
+		opts := dispatchOptions{
+			state:   dispatchState,
+			stepMsg: formatStep(i, total),
+			node:    n,
+			shlex:   shlex,
+		}
+		if _, err := b.dispatch(opts); err != nil {
+			return nil, err
 		}
 	}
-	return nil
+	return dispatchState.runConfig, nil
 }

builder/dockerfile/dispatchers.go

@@ -47,6 +47,7 @@ func env(req dispatchRequest) error {
 		return err
 	}
 
+	runConfig := req.state.runConfig
 	commitMessage := bytes.NewBufferString("ENV")
 
 	for j := 0; j < len(req.args); j += 2 {
@@ -59,21 +60,21 @@ func env(req dispatchRequest) error {
 		commitMessage.WriteString(" " + newVar)
 
 		gotOne := false
-		for i, envVar := range req.runConfig.Env {
+		for i, envVar := range runConfig.Env {
 			envParts := strings.SplitN(envVar, "=", 2)
 			compareFrom := envParts[0]
 			if equalEnvKeys(compareFrom, name) {
-				req.runConfig.Env[i] = newVar
+				runConfig.Env[i] = newVar
 				gotOne = true
 				break
 			}
 		}
 		if !gotOne {
-			req.runConfig.Env = append(req.runConfig.Env, newVar)
+			runConfig.Env = append(runConfig.Env, newVar)
 		}
 	}
 
-	return req.builder.commit(commitMessage.String())
+	return req.builder.commit(req.state, commitMessage.String())
 }
 
 // MAINTAINER some text <maybe@an.email.address>
@@ -89,8 +90,8 @@ func maintainer(req dispatchRequest) error {
 	}
 
 	maintainer := req.args[0]
-	req.builder.maintainer = maintainer
-	return req.builder.commit("MAINTAINER " + maintainer)
+	req.state.maintainer = maintainer
+	return req.builder.commit(req.state, "MAINTAINER "+maintainer)
 }
 
 // LABEL some json data describing the image
@@ -111,26 +112,25 @@ func label(req dispatchRequest) error {
 	}
 
 	commitStr := "LABEL"
+	runConfig := req.state.runConfig
 
-	if req.runConfig.Labels == nil {
-		req.runConfig.Labels = map[string]string{}
+	if runConfig.Labels == nil {
+		runConfig.Labels = map[string]string{}
 	}
 
 	for j := 0; j < len(req.args); j++ {
-		// name  ==> req.args[j]
-		// value ==> req.args[j+1]
-
-		if len(req.args[j]) == 0 {
+		name := req.args[j]
+		if name == "" {
 			return errBlankCommandNames("LABEL")
 		}
 
-		newVar := req.args[j] + "=" + req.args[j+1] + ""
-		commitStr += " " + newVar
+		value := req.args[j+1]
+		commitStr += " " + name + "=" + value
 
-		req.runConfig.Labels[req.args[j]] = req.args[j+1]
+		runConfig.Labels[name] = value
 		j++
 	}
-	return req.builder.commit(commitStr)
+	return req.builder.commit(req.state, commitStr)
 }
 
 // ADD foo /path
@@ -147,7 +147,7 @@ func add(req dispatchRequest) error {
 		return err
 	}
 
-	return req.builder.runContextCommand(req.args, true, true, "ADD", nil)
+	return req.builder.runContextCommand(req, true, true, "ADD", nil)
 }
 
 // COPY foo /path
@@ -174,13 +174,13 @@ func dispatchCopy(req dispatchRequest) error {
 		}
 	}
 
-	return req.builder.runContextCommand(req.args, false, false, "COPY", im)
+	return req.builder.runContextCommand(req, false, false, "COPY", im)
 }
 
 // FROM imagename[:tag | @digest] [AS build-stage-name]
 //
 func from(req dispatchRequest) error {
-	ctxName, err := parseBuildStageName(req.args)
+	stageName, err := parseBuildStageName(req.args)
 	if err != nil {
 		return err
 	}
@@ -190,21 +190,23 @@ func from(req dispatchRequest) error {
 	}
 
 	req.builder.resetImageCache()
-	if _, err := req.builder.imageContexts.add(ctxName); err != nil {
+	req.state.noBaseImage = false
+	req.state.stageName = stageName
+	if _, err := req.builder.imageContexts.add(stageName); err != nil {
 		return err
 	}
 
-	image, err := req.builder.getFromImage(req.shlex, req.args[0])
+	image, err := req.builder.getFromImage(req.state, req.shlex, req.args[0])
 	if err != nil {
 		return err
 	}
 	if image != nil {
 		req.builder.imageContexts.update(image.ImageID(), image.RunConfig())
 	}
-	req.builder.from = image
+	req.state.baseImage = image
 
 	req.builder.buildArgs.ResetAllowed()
-	return req.builder.processImageFrom(image)
+	return req.builder.processImageFrom(req.state, image)
 }
 
 func parseBuildStageName(args []string) (string, error) {
@@ -222,7 +224,7 @@ func parseBuildStageName(args []string) (string, error) {
 	return stageName, nil
 }
 
-func (b *Builder) getFromImage(shlex *ShellLex, name string) (builder.Image, error) {
+func (b *Builder) getFromImage(dispatchState *dispatchState, shlex *ShellLex, name string) (builder.Image, error) {
 	substitutionArgs := []string{}
 	for key, value := range b.buildArgs.GetAllMeta() {
 		substitutionArgs = append(substitutionArgs, key+"="+value)
@@ -246,8 +248,8 @@ func (b *Builder) getFromImage(shlex *ShellLex, name string) (builder.Image, err
 		if runtime.GOOS == "windows" {
 			return nil, errors.New("Windows does not support FROM scratch")
 		}
-		b.image = ""
-		b.noBaseImage = true
+		dispatchState.imageID = ""
+		dispatchState.noBaseImage = true
 		return nil, nil
 	}
 	return pullOrGetImage(b, name)
@@ -279,9 +281,10 @@ func onbuild(req dispatchRequest) error {
 		return fmt.Errorf("%s isn't allowed as an ONBUILD trigger", triggerInstruction)
 	}
 
+	runConfig := req.state.runConfig
 	original := regexp.MustCompile(`(?i)^\s*ONBUILD\s*`).ReplaceAllString(req.original, "")
-	req.runConfig.OnBuild = append(req.runConfig.OnBuild, original)
-	return req.builder.commit("ONBUILD " + original)
+	runConfig.OnBuild = append(runConfig.OnBuild, original)
+	return req.builder.commit(req.state, "ONBUILD "+original)
 }
 
 // WORKDIR /tmp
@@ -298,9 +301,10 @@ func workdir(req dispatchRequest) error {
 		return err
 	}
 
+	runConfig := req.state.runConfig
 	// This is from the Dockerfile and will not necessarily be in platform
 	// specific semantics, hence ensure it is converted.
-	req.runConfig.WorkingDir, err = normaliseWorkdir(req.runConfig.WorkingDir, req.args[0])
+	runConfig.WorkingDir, err = normaliseWorkdir(runConfig.WorkingDir, req.args[0])
 	if err != nil {
 		return err
 	}
@@ -315,9 +319,9 @@ func workdir(req dispatchRequest) error {
 		return nil
 	}
 
-	comment := "WORKDIR " + req.runConfig.WorkingDir
-	runConfigWithCommentCmd := copyRunConfig(req.runConfig, withCmdCommentString(comment))
-	if hit, err := req.builder.probeCache(req.builder.image, runConfigWithCommentCmd); err != nil || hit {
+	comment := "WORKDIR " + runConfig.WorkingDir
+	runConfigWithCommentCmd := copyRunConfig(runConfig, withCmdCommentString(comment))
+	if hit, err := req.builder.probeCache(req.state, runConfigWithCommentCmd); err != nil || hit {
 		return err
 	}
 
@@ -334,7 +338,7 @@ func workdir(req dispatchRequest) error {
 		return err
 	}
 
-	return req.builder.commitContainer(container.ID, runConfigWithCommentCmd)
+	return req.builder.commitContainer(req.state, container.ID, runConfigWithCommentCmd)
 }
 
 // RUN some command yo
@@ -348,7 +352,7 @@ func workdir(req dispatchRequest) error {
 // RUN [ "echo", "hi" ] # echo hi
 //
 func run(req dispatchRequest) error {
-	if !req.builder.hasFromImage() {
+	if !req.state.hasFromImage() {
 		return errors.New("Please provide a source image with `from` prior to run")
 	}
 
@@ -356,29 +360,30 @@ func run(req dispatchRequest) error {
 		return err
 	}
 
+	stateRunConfig := req.state.runConfig
 	args := handleJSONArgs(req.args, req.attributes)
 	if !req.attributes["json"] {
-		args = append(getShell(req.runConfig), args...)
+		args = append(getShell(stateRunConfig), args...)
 	}
 	cmdFromArgs := strslice.StrSlice(args)
-	buildArgs := req.builder.buildArgsWithoutConfigEnv()
+	buildArgs := req.builder.buildArgs.FilterAllowed(stateRunConfig.Env)
 
 	saveCmd := cmdFromArgs
 	if len(buildArgs) > 0 {
 		saveCmd = prependEnvOnCmd(req.builder.buildArgs, buildArgs, cmdFromArgs)
 	}
 
-	runConfigForCacheProbe := copyRunConfig(req.runConfig,
+	runConfigForCacheProbe := copyRunConfig(stateRunConfig,
 		withCmd(saveCmd),
 		withEntrypointOverride(saveCmd, nil))
-	hit, err := req.builder.probeCache(req.builder.image, runConfigForCacheProbe)
+	hit, err := req.builder.probeCache(req.state, runConfigForCacheProbe)
 	if err != nil || hit {
 		return err
 	}
 
-	runConfig := copyRunConfig(req.runConfig,
+	runConfig := copyRunConfig(stateRunConfig,
 		withCmd(cmdFromArgs),
-		withEnv(append(req.runConfig.Env, buildArgs...)),
+		withEnv(append(stateRunConfig.Env, buildArgs...)),
 		withEntrypointOverride(saveCmd, strslice.StrSlice{""}))
 
 	// set config as already being escaped, this prevents double escaping on windows
@@ -393,7 +398,7 @@ func run(req dispatchRequest) error {
 		return err
 	}
 
-	return req.builder.commitContainer(cID, runConfigForCacheProbe)
+	return req.builder.commitContainer(req.state, cID, runConfigForCacheProbe)
 }
 
 // Derive the command to use for probeCache() and to commit in this container.
@@ -431,22 +436,22 @@ func cmd(req dispatchRequest) error {
 		return err
 	}
 
+	runConfig := req.state.runConfig
 	cmdSlice := handleJSONArgs(req.args, req.attributes)
-
 	if !req.attributes["json"] {
-		cmdSlice = append(getShell(req.runConfig), cmdSlice...)
+		cmdSlice = append(getShell(runConfig), cmdSlice...)
 	}
 
-	req.runConfig.Cmd = strslice.StrSlice(cmdSlice)
+	runConfig.Cmd = strslice.StrSlice(cmdSlice)
 	// set config as already being escaped, this prevents double escaping on windows
-	req.runConfig.ArgsEscaped = true
+	runConfig.ArgsEscaped = true
 
-	if err := req.builder.commit(fmt.Sprintf("CMD %q", cmdSlice)); err != nil {
+	if err := req.builder.commit(req.state, fmt.Sprintf("CMD %q", cmdSlice)); err != nil {
 		return err
 	}
 
 	if len(req.args) != 0 {
-		req.builder.cmdSet = true
+		req.state.cmdSet = true
 	}
 
 	return nil
@@ -478,6 +483,7 @@ func healthcheck(req dispatchRequest) error {
 	if len(req.args) == 0 {
 		return errAtLeastOneArgument("HEALTHCHECK")
 	}
+	runConfig := req.state.runConfig
 	typ := strings.ToUpper(req.args[0])
 	args := req.args[1:]
 	if typ == "NONE" {
@@ -485,12 +491,12 @@ func healthcheck(req dispatchRequest) error {
 			return errors.New("HEALTHCHECK NONE takes no arguments")
 		}
 		test := strslice.StrSlice{typ}
-		req.runConfig.Healthcheck = &container.HealthConfig{
+		runConfig.Healthcheck = &container.HealthConfig{
 			Test: test,
 		}
 	} else {
-		if req.runConfig.Healthcheck != nil {
-			oldCmd := req.runConfig.Healthcheck.Test
+		if runConfig.Healthcheck != nil {
+			oldCmd := runConfig.Healthcheck.Test
 			if len(oldCmd) > 0 && oldCmd[0] != "NONE" {
 				fmt.Fprintf(req.builder.Stdout, "Note: overriding previous HEALTHCHECK: %v\n", oldCmd)
 			}
@@ -554,10 +560,10 @@ func healthcheck(req dispatchRequest) error {
 			healthcheck.Retries = 0
 		}
 
-		req.runConfig.Healthcheck = &healthcheck
+		runConfig.Healthcheck = &healthcheck
 	}
 
-	return req.builder.commit(fmt.Sprintf("HEALTHCHECK %q", req.runConfig.Healthcheck))
+	return req.builder.commit(req.state, fmt.Sprintf("HEALTHCHECK %q", runConfig.Healthcheck))
 }
 
 // ENTRYPOINT /usr/sbin/nginx
@@ -573,27 +579,28 @@ func entrypoint(req dispatchRequest) error {
 		return err
 	}
 
+	runConfig := req.state.runConfig
 	parsed := handleJSONArgs(req.args, req.attributes)
 
 	switch {
 	case req.attributes["json"]:
 		// ENTRYPOINT ["echo", "hi"]
-		req.runConfig.Entrypoint = strslice.StrSlice(parsed)
+		runConfig.Entrypoint = strslice.StrSlice(parsed)
 	case len(parsed) == 0:
 		// ENTRYPOINT []
-		req.runConfig.Entrypoint = nil
+		runConfig.Entrypoint = nil
 	default:
 		// ENTRYPOINT echo hi
-		req.runConfig.Entrypoint = strslice.StrSlice(append(getShell(req.runConfig), parsed[0]))
+		runConfig.Entrypoint = strslice.StrSlice(append(getShell(runConfig), parsed[0]))
 	}
 
 	// when setting the entrypoint if a CMD was not explicitly set then
 	// set the command to nil
-	if !req.builder.cmdSet {
-		req.runConfig.Cmd = nil
+	if !req.state.cmdSet {
+		runConfig.Cmd = nil
 	}
 
-	return req.builder.commit(fmt.Sprintf("ENTRYPOINT %q", req.runConfig.Entrypoint))
+	return req.builder.commit(req.state, fmt.Sprintf("ENTRYPOINT %q", runConfig.Entrypoint))
 }
 
 // EXPOSE 6667/tcp 7000/tcp
@@ -612,8 +619,9 @@ func expose(req dispatchRequest) error {
 		return err
 	}
 
-	if req.runConfig.ExposedPorts == nil {
-		req.runConfig.ExposedPorts = make(nat.PortSet)
+	runConfig := req.state.runConfig
+	if runConfig.ExposedPorts == nil {
+		runConfig.ExposedPorts = make(nat.PortSet)
 	}
 
 	ports, _, err := nat.ParsePortSpecs(portsTab)
@@ -627,14 +635,14 @@ func expose(req dispatchRequest) error {
 	portList := make([]string, len(ports))
 	var i int
 	for port := range ports {
-		if _, exists := req.runConfig.ExposedPorts[port]; !exists {
-			req.runConfig.ExposedPorts[port] = struct{}{}
+		if _, exists := runConfig.ExposedPorts[port]; !exists {
+			runConfig.ExposedPorts[port] = struct{}{}
 		}
 		portList[i] = string(port)
 		i++
 	}
 	sort.Strings(portList)
-	return req.builder.commit("EXPOSE " + strings.Join(portList, " "))
+	return req.builder.commit(req.state, "EXPOSE "+strings.Join(portList, " "))
 }
 
 // USER foo
@@ -651,8 +659,8 @@ func user(req dispatchRequest) error {
 		return err
 	}
 
-	req.runConfig.User = req.args[0]
-	return req.builder.commit(fmt.Sprintf("USER %v", req.args))
+	req.state.runConfig.User = req.args[0]
+	return req.builder.commit(req.state, fmt.Sprintf("USER %v", req.args))
 }
 
 // VOLUME /foo
@@ -668,17 +676,18 @@ func volume(req dispatchRequest) error {
 		return err
 	}
 
-	if req.runConfig.Volumes == nil {
-		req.runConfig.Volumes = map[string]struct{}{}
+	runConfig := req.state.runConfig
+	if runConfig.Volumes == nil {
+		runConfig.Volumes = map[string]struct{}{}
 	}
 	for _, v := range req.args {
 		v = strings.TrimSpace(v)
 		if v == "" {
 			return errors.New("VOLUME specified can not be an empty string")
 		}
-		req.runConfig.Volumes[v] = struct{}{}
+		runConfig.Volumes[v] = struct{}{}
 	}
-	return req.builder.commit(fmt.Sprintf("VOLUME %v", req.args))
+	return req.builder.commit(req.state, fmt.Sprintf("VOLUME %v", req.args))
 }
 
 // STOPSIGNAL signal
@@ -695,8 +704,8 @@ func stopSignal(req dispatchRequest) error {
 		return err
 	}
 
-	req.runConfig.StopSignal = sig
-	return req.builder.commit(fmt.Sprintf("STOPSIGNAL %v", req.args))
+	req.state.runConfig.StopSignal = sig
+	return req.builder.commit(req.state, fmt.Sprintf("STOPSIGNAL %v", req.args))
 }
 
 // ARG name[=value]
@@ -742,11 +751,11 @@ func arg(req dispatchRequest) error {
 	req.builder.buildArgs.AddArg(name, value)
 
 	// Arg before FROM doesn't add a layer
-	if !req.builder.hasFromImage() {
+	if !req.state.hasFromImage() {
 		req.builder.buildArgs.AddMetaArg(name, value)
 		return nil
 	}
-	return req.builder.commit("ARG " + arg)
+	return req.builder.commit(req.state, "ARG "+arg)
 }
 
 // SHELL powershell -command
@@ -763,12 +772,12 @@ func shell(req dispatchRequest) error {
 		return errAtLeastOneArgument("SHELL")
 	case req.attributes["json"]:
 		// SHELL ["powershell", "-command"]
-		req.runConfig.Shell = strslice.StrSlice(shellSlice)
+		req.state.runConfig.Shell = strslice.StrSlice(shellSlice)
 	default:
 		// SHELL powershell -command - not JSON
 		return errNotJSON("SHELL", req.original)
 	}
-	return req.builder.commit(fmt.Sprintf("SHELL %v", shellSlice))
+	return req.builder.commit(req.state, fmt.Sprintf("SHELL %v", shellSlice))
 }
 
 func errAtLeastOneArgument(command string) error {

builder/dockerfile/dispatchers_test.go

@@ -26,7 +26,7 @@ type commandWithFunction struct {
 
 func withArgs(f dispatcher) func([]string) error {
 	return func(args []string) error {
-		return f(dispatchRequest{args: args, runConfig: &container.Config{}})
+		return f(dispatchRequest{args: args})
 	}
 }
 
@@ -38,17 +38,16 @@ func withBuilderAndArgs(builder *Builder, f dispatcher) func([]string) error {
 
 func defaultDispatchReq(builder *Builder, args ...string) dispatchRequest {
 	return dispatchRequest{
-		builder:   builder,
-		args:      args,
-		flags:     NewBFlags(),
-		runConfig: &container.Config{},
-		shlex:     NewShellLex(parser.DefaultEscapeToken),
+		builder: builder,
+		args:    args,
+		flags:   NewBFlags(),
+		shlex:   NewShellLex(parser.DefaultEscapeToken),
+		state:   &dispatchState{runConfig: &container.Config{}},
 	}
 }
 
 func newBuilderWithMockBackend() *Builder {
 	b := &Builder{
-		runConfig:     &container.Config{},
 		options:       &types.ImageBuildOptions{},
 		docker:        &MockBackend{},
 		buildArgs:     newBuildArgs(make(map[string]*string)),
@@ -138,7 +137,7 @@ func TestEnv2Variables(t *testing.T) {
 		fmt.Sprintf("%s=%s", args[0], args[1]),
 		fmt.Sprintf("%s=%s", args[2], args[3]),
 	}
-	assert.Equal(t, expected, req.runConfig.Env)
+	assert.Equal(t, expected, req.state.runConfig.Env)
 }
 
 func TestEnvValueWithExistingRunConfigEnv(t *testing.T) {
@@ -146,7 +145,7 @@ func TestEnvValueWithExistingRunConfigEnv(t *testing.T) {
 
 	args := []string{"var1", "val1"}
 	req := defaultDispatchReq(b, args...)
-	req.runConfig.Env = []string{"var1=old", "var2=fromenv"}
+	req.state.runConfig.Env = []string{"var1=old", "var2=fromenv"}
 	err := env(req)
 	require.NoError(t, err)
 
@@ -154,16 +153,17 @@ func TestEnvValueWithExistingRunConfigEnv(t *testing.T) {
 		fmt.Sprintf("%s=%s", args[0], args[1]),
 		"var2=fromenv",
 	}
-	assert.Equal(t, expected, req.runConfig.Env)
+	assert.Equal(t, expected, req.state.runConfig.Env)
 }
 
 func TestMaintainer(t *testing.T) {
 	maintainerEntry := "Some Maintainer <maintainer@example.com>"
 
 	b := newBuilderWithMockBackend()
-	err := maintainer(defaultDispatchReq(b, maintainerEntry))
+	req := defaultDispatchReq(b, maintainerEntry)
+	err := maintainer(req)
 	require.NoError(t, err)
-	assert.Equal(t, maintainerEntry, b.maintainer)
+	assert.Equal(t, maintainerEntry, req.state.maintainer)
 }
 
 func TestLabel(t *testing.T) {
@@ -176,13 +176,14 @@ func TestLabel(t *testing.T) {
 	err := label(req)
 	require.NoError(t, err)
 
-	require.Contains(t, req.runConfig.Labels, labelName)
-	assert.Equal(t, req.runConfig.Labels[labelName], labelValue)
+	require.Contains(t, req.state.runConfig.Labels, labelName)
+	assert.Equal(t, req.state.runConfig.Labels[labelName], labelValue)
 }
 
 func TestFromScratch(t *testing.T) {
 	b := newBuilderWithMockBackend()
-	err := from(defaultDispatchReq(b, "scratch"))
+	req := defaultDispatchReq(b, "scratch")
+	err := from(req)
 
 	if runtime.GOOS == "windows" {
 		assert.EqualError(t, err, "Windows does not support FROM scratch")
@@ -190,8 +191,8 @@ func TestFromScratch(t *testing.T) {
 	}
 
 	require.NoError(t, err)
-	assert.Equal(t, "", b.image)
-	assert.Equal(t, true, b.noBaseImage)
+	assert.Equal(t, "", req.state.imageID)
+	assert.Equal(t, true, req.state.noBaseImage)
 }
 
 func TestFromWithArg(t *testing.T) {
@@ -205,11 +206,12 @@ func TestFromWithArg(t *testing.T) {
 	b.docker.(*MockBackend).getImageOnBuildFunc = getImage
 
 	require.NoError(t, arg(defaultDispatchReq(b, "THETAG="+tag)))
-	err := from(defaultDispatchReq(b, "alpine${THETAG}"))
+	req := defaultDispatchReq(b, "alpine${THETAG}")
+	err := from(req)
 
 	require.NoError(t, err)
-	assert.Equal(t, expected, b.image)
-	assert.Equal(t, expected, b.from.ImageID())
+	assert.Equal(t, expected, req.state.imageID)
+	assert.Equal(t, expected, req.state.baseImage.ImageID())
 	assert.Len(t, b.buildArgs.GetAllAllowed(), 0)
 	assert.Len(t, b.buildArgs.GetAllMeta(), 1)
 }
@@ -225,9 +227,10 @@ func TestFromWithUndefinedArg(t *testing.T) {
 	b.docker.(*MockBackend).getImageOnBuildFunc = getImage
 	b.options.BuildArgs = map[string]*string{"THETAG": &tag}
 
-	err := from(defaultDispatchReq(b, "alpine${THETAG}"))
+	req := defaultDispatchReq(b, "alpine${THETAG}")
+	err := from(req)
 	require.NoError(t, err)
-	assert.Equal(t, expected, b.image)
+	assert.Equal(t, expected, req.state.imageID)
 }
 
 func TestOnbuildIllegalTriggers(t *testing.T) {
@@ -249,11 +252,11 @@ func TestOnbuild(t *testing.T) {
 
 	req := defaultDispatchReq(b, "ADD", ".", "/app/src")
 	req.original = "ONBUILD ADD . /app/src"
-	req.runConfig = &container.Config{}
+	req.state.runConfig = &container.Config{}
 
 	err := onbuild(req)
 	require.NoError(t, err)
-	assert.Equal(t, "ADD . /app/src", req.runConfig.OnBuild[0])
+	assert.Equal(t, "ADD . /app/src", req.state.runConfig.OnBuild[0])
 }
 
 func TestWorkdir(t *testing.T) {
@@ -266,7 +269,7 @@ func TestWorkdir(t *testing.T) {
 	req := defaultDispatchReq(b, workingDir)
 	err := workdir(req)
 	require.NoError(t, err)
-	assert.Equal(t, workingDir, req.runConfig.WorkingDir)
+	assert.Equal(t, workingDir, req.state.runConfig.WorkingDir)
 }
 
 func TestCmd(t *testing.T) {
@@ -284,8 +287,8 @@ func TestCmd(t *testing.T) {
 		expectedCommand = strslice.StrSlice(append([]string{"/bin/sh"}, "-c", command))
 	}
 
-	assert.Equal(t, expectedCommand, req.runConfig.Cmd)
-	assert.True(t, b.cmdSet)
+	assert.Equal(t, expectedCommand, req.state.runConfig.Cmd)
+	assert.True(t, req.state.cmdSet)
 }
 
 func TestHealthcheckNone(t *testing.T) {
@@ -295,8 +298,8 @@ func TestHealthcheckNone(t *testing.T) {
 	err := healthcheck(req)
 	require.NoError(t, err)
 
-	require.NotNil(t, req.runConfig.Healthcheck)
-	assert.Equal(t, []string{"NONE"}, req.runConfig.Healthcheck.Test)
+	require.NotNil(t, req.state.runConfig.Healthcheck)
+	assert.Equal(t, []string{"NONE"}, req.state.runConfig.Healthcheck.Test)
 }
 
 func TestHealthcheckCmd(t *testing.T) {
@@ -307,9 +310,9 @@ func TestHealthcheckCmd(t *testing.T) {
 	err := healthcheck(req)
 	require.NoError(t, err)
 
-	require.NotNil(t, req.runConfig.Healthcheck)
+	require.NotNil(t, req.state.runConfig.Healthcheck)
 	expectedTest := []string{"CMD-SHELL", "curl -f http://localhost/ || exit 1"}
-	assert.Equal(t, expectedTest, req.runConfig.Healthcheck.Test)
+	assert.Equal(t, expectedTest, req.state.runConfig.Healthcheck.Test)
 }
 
 func TestEntrypoint(t *testing.T) {
@@ -319,7 +322,7 @@ func TestEntrypoint(t *testing.T) {
 	req := defaultDispatchReq(b, entrypointCmd)
 	err := entrypoint(req)
 	require.NoError(t, err)
-	require.NotNil(t, req.runConfig.Entrypoint)
+	require.NotNil(t, req.state.runConfig.Entrypoint)
 
 	var expectedEntrypoint strslice.StrSlice
 	if runtime.GOOS == "windows" {
@@ -327,7 +330,7 @@ func TestEntrypoint(t *testing.T) {
 	} else {
 		expectedEntrypoint = strslice.StrSlice(append([]string{"/bin/sh"}, "-c", entrypointCmd))
 	}
-	assert.Equal(t, expectedEntrypoint, req.runConfig.Entrypoint)
+	assert.Equal(t, expectedEntrypoint, req.state.runConfig.Entrypoint)
 }
 
 func TestExpose(t *testing.T) {
@@ -338,12 +341,12 @@ func TestExpose(t *testing.T) {
 	err := expose(req)
 	require.NoError(t, err)
 
-	require.NotNil(t, req.runConfig.ExposedPorts)
-	require.Len(t, req.runConfig.ExposedPorts, 1)
+	require.NotNil(t, req.state.runConfig.ExposedPorts)
+	require.Len(t, req.state.runConfig.ExposedPorts, 1)
 
 	portsMapping, err := nat.ParsePortSpec(exposedPort)
 	require.NoError(t, err)
-	assert.Contains(t, req.runConfig.ExposedPorts, portsMapping[0].Port)
+	assert.Contains(t, req.state.runConfig.ExposedPorts, portsMapping[0].Port)
 }
 
 func TestUser(t *testing.T) {
@@ -353,7 +356,7 @@ func TestUser(t *testing.T) {
 	req := defaultDispatchReq(b, userCommand)
 	err := user(req)
 	require.NoError(t, err)
-	assert.Equal(t, userCommand, req.runConfig.User)
+	assert.Equal(t, userCommand, req.state.runConfig.User)
 }
 
 func TestVolume(t *testing.T) {
@@ -365,9 +368,9 @@ func TestVolume(t *testing.T) {
 	err := volume(req)
 	require.NoError(t, err)
 
-	require.NotNil(t, req.runConfig.Volumes)
-	assert.Len(t, req.runConfig.Volumes, 1)
-	assert.Contains(t, req.runConfig.Volumes, exposedVolume)
+	require.NotNil(t, req.state.runConfig.Volumes)
+	assert.Len(t, req.state.runConfig.Volumes, 1)
+	assert.Contains(t, req.state.runConfig.Volumes, exposedVolume)
 }
 
 func TestStopSignal(t *testing.T) {
@@ -377,7 +380,7 @@ func TestStopSignal(t *testing.T) {
 	req := defaultDispatchReq(b, signal)
 	err := stopSignal(req)
 	require.NoError(t, err)
-	assert.Equal(t, signal, req.runConfig.StopSignal)
+	assert.Equal(t, signal, req.state.runConfig.StopSignal)
 }
 
 func TestArg(t *testing.T) {
@@ -405,7 +408,7 @@ func TestShell(t *testing.T) {
 	require.NoError(t, err)
 
 	expectedShell := strslice.StrSlice([]string{shellCmd})
-	assert.Equal(t, expectedShell, req.runConfig.Shell)
+	assert.Equal(t, expectedShell, req.state.runConfig.Shell)
 }
 
 func TestParseOptInterval(t *testing.T) {
@@ -439,8 +442,9 @@ func TestRunWithBuildArgs(t *testing.T) {
 	b.buildArgs.argsFromOptions["HTTP_PROXY"] = strPtr("FOO")
 	b.disableCommit = false
 
+	runConfig := &container.Config{}
 	origCmd := strslice.StrSlice([]string{"cmd", "in", "from", "image"})
-	cmdWithShell := strslice.StrSlice(append(getShell(b.runConfig), "echo foo"))
+	cmdWithShell := strslice.StrSlice(append(getShell(runConfig), "echo foo"))
 	envVars := []string{"|1", "one=two"}
 	cachedCmd := strslice.StrSlice(append(envVars, cmdWithShell...))
 
@@ -477,12 +481,10 @@ func TestRunWithBuildArgs(t *testing.T) {
 	req := defaultDispatchReq(b, "abcdef")
 	require.NoError(t, from(req))
 	b.buildArgs.AddArg("one", strPtr("two"))
-	// TODO: this can be removed with b.runConfig
-	req.runConfig.Cmd = origCmd
 
 	req.args = []string{"echo foo"}
 	require.NoError(t, run(req))
 
 	// Check that runConfig.Cmd has not been modified by run
-	assert.Equal(t, origCmd, b.runConfig.Cmd)
+	assert.Equal(t, origCmd, req.state.runConfig.Cmd)
 }

builder/dockerfile/evaluator.go

@@ -25,6 +25,7 @@ import (
 	"strings"
 
 	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder/dockerfile/command"
 	"github.com/docker/docker/builder/dockerfile/parser"
 	"github.com/docker/docker/runconfig/opts"
@@ -64,19 +65,19 @@ type dispatchRequest struct {
 	attributes map[string]bool
 	flags      *BFlags
 	original   string
-	runConfig  *container.Config
 	shlex      *ShellLex
+	state      *dispatchState
 }
 
-func newDispatchRequestFromNode(node *parser.Node, builder *Builder, args []string, shlex *ShellLex) dispatchRequest {
+func newDispatchRequestFromOptions(options dispatchOptions, builder *Builder, args []string) dispatchRequest {
 	return dispatchRequest{
 		builder:    builder,
 		args:       args,
-		attributes: node.Attributes,
-		original:   node.Original,
-		flags:      NewBFlagsWithArgs(node.Flags),
-		runConfig:  builder.runConfig,
-		shlex:      shlex,
+		attributes: options.node.Attributes,
+		original:   options.node.Original,
+		flags:      NewBFlagsWithArgs(options.node.Flags),
+		shlex:      options.shlex,
+		state:      options.state,
 	}
 }
 
@@ -107,6 +108,10 @@ func init() {
 	}
 }
 
+func formatStep(stepN int, stepTotal int) string {
+	return fmt.Sprintf("%d/%d", stepN+1, stepTotal)
+}
+
 // This method is the entrypoint to all statement handling routines.
 //
 // Almost all nodes will have this structure:
@@ -121,117 +126,144 @@ func init() {
 // such as `RUN` in ONBUILD RUN foo. There is special case logic in here to
 // deal with that, at least until it becomes more of a general concern with new
 // features.
-func (b *Builder) dispatch(stepN int, stepTotal int, node *parser.Node, shlex *ShellLex) error {
+func (b *Builder) dispatch(options dispatchOptions) (*dispatchState, error) {
+	node := options.node
 	cmd := node.Value
 	upperCasedCmd := strings.ToUpper(cmd)
 
 	// To ensure the user is given a decent error message if the platform
 	// on which the daemon is running does not support a builder command.
 	if err := platformSupports(strings.ToLower(cmd)); err != nil {
-		return err
+		return nil, err
 	}
 
-	strList := []string{}
-	msg := bytes.NewBufferString(fmt.Sprintf("Step %d/%d : %s", stepN+1, stepTotal, upperCasedCmd))
-
-	if len(node.Flags) > 0 {
-		msg.WriteString(strings.Join(node.Flags, " "))
-	}
+	msg := bytes.NewBufferString(fmt.Sprintf("Step %s : %s%s",
+		options.stepMsg, upperCasedCmd, formatFlags(node.Flags)))
 
+	args := []string{}
 	ast := node
-	if cmd == "onbuild" {
-		if ast.Next == nil {
-			return errors.New("ONBUILD requires at least one argument")
+	if cmd == command.Onbuild {
+		var err error
+		ast, args, err = handleOnBuildNode(node, msg)
+		if err != nil {
+			return nil, err
 		}
-		ast = ast.Next.Children[0]
-		strList = append(strList, ast.Value)
-		msg.WriteString(" " + ast.Value)
+	}
 
-		if len(ast.Flags) > 0 {
-			msg.WriteString(" " + strings.Join(ast.Flags, " "))
-		}
+	runConfigEnv := options.state.runConfig.Env
+	envs := append(runConfigEnv, b.buildArgs.FilterAllowed(runConfigEnv)...)
+	processFunc := createProcessWordFunc(options.shlex, cmd, envs)
+	words, err := getDispatchArgsFromNode(ast, processFunc, msg)
+	if err != nil {
+		return nil, err
 	}
+	args = append(args, words...)
 
-	msgList := initMsgList(ast)
-	// Append build args to runConfig environment variables
-	envs := append(b.runConfig.Env, b.buildArgsWithoutConfigEnv()...)
+	fmt.Fprintln(b.Stdout, msg.String())
 
-	processFunc := getProcessFunc(shlex, cmd)
-	for i := 0; ast.Next != nil; i++ {
-		ast = ast.Next
-		words, err := processFunc(ast.Value, envs)
-		if err != nil {
-			return err
-		}
-		strList = append(strList, words...)
-		msgList[i] = ast.Value
+	f, ok := evaluateTable[cmd]
+	if !ok {
+		return nil, fmt.Errorf("unknown instruction: %s", upperCasedCmd)
 	}
+	if err := f(newDispatchRequestFromOptions(options, b, args)); err != nil {
+		return nil, err
+	}
+	options.state.updateRunConfig()
+	return options.state, nil
+}
 
-	msg.WriteString(" " + strings.Join(msgList, " "))
-	fmt.Fprintln(b.Stdout, msg.String())
+type dispatchOptions struct {
+	state   *dispatchState
+	stepMsg string
+	node    *parser.Node
+	shlex   *ShellLex
+}
 
-	// XXX yes, we skip any cmds that are not valid; the parser should have
-	// picked these out already.
-	if f, ok := evaluateTable[cmd]; ok {
-		if err := f(newDispatchRequestFromNode(node, b, strList, shlex)); err != nil {
-			return err
-		}
-		// TODO: return an object instead of setting things on builder
-		// If the step created a new image set it as the imageID for the
-		// current runConfig
-		b.runConfig.Image = b.image
-		return nil
+// dispatchState is a data object which is modified by dispatchers
+type dispatchState struct {
+	runConfig   *container.Config
+	maintainer  string
+	cmdSet      bool
+	noBaseImage bool
+	imageID     string
+	baseImage   builder.Image
+	stageName   string
+}
+
+func newDispatchState() *dispatchState {
+	return &dispatchState{runConfig: &container.Config{}}
+}
+
+func (r *dispatchState) updateRunConfig() {
+	r.runConfig.Image = r.imageID
+}
+
+// hasFromImage returns true if the builder has processed a `FROM <image>` line
+func (r *dispatchState) hasFromImage() bool {
+	return r.imageID != "" || r.noBaseImage
+}
+
+func (r *dispatchState) runConfigEnvMapping() map[string]string {
+	return opts.ConvertKVStringsToMap(r.runConfig.Env)
+}
+
+func (r *dispatchState) isCurrentStage(target string) bool {
+	if target == "" {
+		return false
 	}
+	return strings.EqualFold(r.stageName, target)
+}
 
-	return fmt.Errorf("Unknown instruction: %s", upperCasedCmd)
+func handleOnBuildNode(ast *parser.Node, msg *bytes.Buffer) (*parser.Node, []string, error) {
+	if ast.Next == nil {
+		return nil, nil, errors.New("ONBUILD requires at least one argument")
+	}
+	ast = ast.Next.Children[0]
+	msg.WriteString(" " + ast.Value + formatFlags(ast.Flags))
+	return ast, []string{ast.Value}, nil
 }
 
-// count the number of nodes that we are going to traverse first
-// allocation of those list a lot when they have a lot of arguments
-func initMsgList(cursor *parser.Node) []string {
-	var n int
-	for ; cursor.Next != nil; n++ {
-		cursor = cursor.Next
+func formatFlags(flags []string) string {
+	if len(flags) > 0 {
+		return " " + strings.Join(flags, " ")
 	}
-	return make([]string, n)
+	return ""
 }
 
-type processFunc func(string, []string) ([]string, error)
+func getDispatchArgsFromNode(ast *parser.Node, processFunc processWordFunc, msg *bytes.Buffer) ([]string, error) {
+	args := []string{}
+	for i := 0; ast.Next != nil; i++ {
+		ast = ast.Next
+		words, err := processFunc(ast.Value)
+		if err != nil {
+			return nil, err
+		}
+		args = append(args, words...)
+		msg.WriteString(" " + ast.Value)
+	}
+	return args, nil
+}
 
-func getProcessFunc(shlex *ShellLex, cmd string) processFunc {
+type processWordFunc func(string) ([]string, error)
+
+func createProcessWordFunc(shlex *ShellLex, cmd string, envs []string) processWordFunc {
 	switch {
 	case !replaceEnvAllowed[cmd]:
-		return func(word string, _ []string) ([]string, error) {
+		return func(word string) ([]string, error) {
 			return []string{word}, nil
 		}
 	case allowWordExpansion[cmd]:
-		return shlex.ProcessWords
+		return func(word string) ([]string, error) {
+			return shlex.ProcessWords(word, envs)
+		}
 	default:
-		return func(word string, envs []string) ([]string, error) {
+		return func(word string) ([]string, error) {
 			word, err := shlex.ProcessWord(word, envs)
 			return []string{word}, err
 		}
 	}
 }
 
-// buildArgsWithoutConfigEnv returns a list of key=value pairs for all the build
-// args that are not overriden by runConfig environment variables.
-func (b *Builder) buildArgsWithoutConfigEnv() []string {
-	envs := []string{}
-	configEnv := b.runConfigEnvMapping()
-
-	for key, val := range b.buildArgs.GetAllAllowed() {
-		if _, ok := configEnv[key]; !ok {
-			envs = append(envs, fmt.Sprintf("%s=%s", key, val))
-		}
-	}
-	return envs
-}
-
-func (b *Builder) runConfigEnvMapping() map[string]string {
-	return opts.ConvertKVStringsToMap(b.runConfig.Env)
-}
-
 // checkDispatch does a simple check for syntax errors of the Dockerfile.
 // Because some of the instructions can only be validated through runtime,
 // arg, env, etc., this syntax check will not be complete and could not replace

builder/dockerfile/evaluator_test.go

@@ -123,7 +123,7 @@ func initDispatchTestCases() []dispatchTestCase {
 		{
 			name:          "Invalid instruction",
 			dockerfile:    `foo bar`,
-			expectedError: "Unknown instruction: FOO",
+			expectedError: "unknown instruction: FOO",
 			files:         nil,
 		}}
 
@@ -177,13 +177,11 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) {
 		t.Fatalf("Error when parsing Dockerfile: %s", err)
 	}
 
-	config := &container.Config{}
 	options := &types.ImageBuildOptions{
 		BuildArgs: make(map[string]*string),
 	}
 
 	b := &Builder{
-		runConfig: config,
 		options:   options,
 		Stdout:    ioutil.Discard,
 		source:    context,
@@ -192,7 +190,14 @@ func executeTestCase(t *testing.T, testCase dispatchTestCase) {
 
 	shlex := NewShellLex(parser.DefaultEscapeToken)
 	n := result.AST
-	err = b.dispatch(0, len(n.Children), n.Children[0], shlex)
+	state := &dispatchState{runConfig: &container.Config{}}
+	opts := dispatchOptions{
+		state:   state,
+		stepMsg: formatStep(0, len(n.Children)),
+		node:    n.Children[0],
+		shlex:   shlex,
+	}
+	state, err = b.dispatch(opts)
 
 	if err == nil {
 		t.Fatalf("No error when executing test %s", testCase.name)

builder/dockerfile/imagecontext.go

@@ -19,11 +19,10 @@ type pathCache interface {
 // imageContexts is a helper for stacking up built image rootfs and reusing
 // them as contexts
 type imageContexts struct {
-	b           *Builder
-	list        []*imageMount
-	byName      map[string]*imageMount
-	cache       pathCache
-	currentName string
+	b      *Builder
+	list   []*imageMount
+	byName map[string]*imageMount
+	cache  pathCache
 }
 
 func (ic *imageContexts) newImageMount(id string) *imageMount {
@@ -41,7 +40,6 @@ func (ic *imageContexts) add(name string) (*imageMount, error) {
 		}
 		ic.byName[name] = im
 	}
-	ic.currentName = name
 	ic.list = append(ic.list, im)
 	return im, nil
 }
@@ -96,13 +94,6 @@ func (ic *imageContexts) unmount() (retErr error) {
 	return
 }
 
-func (ic *imageContexts) isCurrentTarget(target string) bool {
-	if target == "" {
-		return false
-	}
-	return strings.EqualFold(ic.currentName, target)
-}
-
 func (ic *imageContexts) getCache(id, path string) (interface{}, bool) {
 	if ic.cache != nil {
 		if id == "" {

builder/dockerfile/internals.go

@@ -35,16 +35,16 @@ import (
 	"github.com/pkg/errors"
 )
 
-func (b *Builder) commit(comment string) error {
+func (b *Builder) commit(dispatchState *dispatchState, comment string) error {
 	if b.disableCommit {
 		return nil
 	}
-	if !b.hasFromImage() {
+	if !dispatchState.hasFromImage() {
 		return errors.New("Please provide a source image with `from` prior to commit")
 	}
 
-	runConfigWithCommentCmd := copyRunConfig(b.runConfig, withCmdComment(comment))
-	hit, err := b.probeCache(b.image, runConfigWithCommentCmd)
+	runConfigWithCommentCmd := copyRunConfig(dispatchState.runConfig, withCmdComment(comment))
+	hit, err := b.probeCache(dispatchState, runConfigWithCommentCmd)
 	if err != nil || hit {
 		return err
 	}
@@ -53,20 +53,21 @@ func (b *Builder) commit(comment string) error {
 		return err
 	}
 
-	return b.commitContainer(id, runConfigWithCommentCmd)
+	return b.commitContainer(dispatchState, id, runConfigWithCommentCmd)
 }
 
-func (b *Builder) commitContainer(id string, containerConfig *container.Config) error {
+// TODO: see if any args can be dropped
+func (b *Builder) commitContainer(dispatchState *dispatchState, id string, containerConfig *container.Config) error {
 	if b.disableCommit {
 		return nil
 	}
 
 	commitCfg := &backend.ContainerCommitConfig{
 		ContainerCommitConfig: types.ContainerCommitConfig{
-			Author: b.maintainer,
+			Author: dispatchState.maintainer,
 			Pause:  true,
 			// TODO: this should be done by Commit()
-			Config: copyRunConfig(b.runConfig),
+			Config: copyRunConfig(dispatchState.runConfig),
 		},
 		ContainerConfig: containerConfig,
 	}
@@ -77,10 +78,8 @@ func (b *Builder) commitContainer(id string, containerConfig *container.Config)
 		return err
 	}
 
-	// TODO: this function should return imageID and runConfig instead of setting
-	// then on the builder
-	b.image = imageID
-	b.imageContexts.update(imageID, b.runConfig)
+	dispatchState.imageID = imageID
+	b.imageContexts.update(imageID, dispatchState.runConfig)
 	return nil
 }
 
@@ -91,7 +90,9 @@ type copyInfo struct {
 	decompress bool
 }
 
-func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalDecompression bool, cmdName string, imageSource *imageMount) error {
+// TODO: this needs to be split so that a Builder method doesn't accept req
+func (b *Builder) runContextCommand(req dispatchRequest, allowRemote bool, allowLocalDecompression bool, cmdName string, imageSource *imageMount) error {
+	args := req.args
 	if len(args) < 2 {
 		return fmt.Errorf("Invalid %s format - at least two arguments required", cmdName)
 	}
@@ -163,9 +164,9 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD
 
 	// TODO: should this have been using origPaths instead of srcHash in the comment?
 	runConfigWithCommentCmd := copyRunConfig(
-		b.runConfig,
+		req.state.runConfig,
 		withCmdCommentString(fmt.Sprintf("%s %s in %s ", cmdName, srcHash, dest)))
-	if hit, err := b.probeCache(b.image, runConfigWithCommentCmd); err != nil || hit {
+	if hit, err := b.probeCache(req.state, runConfigWithCommentCmd); err != nil || hit {
 		return err
 	}
 
@@ -181,7 +182,7 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD
 
 	// Twiddle the destination when it's a relative path - meaning, make it
 	// relative to the WORKINGDIR
-	if dest, err = normaliseDest(cmdName, b.runConfig.WorkingDir, dest); err != nil {
+	if dest, err = normaliseDest(cmdName, req.state.runConfig.WorkingDir, dest); err != nil {
 		return err
 	}
 
@@ -191,7 +192,7 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD
 		}
 	}
 
-	return b.commitContainer(container.ID, runConfigWithCommentCmd)
+	return b.commitContainer(req.state, container.ID, runConfigWithCommentCmd)
 }
 
 type runConfigModifier func(*container.Config)
@@ -479,20 +480,20 @@ func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression
 	return copyInfos, nil
 }
 
-func (b *Builder) processImageFrom(img builder.Image) error {
+func (b *Builder) processImageFrom(dispatchState *dispatchState, img builder.Image) error {
 	if img != nil {
-		b.image = img.ImageID()
+		dispatchState.imageID = img.ImageID()
 
 		if img.RunConfig() != nil {
-			b.runConfig = img.RunConfig()
+			dispatchState.runConfig = img.RunConfig()
 		}
 	}
 
 	// Check to see if we have a default PATH, note that windows won't
 	// have one as it's set by HCS
 	if system.DefaultPathEnv != "" {
-		if _, ok := b.runConfigEnvMapping()["PATH"]; !ok {
-			b.runConfig.Env = append(b.runConfig.Env,
+		if _, ok := dispatchState.runConfigEnvMapping()["PATH"]; !ok {
+			dispatchState.runConfig.Env = append(dispatchState.runConfig.Env,
 				"PATH="+system.DefaultPathEnv)
 		}
 	}
@@ -503,7 +504,7 @@ func (b *Builder) processImageFrom(img builder.Image) error {
 	}
 
 	// Process ONBUILD triggers if they exist
-	if nTriggers := len(b.runConfig.OnBuild); nTriggers != 0 {
+	if nTriggers := len(dispatchState.runConfig.OnBuild); nTriggers != 0 {
 		word := "trigger"
 		if nTriggers > 1 {
 			word = "triggers"
@@ -512,21 +513,21 @@ func (b *Builder) processImageFrom(img builder.Image) error {
 	}
 
 	// Copy the ONBUILD triggers, and remove them from the config, since the config will be committed.
-	onBuildTriggers := b.runConfig.OnBuild
-	b.runConfig.OnBuild = []string{}
+	onBuildTriggers := dispatchState.runConfig.OnBuild
+	dispatchState.runConfig.OnBuild = []string{}
 
 	// Reset stdin settings as all build actions run without stdin
-	b.runConfig.OpenStdin = false
-	b.runConfig.StdinOnce = false
+	dispatchState.runConfig.OpenStdin = false
+	dispatchState.runConfig.StdinOnce = false
 
 	// parse the ONBUILD triggers by invoking the parser
 	for _, step := range onBuildTriggers {
-		result, err := parser.Parse(strings.NewReader(step))
+		dockerfile, err := parser.Parse(strings.NewReader(step))
 		if err != nil {
 			return err
 		}
 
-		for _, n := range result.AST.Children {
+		for _, n := range dockerfile.AST.Children {
 			if err := checkDispatch(n); err != nil {
 				return err
 			}
@@ -540,7 +541,7 @@ func (b *Builder) processImageFrom(img builder.Image) error {
 			}
 		}
 
-		if err := dispatchFromDockerfile(b, result); err != nil {
+		if _, err := dispatchFromDockerfile(b, dockerfile, dispatchState); err != nil {
 			return err
 		}
 	}
@@ -551,12 +552,12 @@ func (b *Builder) processImageFrom(img builder.Image) error {
 // If an image is found, probeCache returns `(true, nil)`.
 // If no image is found, it returns `(false, nil)`.
 // If there is any error, it returns `(false, err)`.
-func (b *Builder) probeCache(parentID string, runConfig *container.Config) (bool, error) {
+func (b *Builder) probeCache(dispatchState *dispatchState, runConfig *container.Config) (bool, error) {
 	c := b.imageCache
 	if c == nil || b.options.NoCache || b.cacheBusted {
 		return false, nil
 	}
-	cache, err := c.GetCache(parentID, runConfig)
+	cache, err := c.GetCache(dispatchState.imageID, runConfig)
 	if err != nil {
 		return false, err
 	}
@@ -568,16 +569,13 @@ func (b *Builder) probeCache(parentID string, runConfig *container.Config) (bool
 
 	fmt.Fprint(b.Stdout, " ---> Using cache\n")
 	logrus.Debugf("[BUILDER] Use cached version: %s", runConfig.Cmd)
-	b.image = string(cache)
-	b.imageContexts.update(b.image, runConfig)
+	dispatchState.imageID = string(cache)
+	b.imageContexts.update(dispatchState.imageID, runConfig)
 
 	return true, nil
 }
 
 func (b *Builder) create(runConfig *container.Config) (string, error) {
-	if !b.hasFromImage() {
-		return "", errors.New("Please provide a source image with `from` prior to run")
-	}
 	resources := container.Resources{
 		CgroupParent: b.options.CgroupParent,
 		CPUShares:    b.options.CPUShares,