diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index 715161270d..f83a17ce33 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -315,10 +315,6 @@ func workdir(req dispatchRequest) error { return nil } - // TODO: why is this done here. This seems to be done at random places all over - // the builder - req.runConfig.Image = req.builder.image - comment := "WORKDIR " + req.runConfig.WorkingDir runConfigWithCommentCmd := copyRunConfig(req.runConfig, withCmdCommentString(comment)) if hit, err := req.builder.probeCache(req.builder.image, runConfigWithCommentCmd); err != nil || hit { @@ -372,9 +368,6 @@ func run(req dispatchRequest) error { saveCmd = prependEnvOnCmd(req.builder.buildArgs, buildArgs, cmdFromArgs) } - // TODO: this was previously in b.create(), why is it necessary? - req.runConfig.Image = req.builder.image - runConfigForCacheProbe := copyRunConfig(req.runConfig, withCmd(saveCmd)) hit, err := req.builder.probeCache(req.builder.image, runConfigForCacheProbe) if err != nil || hit { diff --git a/builder/dockerfile/evaluator.go b/builder/dockerfile/evaluator.go index abad15e096..e98a4e2f0d 100644 --- a/builder/dockerfile/evaluator.go +++ b/builder/dockerfile/evaluator.go @@ -173,7 +173,14 @@ func (b *Builder) dispatch(stepN int, stepTotal int, node *parser.Node, shlex *S // 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(newDispatchRequestFromNode(node, b, strList, shlex)) + 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 } return fmt.Errorf("Unknown instruction: %s", upperCasedCmd) diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index a772c615a8..ec411c4638 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -42,8 +42,6 @@ func (b *Builder) commit(comment string) error { if !b.hasFromImage() { return errors.New("Please provide a source image with `from` prior to commit") } - // TODO: why is this set here? - b.runConfig.Image = b.image runConfigWithCommentCmd := copyRunConfig(b.runConfig, withCmdComment(comment)) hit, err := b.probeCache(b.image, runConfigWithCommentCmd) @@ -100,10 +98,6 @@ func (b *Builder) runContextCommand(args []string, allowRemote bool, allowLocalD // Work in daemon-specific filepath semantics dest := filepath.FromSlash(args[len(args)-1]) // last one is always the dest - // TODO: why is this done here. This seems to be done at random places all over - // the builder - b.runConfig.Image = b.image - var infos []copyInfo // Loop through each src file and calculate the info we need to @@ -542,12 +536,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(imageID string, runConfig *container.Config) (bool, error) { +func (b *Builder) probeCache(parentID string, runConfig *container.Config) (bool, error) { c := b.imageCache if c == nil || b.options.NoCache || b.cacheBusted { return false, nil } - cache, err := c.GetCache(imageID, runConfig) + cache, err := c.GetCache(parentID, runConfig) if err != nil { return false, err }