Merge pull request #33454 from dnephin/refactor-builder-remove-copy-on-build

[Builder] Move file coping from the daemon to the builder

api/server/backend/build/backend.go

@@ -8,6 +8,7 @@ import (
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder/dockerfile"
 	"github.com/docker/docker/image"
+	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/pkg/errors"
 	"golang.org/x/net/context"
@@ -26,8 +27,8 @@ type Backend struct {
 }
 
 // NewBackend creates a new build backend from components
-func NewBackend(components ImageComponent, builderBackend builder.Backend) *Backend {
-	manager := dockerfile.NewBuildManager(builderBackend)
+func NewBackend(components ImageComponent, builderBackend builder.Backend, idMappings *idtools.IDMappings) *Backend {
+	manager := dockerfile.NewBuildManager(builderBackend, idMappings)
 	return &Backend{imageComponent: components, manager: manager}
 }
 

builder/builder.go

@@ -11,6 +11,7 @@ import (
 	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/api/types/container"
 	containerpkg "github.com/docker/docker/container"
+	"github.com/docker/docker/layer"
 	"golang.org/x/net/context"
 )
 
@@ -42,11 +43,7 @@ type Backend interface {
 	// ContainerCreateWorkdir creates the workdir
 	ContainerCreateWorkdir(containerID string) error
 
-	// ContainerCopy copies/extracts a source FileInfo to a destination path inside a container
-	// specified by a container object.
-	// TODO: extract in the builder instead of passing `decompress`
-	// TODO: use containerd/fs.changestream instead as a source
-	CopyOnBuild(containerID string, destPath string, srcRoot string, srcPath string, decompress bool) error
+	CreateImage(config []byte, parent string) (Image, error)
 
 	ImageCacheBuilder
 }
@@ -96,10 +93,13 @@ type ImageCache interface {
 type Image interface {
 	ImageID() string
 	RunConfig() *container.Config
+	MarshalJSON() ([]byte, error)
 }
 
 // ReleaseableLayer is an image layer that can be mounted and released
 type ReleaseableLayer interface {
 	Release() error
 	Mount() (string, error)
+	Commit() (ReleaseableLayer, error)
+	DiffID() layer.DiffID
 }

builder/dockerfile/builder.go

@@ -15,6 +15,9 @@ import (
 	"github.com/docker/docker/builder/dockerfile/command"
 	"github.com/docker/docker/builder/dockerfile/parser"
 	"github.com/docker/docker/builder/remotecontext"
+	"github.com/docker/docker/pkg/archive"
+	"github.com/docker/docker/pkg/chrootarchive"
+	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/streamformatter"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/pkg/errors"
@@ -37,15 +40,17 @@ var validCommitCommands = map[string]bool{
 
 // BuildManager is shared across all Builder objects
 type BuildManager struct {
+	archiver  *archive.Archiver
 	backend   builder.Backend
 	pathCache pathCache // TODO: make this persistent
 }
 
 // NewBuildManager creates a BuildManager
-func NewBuildManager(b builder.Backend) *BuildManager {
+func NewBuildManager(b builder.Backend, idMappings *idtools.IDMappings) *BuildManager {
 	return &BuildManager{
 		backend:   b,
 		pathCache: &syncmap.Map{},
+		archiver:  chrootarchive.NewArchiver(idMappings),
 	}
 }
 
@@ -73,6 +78,7 @@ func (bm *BuildManager) Build(ctx context.Context, config backend.BuildConfig) (
 		ProgressWriter: config.ProgressWriter,
 		Backend:        bm.backend,
 		PathCache:      bm.pathCache,
+		Archiver:       bm.archiver,
 	}
 	return newBuilder(ctx, builderOptions).build(source, dockerfile)
 }
@@ -83,6 +89,7 @@ type builderOptions struct {
 	Backend        builder.Backend
 	ProgressWriter backend.ProgressWriter
 	PathCache      pathCache
+	Archiver       *archive.Archiver
 }
 
 // Builder is a Dockerfile builder
@@ -98,6 +105,7 @@ type Builder struct {
 	docker    builder.Backend
 	clientCtx context.Context
 
+	archiver         *archive.Archiver
 	buildStages      *buildStages
 	disableCommit    bool
 	buildArgs        *buildArgs
@@ -121,6 +129,7 @@ func newBuilder(clientCtx context.Context, options builderOptions) *Builder {
 		Aux:              options.ProgressWriter.AuxFormatter,
 		Output:           options.ProgressWriter.Output,
 		docker:           options.Backend,
+		archiver:         options.Archiver,
 		buildArgs:        newBuildArgs(config.BuildArgs),
 		buildStages:      newBuildStages(),
 		imageSources:     newImageSources(clientCtx, options),

builder/dockerfile/copy.go

@@ -13,9 +13,12 @@ import (
 
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder/remotecontext"
+	"github.com/docker/docker/pkg/archive"
+	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/ioutils"
 	"github.com/docker/docker/pkg/progress"
 	"github.com/docker/docker/pkg/streamformatter"
+	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/pkg/urlutil"
 	"github.com/pkg/errors"
@@ -34,6 +37,10 @@ type copyInfo struct {
 	hash string
 }
 
+func (c copyInfo) fullPath() (string, error) {
+	return symlink.FollowSymlinkInScope(filepath.Join(c.root, c.path), c.root)
+}
+
 func newCopyInfoFromSource(source builder.Source, path string, hash string) copyInfo {
 	return copyInfo{root: source.Root(), path: path, hash: hash}
 }
@@ -148,7 +155,7 @@ func (o *copier) calcCopyInfo(origPath string, allowWildcards bool) ([]copyInfo,
 		var err error
 		o.source, err = imageSource.Source()
 		if err != nil {
-			return nil, errors.Wrapf(err, "failed to copy")
+			return nil, errors.Wrapf(err, "failed to copy from %s", imageSource.ImageID())
 		}
 	}
 
@@ -352,6 +359,83 @@ func downloadSource(output io.Writer, stdout io.Writer, srcURL string) (remote b
 		return
 	}
 
-	lc, err := remotecontext.NewLazyContext(tmpDir)
+	lc, err := remotecontext.NewLazySource(tmpDir)
 	return lc, filename, err
 }
+
+type copyFileOptions struct {
+	decompress bool
+	archiver   *archive.Archiver
+}
+
+func performCopyForInfo(dest copyInfo, source copyInfo, options copyFileOptions) error {
+	srcPath, err := source.fullPath()
+	if err != nil {
+		return err
+	}
+	destPath, err := dest.fullPath()
+	if err != nil {
+		return err
+	}
+
+	archiver := options.archiver
+
+	src, err := os.Stat(srcPath)
+	if err != nil {
+		return errors.Wrapf(err, "source path not found")
+	}
+	if src.IsDir() {
+		return copyDirectory(archiver, srcPath, destPath)
+	}
+	if options.decompress && archive.IsArchivePath(srcPath) {
+		return archiver.UntarPath(srcPath, destPath)
+	}
+
+	destExistsAsDir, err := isExistingDirectory(destPath)
+	if err != nil {
+		return err
+	}
+	// dest.path must be used because destPath has already been cleaned of any
+	// trailing slash
+	if endsInSlash(dest.path) || destExistsAsDir {
+		// source.path must be used to get the correct filename when the source
+		// is a symlink
+		destPath = filepath.Join(destPath, filepath.Base(source.path))
+	}
+	return copyFile(archiver, srcPath, destPath)
+}
+
+func copyDirectory(archiver *archive.Archiver, source, dest string) error {
+	if err := archiver.CopyWithTar(source, dest); err != nil {
+		return errors.Wrapf(err, "failed to copy directory")
+	}
+	return fixPermissions(source, dest, archiver.IDMappings.RootPair())
+}
+
+func copyFile(archiver *archive.Archiver, source, dest string) error {
+	rootIDs := archiver.IDMappings.RootPair()
+
+	if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest), 0755, rootIDs); err != nil {
+		return errors.Wrapf(err, "failed to create new directory")
+	}
+	if err := archiver.CopyFileWithTar(source, dest); err != nil {
+		return errors.Wrapf(err, "failed to copy file")
+	}
+	return fixPermissions(source, dest, rootIDs)
+}
+
+func endsInSlash(path string) bool {
+	return strings.HasSuffix(path, string(os.PathSeparator))
+}
+
+// isExistingDirectory returns true if the path exists and is a directory
+func isExistingDirectory(path string) (bool, error) {
+	destStat, err := os.Stat(path)
+	switch {
+	case os.IsNotExist(err):
+		return false, nil
+	case err != nil:
+		return false, err
+	}
+	return destStat.IsDir(), nil
+}

builder/dockerfile/copy_test.go

@@ -0,0 +1,45 @@
+package dockerfile
+
+import (
+	"testing"
+
+	"github.com/docker/docker/pkg/testutil/tempfile"
+	"github.com/stretchr/testify/assert"
+)
+
+func TestIsExistingDirectory(t *testing.T) {
+	tmpfile := tempfile.NewTempFile(t, "file-exists-test", "something")
+	defer tmpfile.Remove()
+	tmpdir := tempfile.NewTempDir(t, "dir-exists-test")
+	defer tmpdir.Remove()
+
+	var testcases = []struct {
+		doc      string
+		path     string
+		expected bool
+	}{
+		{
+			doc:      "directory exists",
+			path:     tmpdir.Path,
+			expected: true,
+		},
+		{
+			doc:      "path doesn't exist",
+			path:     "/bogus/path/does/not/exist",
+			expected: false,
+		},
+		{
+			doc:      "file exists",
+			path:     tmpfile.Name(),
+			expected: false,
+		},
+	}
+
+	for _, testcase := range testcases {
+		result, err := isExistingDirectory(testcase.path)
+		if !assert.NoError(t, err) {
+			continue
+		}
+		assert.Equal(t, testcase.expected, result, testcase.doc)
+	}
+}

builder/dockerfile/copy_unix.go

@@ -0,0 +1,36 @@
+// +build !windows
+
+package dockerfile
+
+import (
+	"os"
+	"path/filepath"
+
+	"github.com/docker/docker/pkg/idtools"
+)
+
+func fixPermissions(source, destination string, rootIDs idtools.IDPair) error {
+	skipChownRoot, err := isExistingDirectory(destination)
+	if err != nil {
+		return err
+	}
+
+	// We Walk on the source rather than on the destination because we don't
+	// want to change permissions on things we haven't created or modified.
+	return filepath.Walk(source, func(fullpath string, info os.FileInfo, err error) error {
+		// Do not alter the walk root iff. it existed before, as it doesn't fall under
+		// the domain of "things we should chown".
+		if skipChownRoot && source == fullpath {
+			return nil
+		}
+
+		// Path is prefixed by source: substitute with destination instead.
+		cleaned, err := filepath.Rel(source, fullpath)
+		if err != nil {
+			return err
+		}
+
+		fullpath = filepath.Join(destination, cleaned)
+		return os.Lchown(fullpath, rootIDs.UID, rootIDs.GID)
+	})
+}

builder/dockerfile/copy_windows.go

@@ -0,0 +1,8 @@
+package dockerfile
+
+import "github.com/docker/docker/pkg/idtools"
+
+func fixPermissions(source, destination string, rootIDs idtools.IDPair) error {
+	// chown is not supported on Windows
+	return nil
+}

builder/dockerfile/dispatchers.go

@@ -23,6 +23,7 @@ import (
 	"github.com/docker/docker/api/types/strslice"
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder/dockerfile/parser"
+	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/jsonmessage"
 	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/go-connections/nat"
@@ -251,10 +252,8 @@ func parseBuildStageName(args []string) (string, error) {
 	return stageName, nil
 }
 
-// scratchImage is used as a token for the empty base image. It uses buildStage
-// as a convenient implementation of builder.Image, but is not actually a
-// buildStage.
-var scratchImage builder.Image = &buildStage{}
+// scratchImage is used as a token for the empty base image.
+var scratchImage builder.Image = &image.Image{}
 
 func (b *Builder) getFromImage(shlex *ShellLex, name string) (builder.Image, error) {
 	substitutionArgs := []string{}
@@ -267,8 +266,8 @@ func (b *Builder) getFromImage(shlex *ShellLex, name string) (builder.Image, err
 		return nil, err
 	}
 
-	if im, ok := b.buildStages.getByName(name); ok {
-		return im, nil
+	if stage, ok := b.buildStages.getByName(name); ok {
+		name = stage.ImageID()
 	}
 
 	// Windows cannot support a container with no base image.

builder/dockerfile/imagecontext.go

@@ -6,37 +6,29 @@ import (
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/docker/api/types/backend"
-	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/builder/remotecontext"
+	dockerimage "github.com/docker/docker/image"
 	"github.com/pkg/errors"
 	"golang.org/x/net/context"
 )
 
 type buildStage struct {
-	id     string
-	config *container.Config
+	id string
 }
 
-func newBuildStageFromImage(image builder.Image) *buildStage {
-	return &buildStage{id: image.ImageID(), config: image.RunConfig()}
+func newBuildStage(imageID string) *buildStage {
+	return &buildStage{id: imageID}
 }
 
 func (b *buildStage) ImageID() string {
 	return b.id
 }
 
-func (b *buildStage) RunConfig() *container.Config {
-	return b.config
-}
-
-func (b *buildStage) update(imageID string, runConfig *container.Config) {
+func (b *buildStage) update(imageID string) {
 	b.id = imageID
-	b.config = runConfig
 }
 
-var _ builder.Image = &buildStage{}
-
 // buildStages tracks each stage of a build so they can be retrieved by index
 // or by name.
 type buildStages struct {
@@ -48,12 +40,12 @@ func newBuildStages() *buildStages {
 	return &buildStages{byName: make(map[string]*buildStage)}
 }
 
-func (s *buildStages) getByName(name string) (builder.Image, bool) {
+func (s *buildStages) getByName(name string) (*buildStage, bool) {
 	stage, ok := s.byName[strings.ToLower(name)]
 	return stage, ok
 }
 
-func (s *buildStages) get(indexOrName string) (builder.Image, error) {
+func (s *buildStages) get(indexOrName string) (*buildStage, error) {
 	index, err := strconv.Atoi(indexOrName)
 	if err == nil {
 		if err := s.validateIndex(index); err != nil {
@@ -78,7 +70,7 @@ func (s *buildStages) validateIndex(i int) error {
 }
 
 func (s *buildStages) add(name string, image builder.Image) error {
-	stage := newBuildStageFromImage(image)
+	stage := newBuildStage(image.ImageID())
 	name = strings.ToLower(name)
 	if len(name) > 0 {
 		if _, ok := s.byName[name]; ok {
@@ -90,8 +82,8 @@ func (s *buildStages) add(name string, image builder.Image) error {
 	return nil
 }
 
-func (s *buildStages) update(imageID string, runConfig *container.Config) {
-	s.sequence[len(s.sequence)-1].update(imageID, runConfig)
+func (s *buildStages) update(imageID string) {
+	s.sequence[len(s.sequence)-1].update(imageID)
 }
 
 type getAndMountFunc func(string) (builder.Image, builder.ReleaseableLayer, error)
@@ -100,6 +92,7 @@ type getAndMountFunc func(string) (builder.Image, builder.ReleaseableLayer, erro
 // all images so they can be unmounted at the end of the build.
 type imageSources struct {
 	byImageID map[string]*imageMount
+	withoutID []*imageMount
 	getImage  getAndMountFunc
 	cache     pathCache // TODO: remove
 }
@@ -129,7 +122,7 @@ func (m *imageSources) Get(idOrRef string) (*imageMount, error) {
 		return nil, err
 	}
 	im := newImageMount(image, layer)
-	m.byImageID[image.ImageID()] = im
+	m.Add(im)
 	return im, nil
 }
 
@@ -140,9 +133,25 @@ func (m *imageSources) Unmount() (retErr error) {
 			retErr = err
 		}
 	}
+	for _, im := range m.withoutID {
+		if err := im.unmount(); err != nil {
+			logrus.Error(err)
+			retErr = err
+		}
+	}
 	return
 }
 
+func (m *imageSources) Add(im *imageMount) {
+	switch im.image {
+	case nil:
+		im.image = &dockerimage.Image{}
+		m.withoutID = append(m.withoutID, im)
+	default:
+		m.byImageID[im.image.ImageID()] = im
+	}
+}
+
 // imageMount is a reference to an image that can be used as a builder.Source
 type imageMount struct {
 	image  builder.Image
@@ -164,7 +173,7 @@ func (im *imageMount) Source() (builder.Source, error) {
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to mount %s", im.image.ImageID())
 		}
-		source, err := remotecontext.NewLazyContext(mountPath)
+		source, err := remotecontext.NewLazySource(mountPath)
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to create lazycontext for %s", mountPath)
 		}
@@ -180,6 +189,7 @@ func (im *imageMount) unmount() error {
 	if err := im.layer.Release(); err != nil {
 		return errors.Wrapf(err, "failed to unmount previous build image %s", im.image.ImageID())
 	}
+	im.layer = nil
 	return nil
 }
 
@@ -187,6 +197,10 @@ func (im *imageMount) Image() builder.Image {
 	return im.image
 }
 
+func (im *imageMount) Layer() builder.ReleaseableLayer {
+	return im.layer
+}
+
 func (im *imageMount) ImageID() string {
 	return im.image.ImageID()
 }

builder/dockerfile/internals.go

@@ -12,6 +12,7 @@ import (
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/backend"
 	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/pkg/errors"
 )
@@ -37,7 +38,6 @@ func (b *Builder) commit(dispatchState *dispatchState, comment string) error {
 	return b.commitContainer(dispatchState, id, runConfigWithCommentCmd)
 }
 
-// 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
@@ -60,7 +60,47 @@ func (b *Builder) commitContainer(dispatchState *dispatchState, id string, conta
 	}
 
 	dispatchState.imageID = imageID
-	b.buildStages.update(imageID, dispatchState.runConfig)
+	b.buildStages.update(imageID)
+	return nil
+}
+
+func (b *Builder) exportImage(state *dispatchState, imageMount *imageMount, runConfig *container.Config) error {
+	newLayer, err := imageMount.Layer().Commit()
+	if err != nil {
+		return err
+	}
+
+	// add an image mount without an image so the layer is properly unmounted
+	// if there is an error before we can add the full mount with image
+	b.imageSources.Add(newImageMount(nil, newLayer))
+
+	parentImage, ok := imageMount.Image().(*image.Image)
+	if !ok {
+		return errors.Errorf("unexpected image type")
+	}
+
+	newImage := image.NewChildImage(parentImage, image.ChildConfig{
+		Author:          state.maintainer,
+		ContainerConfig: runConfig,
+		DiffID:          newLayer.DiffID(),
+		Config:          copyRunConfig(state.runConfig),
+	})
+
+	// TODO: it seems strange to marshal this here instead of just passing in the
+	// image struct
+	config, err := newImage.MarshalJSON()
+	if err != nil {
+		return errors.Wrap(err, "failed to encode image config")
+	}
+
+	exportedImage, err := b.docker.CreateImage(config, state.imageID)
+	if err != nil {
+		return errors.Wrapf(err, "failed to export image")
+	}
+
+	state.imageID = exportedImage.ImageID()
+	b.imageSources.Add(newImageMount(exportedImage, newLayer))
+	b.buildStages.update(state.imageID)
 	return nil
 }
 
@@ -71,24 +111,46 @@ func (b *Builder) performCopy(state *dispatchState, inst copyInstruction) error
 	runConfigWithCommentCmd := copyRunConfig(
 		state.runConfig,
 		withCmdCommentString(fmt.Sprintf("%s %s in %s ", inst.cmdName, srcHash, inst.dest)))
-	containerID, err := b.probeAndCreate(state, runConfigWithCommentCmd)
-	if err != nil || containerID == "" {
+	hit, err := b.probeCache(state, runConfigWithCommentCmd)
+	if err != nil || hit {
 		return err
 	}
 
-	// Twiddle the destination when it's a relative path - meaning, make it
-	// relative to the WORKINGDIR
-	dest, err := normaliseDest(inst.cmdName, state.runConfig.WorkingDir, inst.dest)
+	imageMount, err := b.imageSources.Get(state.imageID)
+	if err != nil {
+		return errors.Wrapf(err, "failed to get destination image %q", state.imageID)
+	}
+	destInfo, err := createDestInfo(state.runConfig.WorkingDir, inst, imageMount)
 	if err != nil {
 		return err
 	}
 
+	opts := copyFileOptions{
+		decompress: inst.allowLocalDecompression,
+		archiver:   b.archiver,
+	}
 	for _, info := range inst.infos {
-		if err := b.docker.CopyOnBuild(containerID, dest, info.root, info.path, inst.allowLocalDecompression); err != nil {
-			return err
+		if err := performCopyForInfo(destInfo, info, opts); err != nil {
+			return errors.Wrapf(err, "failed to copy files")
 		}
 	}
-	return b.commitContainer(state, containerID, runConfigWithCommentCmd)
+	return b.exportImage(state, imageMount, runConfigWithCommentCmd)
+}
+
+func createDestInfo(workingDir string, inst copyInstruction, imageMount *imageMount) (copyInfo, error) {
+	// Twiddle the destination when it's a relative path - meaning, make it
+	// relative to the WORKINGDIR
+	dest, err := normaliseDest(workingDir, inst.dest)
+	if err != nil {
+		return copyInfo{}, errors.Wrapf(err, "invalid %s", inst.cmdName)
+	}
+
+	destMount, err := imageMount.Source()
+	if err != nil {
+		return copyInfo{}, errors.Wrapf(err, "failed to mount copy source")
+	}
+
+	return newCopyInfoFromSource(destMount, dest, ""), nil
 }
 
 // For backwards compat, if there's just one info then use it as the
@@ -182,7 +244,7 @@ func (b *Builder) probeCache(dispatchState *dispatchState, runConfig *container.
 	fmt.Fprint(b.Stdout, " ---> Using cache\n")
 
 	dispatchState.imageID = string(cachedID)
-	b.buildStages.update(dispatchState.imageID, runConfig)
+	b.buildStages.update(dispatchState.imageID)
 	return true, nil
 }
 

builder/dockerfile/internals_unix.go

@@ -12,7 +12,7 @@ import (
 
 // normaliseDest normalises the destination of a COPY/ADD command in a
 // platform semantically consistent way.
-func normaliseDest(cmdName, workingDir, requested string) (string, error) {
+func normaliseDest(workingDir, requested string) (string, error) {
 	dest := filepath.FromSlash(requested)
 	endsInSlash := strings.HasSuffix(requested, string(os.PathSeparator))
 	if !system.IsAbs(requested) {

builder/dockerfile/internals_windows.go

@@ -12,7 +12,7 @@ import (
 
 // normaliseDest normalises the destination of a COPY/ADD command in a
 // platform semantically consistent way.
-func normaliseDest(cmdName, workingDir, requested string) (string, error) {
+func normaliseDest(workingDir, requested string) (string, error) {
 	dest := filepath.FromSlash(requested)
 	endsInSlash := strings.HasSuffix(dest, string(os.PathSeparator))
 
@@ -32,7 +32,7 @@ func normaliseDest(cmdName, workingDir, requested string) (string, error) {
 	// we only want to validate where the DriveColon part has been supplied.
 	if filepath.IsAbs(dest) {
 		if strings.ToUpper(string(dest[0])) != "C" {
-			return "", fmt.Errorf("Windows does not support %s with a destinations not on the system drive (C:)", cmdName)
+			return "", fmt.Errorf("Windows does not support destinations not on the system drive (C:)")
 		}
 		dest = dest[2:] // Strip the drive letter
 	}
@@ -44,7 +44,7 @@ func normaliseDest(cmdName, workingDir, requested string) (string, error) {
 		}
 		if !system.IsAbs(dest) {
 			if string(workingDir[0]) != "C" {
-				return "", fmt.Errorf("Windows does not support %s with relative paths when WORKDIR is not the system drive", cmdName)
+				return "", fmt.Errorf("Windows does not support relative paths when WORKDIR is not the system drive")
 			}
 			dest = filepath.Join(string(os.PathSeparator), workingDir[2:], dest)
 			// Make sure we preserve any trailing slash

builder/dockerfile/internals_windows_test.go

@@ -2,16 +2,22 @@
 
 package dockerfile
 
-import "testing"
+import (
+	"fmt"
+	"testing"
+
+	"github.com/docker/docker/pkg/testutil"
+	"github.com/stretchr/testify/assert"
+)
 
 func TestNormaliseDest(t *testing.T) {
 	tests := []struct{ current, requested, expected, etext string }{
-		{``, `D:\`, ``, `Windows does not support TEST with a destinations not on the system drive (C:)`},
-		{``, `e:/`, ``, `Windows does not support TEST with a destinations not on the system drive (C:)`},
+		{``, `D:\`, ``, `Windows does not support destinations not on the system drive (C:)`},
+		{``, `e:/`, ``, `Windows does not support destinations not on the system drive (C:)`},
 		{`invalid`, `./c1`, ``, `Current WorkingDir invalid is not platform consistent`},
 		{`C:`, ``, ``, `Current WorkingDir C: is not platform consistent`},
 		{`C`, ``, ``, `Current WorkingDir C is not platform consistent`},
-		{`D:\`, `.`, ``, "Windows does not support TEST with relative paths when WORKDIR is not the system drive"},
+		{`D:\`, `.`, ``, "Windows does not support relative paths when WORKDIR is not the system drive"},
 		{``, `D`, `D`, ``},
 		{``, `./a1`, `.\a1`, ``},
 		{``, `.\b1`, `.\b1`, ``},
@@ -32,20 +38,16 @@ func TestNormaliseDest(t *testing.T) {
 		{`C:\wdm`, `foo/bar/`, `\wdm\foo\bar\`, ``},
 		{`C:\wdn`, `foo\bar/`, `\wdn\foo\bar\`, ``},
 	}
-	for _, i := range tests {
-		got, err := normaliseDest("TEST", i.current, i.requested)
-		if err != nil && i.etext == "" {
-			t.Fatalf("TestNormaliseDest Got unexpected error %q for %s %s. ", err.Error(), i.current, i.requested)
-		}
-		if i.etext != "" && ((err == nil) || (err != nil && err.Error() != i.etext)) {
-			if err == nil {
-				t.Fatalf("TestNormaliseDest Expected an error for %s %s but didn't get one", i.current, i.requested)
-			} else {
-				t.Fatalf("TestNormaliseDest Wrong error text for %s %s - %s", i.current, i.requested, err.Error())
+	for _, testcase := range tests {
+		msg := fmt.Sprintf("Input: %s, %s", testcase.current, testcase.requested)
+		actual, err := normaliseDest(testcase.current, testcase.requested)
+		if testcase.etext == "" {
+			if !assert.NoError(t, err, msg) {
+				continue
 			}
-		}
-		if i.etext == "" && got != i.expected {
-			t.Fatalf("TestNormaliseDest Expected %q for %q and %q. Got %q", i.expected, i.current, i.requested, got)
+			assert.Equal(t, testcase.expected, actual, msg)
+		} else {
+			testutil.ErrorContains(t, err, testcase.etext)
 		}
 	}
 }

builder/dockerfile/mockbackend_test.go

@@ -1,6 +1,7 @@
 package dockerfile
 
 import (
+	"encoding/json"
 	"io"
 
 	"github.com/docker/docker/api/types"
@@ -8,6 +9,7 @@ import (
 	"github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder"
 	containerpkg "github.com/docker/docker/container"
+	"github.com/docker/docker/layer"
 	"golang.org/x/net/context"
 )
 
@@ -76,6 +78,10 @@ func (m *MockBackend) MakeImageCache(cacheFrom []string) builder.ImageCache {
 	return nil
 }
 
+func (m *MockBackend) CreateImage(config []byte, parent string) (builder.Image, error) {
+	return nil, nil
+}
+
 type mockImage struct {
 	id     string
 	config *container.Config
@@ -89,6 +95,11 @@ func (i *mockImage) RunConfig() *container.Config {
 	return i.config
 }
 
+func (i *mockImage) MarshalJSON() ([]byte, error) {
+	type rawImage mockImage
+	return json.Marshal(rawImage(*i))
+}
+
 type mockImageCache struct {
 	getCacheFunc func(parentID string, cfg *container.Config) (string, error)
 }
@@ -109,3 +120,11 @@ func (l *mockLayer) Release() error {
 func (l *mockLayer) Mount() (string, error) {
 	return "mountPath", nil
 }
+
+func (l *mockLayer) Commit() (builder.ReleaseableLayer, error) {
+	return nil, nil
+}
+
+func (l *mockLayer) DiffID() layer.DiffID {
+	return layer.DiffID("abcdef")
+}

builder/remotecontext/detect.go

@@ -81,7 +81,7 @@ func withDockerfileFromContext(c modifiableContext, dockerfilePath string) (buil
 }
 
 func newGitRemote(gitURL string, dockerfilePath string) (builder.Source, *parser.Result, error) {
-	c, err := MakeGitContext(gitURL) // TODO: change this to NewLazyContext
+	c, err := MakeGitContext(gitURL) // TODO: change this to NewLazySource
 	if err != nil {
 		return nil, nil, err
 	}

builder/remotecontext/lazycontext.go

@@ -12,30 +12,30 @@ import (
 	"github.com/pkg/errors"
 )
 
-// NewLazyContext creates a new LazyContext. LazyContext defines a hashed build
+// NewLazySource creates a new LazyContext. LazyContext defines a hashed build
 // context based on a root directory. Individual files are hashed first time
 // they are asked. It is not safe to call methods of LazyContext concurrently.
-func NewLazyContext(root string) (builder.Source, error) {
-	return &lazyContext{
+func NewLazySource(root string) (builder.Source, error) {
+	return &lazySource{
 		root: root,
 		sums: make(map[string]string),
 	}, nil
 }
 
-type lazyContext struct {
+type lazySource struct {
 	root string
 	sums map[string]string
 }
 
-func (c *lazyContext) Root() string {
+func (c *lazySource) Root() string {
 	return c.root
 }
 
-func (c *lazyContext) Close() error {
+func (c *lazySource) Close() error {
 	return nil
 }
 
-func (c *lazyContext) Hash(path string) (string, error) {
+func (c *lazySource) Hash(path string) (string, error) {
 	cleanPath, fullPath, err := normalize(path, c.root)
 	if err != nil {
 		return "", err
@@ -62,7 +62,7 @@ func (c *lazyContext) Hash(path string) (string, error) {
 	return sum, nil
 }
 
-func (c *lazyContext) prepareHash(relPath string, fi os.FileInfo) (string, error) {
+func (c *lazySource) prepareHash(relPath string, fi os.FileInfo) (string, error) {
 	p := filepath.Join(c.root, relPath)
 	h, err := NewFileHash(p, relPath, fi)
 	if err != nil {

cmd/dockerd/daemon.go

@@ -453,7 +453,7 @@ func initRouter(s *apiserver.Server, d *daemon.Daemon, c *cluster.Cluster) {
 		image.NewRouter(d, decoder),
 		systemrouter.NewRouter(d, c),
 		volume.NewRouter(d),
-		build.NewRouter(buildbackend.NewBackend(d, d), d),
+		build.NewRouter(buildbackend.NewBackend(d, d, d.IDMappings()), d),
 		swarmrouter.NewRouter(c),
 		pluginrouter.NewRouter(d.PluginManager()),
 		distributionrouter.NewRouter(d),

daemon/archive.go

@@ -10,9 +10,7 @@ import (
 	"github.com/docker/docker/container"
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/chrootarchive"
-	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/ioutils"
-	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/pkg/system"
 	"github.com/pkg/errors"
 )
@@ -362,103 +360,3 @@ func (daemon *Daemon) containerCopy(container *container.Container, resource str
 	daemon.LogContainerEvent(container, "copy")
 	return reader, nil
 }
-
-// CopyOnBuild copies/extracts a source FileInfo to a destination path inside a container
-// specified by a container object.
-// TODO: make sure callers don't unnecessarily convert destPath with filepath.FromSlash (Copy does it already).
-// CopyOnBuild should take in abstract paths (with slashes) and the implementation should convert it to OS-specific paths.
-func (daemon *Daemon) CopyOnBuild(cID, destPath, srcRoot, srcPath string, decompress bool) error {
-	fullSrcPath, err := symlink.FollowSymlinkInScope(filepath.Join(srcRoot, srcPath), srcRoot)
-	if err != nil {
-		return err
-	}
-
-	destExists := true
-	destDir := false
-	rootIDs := daemon.idMappings.RootPair()
-
-	// Work in daemon-local OS specific file paths
-	destPath = filepath.FromSlash(destPath)
-
-	c, err := daemon.GetContainer(cID)
-	if err != nil {
-		return err
-	}
-	err = daemon.Mount(c)
-	if err != nil {
-		return err
-	}
-	defer daemon.Unmount(c)
-
-	dest, err := c.GetResourcePath(destPath)
-	if err != nil {
-		return err
-	}
-
-	// Preserve the trailing slash
-	// TODO: why are we appending another path separator if there was already one?
-	if strings.HasSuffix(destPath, string(os.PathSeparator)) || destPath == "." {
-		destDir = true
-		dest += string(os.PathSeparator)
-	}
-
-	destPath = dest
-
-	destStat, err := os.Stat(destPath)
-	if err != nil {
-		if !os.IsNotExist(err) {
-			//logrus.Errorf("Error performing os.Stat on %s. %s", destPath, err)
-			return err
-		}
-		destExists = false
-	}
-
-	archiver := chrootarchive.NewArchiver(daemon.idMappings)
-	src, err := os.Stat(fullSrcPath)
-	if err != nil {
-		return err
-	}
-
-	if src.IsDir() {
-		// copy as directory
-		if err := archiver.CopyWithTar(fullSrcPath, destPath); err != nil {
-			return err
-		}
-		return fixPermissions(fullSrcPath, destPath, rootIDs.UID, rootIDs.GID, destExists)
-	}
-	if decompress && archive.IsArchivePath(fullSrcPath) {
-		// Only try to untar if it is a file and that we've been told to decompress (when ADD-ing a remote file)
-
-		// First try to unpack the source as an archive
-		// to support the untar feature we need to clean up the path a little bit
-		// because tar is very forgiving.  First we need to strip off the archive's
-		// filename from the path but this is only added if it does not end in slash
-		tarDest := destPath
-		if strings.HasSuffix(tarDest, string(os.PathSeparator)) {
-			tarDest = filepath.Dir(destPath)
-		}
-
-		// try to successfully untar the orig
-		err := archiver.UntarPath(fullSrcPath, tarDest)
-		/*
-			if err != nil {
-				logrus.Errorf("Couldn't untar to %s: %v", tarDest, err)
-			}
-		*/
-		return err
-	}
-
-	// only needed for fixPermissions, but might as well put it before CopyFileWithTar
-	if destDir || (destExists && destStat.IsDir()) {
-		destPath = filepath.Join(destPath, filepath.Base(srcPath))
-	}
-
-	if err := idtools.MkdirAllAndChownNew(filepath.Dir(destPath), 0755, rootIDs); err != nil {
-		return err
-	}
-	if err := archiver.CopyFileWithTar(fullSrcPath, destPath); err != nil {
-		return err
-	}
-
-	return fixPermissions(fullSrcPath, destPath, rootIDs.UID, rootIDs.GID, destExists)
-}

daemon/archive_unix.go

@@ -3,9 +3,6 @@
 package daemon
 
 import (
-	"os"
-	"path/filepath"
-
 	"github.com/docker/docker/container"
 )
 
@@ -25,38 +22,6 @@ func checkIfPathIsInAVolume(container *container.Container, absPath string) (boo
 	return toVolume, nil
 }
 
-func fixPermissions(source, destination string, uid, gid int, destExisted bool) error {
-	// If the destination didn't already exist, or the destination isn't a
-	// directory, then we should Lchown the destination. Otherwise, we shouldn't
-	// Lchown the destination.
-	destStat, err := os.Stat(destination)
-	if err != nil {
-		// This should *never* be reached, because the destination must've already
-		// been created while untar-ing the context.
-		return err
-	}
-	doChownDestination := !destExisted || !destStat.IsDir()
-
-	// We Walk on the source rather than on the destination because we don't
-	// want to change permissions on things we haven't created or modified.
-	return filepath.Walk(source, func(fullpath string, info os.FileInfo, err error) error {
-		// Do not alter the walk root iff. it existed before, as it doesn't fall under
-		// the domain of "things we should chown".
-		if !doChownDestination && (source == fullpath) {
-			return nil
-		}
-
-		// Path is prefixed by source: substitute with destination instead.
-		cleaned, err := filepath.Rel(source, fullpath)
-		if err != nil {
-			return err
-		}
-
-		fullpath = filepath.Join(destination, cleaned)
-		return os.Lchown(fullpath, uid, gid)
-	})
-}
-
 // isOnlineFSOperationPermitted returns an error if an online filesystem operation
 // is not permitted.
 func (daemon *Daemon) isOnlineFSOperationPermitted(container *container.Container) error {

daemon/archive_windows.go

@@ -17,11 +17,6 @@ func checkIfPathIsInAVolume(container *container.Container, absPath string) (boo
 	return false, nil
 }
 
-func fixPermissions(source, destination string, uid, gid int, destExisted bool) error {
-	// chown is not supported on Windows
-	return nil
-}
-
 // isOnlineFSOperationPermitted returns an error if an online filesystem operation
 // is not permitted (such as stat or for copying). Running Hyper-V containers
 // cannot have their file-system interrogated from the host as the filter is

daemon/build.go

@@ -1,6 +1,8 @@
 package daemon
 
 import (
+	"io"
+
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/docker/api/types"
@@ -8,11 +10,11 @@ import (
 	"github.com/docker/docker/builder"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
+	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/stringid"
 	"github.com/docker/docker/registry"
 	"github.com/pkg/errors"
 	"golang.org/x/net/context"
-	"io"
 )
 
 type releaseableLayer struct {
@@ -22,12 +24,14 @@ type releaseableLayer struct {
 }
 
 func (rl *releaseableLayer) Mount() (string, error) {
-	if rl.roLayer == nil {
-		return "", errors.New("can not mount an image with no root FS")
-	}
 	var err error
+	var chainID layer.ChainID
+	if rl.roLayer != nil {
+		chainID = rl.roLayer.ChainID()
+	}
+
 	mountID := stringid.GenerateRandomID()
-	rl.rwLayer, err = rl.layerStore.CreateRWLayer(mountID, rl.roLayer.ChainID(), nil)
+	rl.rwLayer, err = rl.layerStore.CreateRWLayer(mountID, chainID, nil)
 	if err != nil {
 		return "", errors.Wrap(err, "failed to create rwlayer")
 	}
@@ -35,6 +39,36 @@ func (rl *releaseableLayer) Mount() (string, error) {
 	return rl.rwLayer.Mount("")
 }
 
+func (rl *releaseableLayer) Commit() (builder.ReleaseableLayer, error) {
+	var chainID layer.ChainID
+	if rl.roLayer != nil {
+		chainID = rl.roLayer.ChainID()
+	}
+
+	stream, err := rl.rwLayer.TarStream()
+	if err != nil {
+		return nil, err
+	}
+
+	newLayer, err := rl.layerStore.Register(stream, chainID)
+	if err != nil {
+		return nil, err
+	}
+
+	if layer.IsEmpty(newLayer.DiffID()) {
+		_, err := rl.layerStore.Release(newLayer)
+		return &releaseableLayer{layerStore: rl.layerStore}, err
+	}
+	return &releaseableLayer{layerStore: rl.layerStore, roLayer: newLayer}, nil
+}
+
+func (rl *releaseableLayer) DiffID() layer.DiffID {
+	if rl.roLayer == nil {
+		return layer.DigestSHA256EmptyTar
+	}
+	return rl.roLayer.DiffID()
+}
+
 func (rl *releaseableLayer) Release() error {
 	rl.releaseRWLayer()
 	return rl.releaseROLayer()
@@ -62,8 +96,8 @@ func (rl *releaseableLayer) releaseROLayer() error {
 }
 
 func newReleasableLayerForImage(img *image.Image, layerStore layer.Store) (builder.ReleaseableLayer, error) {
-	if img.RootFS.ChainID() == "" {
-		return nil, nil
+	if img == nil || img.RootFS.ChainID() == "" {
+		return &releaseableLayer{layerStore: layerStore}, nil
 	}
 	// Hold a reference to the image layer so that it can't be removed before
 	// it is released
@@ -104,6 +138,11 @@ func (daemon *Daemon) pullForBuilder(ctx context.Context, name string, authConfi
 // Every call to GetImageAndReleasableLayer MUST call releasableLayer.Release() to prevent
 // leaking of layers.
 func (daemon *Daemon) GetImageAndReleasableLayer(ctx context.Context, refOrID string, opts backend.GetImageAndLayerOptions) (builder.Image, builder.ReleaseableLayer, error) {
+	if refOrID == "" {
+		layer, err := newReleasableLayerForImage(nil, daemon.layerStore)
+		return nil, layer, err
+	}
+
 	if !opts.ForcePull {
 		image, _ := daemon.GetImage(refOrID)
 		// TODO: shouldn't we error out if error is different from "not found" ?
@@ -120,3 +159,26 @@ func (daemon *Daemon) GetImageAndReleasableLayer(ctx context.Context, refOrID st
 	layer, err := newReleasableLayerForImage(image, daemon.layerStore)
 	return image, layer, err
 }
+
+// CreateImage creates a new image by adding a config and ID to the image store.
+// This is similar to LoadImage() except that it receives JSON encoded bytes of
+// an image instead of a tar archive.
+func (daemon *Daemon) CreateImage(config []byte, parent string) (builder.Image, error) {
+	id, err := daemon.imageStore.Create(config)
+	if err != nil {
+		return nil, errors.Wrapf(err, "failed to create image")
+	}
+
+	if parent != "" {
+		if err := daemon.imageStore.SetParent(id, image.ID(parent)); err != nil {
+			return nil, errors.Wrapf(err, "failed to set parent %s", parent)
+		}
+	}
+
+	return daemon.imageStore.Get(id)
+}
+
+// IDMappings returns uid/gid mappings for the builder
+func (daemon *Daemon) IDMappings() *idtools.IDMappings {
+	return daemon.idMappings
+}

daemon/commit.go

@@ -12,7 +12,6 @@ import (
 	containertypes "github.com/docker/docker/api/types/container"
 	"github.com/docker/docker/builder/dockerfile"
 	"github.com/docker/docker/container"
-	"github.com/docker/docker/dockerversion"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/layer"
 	"github.com/docker/docker/pkg/ioutils"
@@ -129,11 +128,6 @@ func (daemon *Daemon) Commit(name string, c *backend.ContainerCommitConfig) (str
 		return "", err
 	}
 
-	containerConfig := c.ContainerConfig
-	if containerConfig == nil {
-		containerConfig = container.Config
-	}
-
 	// It is not possible to commit a running container on Windows and on Solaris.
 	if (runtime.GOOS == "windows" || runtime.GOOS == "solaris") && container.IsRunning() {
 		return "", errors.Errorf("%+v does not support commit of a running container", runtime.GOOS)
@@ -165,60 +159,36 @@ func (daemon *Daemon) Commit(name string, c *backend.ContainerCommitConfig) (str
 		}
 	}()
 
-	var history []image.History
-	rootFS := image.NewRootFS()
-	osVersion := ""
-	var osFeatures []string
-
-	if container.ImageID != "" {
-		img, err := daemon.imageStore.Get(container.ImageID)
+	var parent *image.Image
+	if container.ImageID == "" {
+		parent = new(image.Image)
+		parent.RootFS = image.NewRootFS()
+	} else {
+		parent, err = daemon.imageStore.Get(container.ImageID)
 		if err != nil {
 			return "", err
 		}
-		history = img.History
-		rootFS = img.RootFS
-		osVersion = img.OSVersion
-		osFeatures = img.OSFeatures
 	}
 
-	l, err := daemon.layerStore.Register(rwTar, rootFS.ChainID())
+	l, err := daemon.layerStore.Register(rwTar, parent.RootFS.ChainID())
 	if err != nil {
 		return "", err
 	}
 	defer layer.ReleaseAndLog(daemon.layerStore, l)
 
-	h := image.History{
-		Author:     c.Author,
-		Created:    time.Now().UTC(),
-		CreatedBy:  strings.Join(containerConfig.Cmd, " "),
-		Comment:    c.Comment,
-		EmptyLayer: true,
+	containerConfig := c.ContainerConfig
+	if containerConfig == nil {
+		containerConfig = container.Config
 	}
-
-	if diffID := l.DiffID(); layer.DigestSHA256EmptyTar != diffID {
-		h.EmptyLayer = false
-		rootFS.Append(diffID)
+	cc := image.ChildConfig{
+		ContainerID:     container.ID,
+		Author:          c.Author,
+		Comment:         c.Comment,
+		ContainerConfig: containerConfig,
+		Config:          newConfig,
+		DiffID:          l.DiffID(),
 	}
-
-	history = append(history, h)
-
-	config, err := json.Marshal(&image.Image{
-		V1Image: image.V1Image{
-			DockerVersion:   dockerversion.Version,
-			Config:          newConfig,
-			Architecture:    runtime.GOARCH,
-			OS:              runtime.GOOS,
-			Container:       container.ID,
-			ContainerConfig: *containerConfig,
-			Author:          c.Author,
-			Created:         h.Created,
-		},
-		RootFS:     rootFS,
-		History:    history,
-		OSFeatures: osFeatures,
-		OSVersion:  osVersion,
-	})
-
+	config, err := json.Marshal(image.NewChildImage(parent, cc))
 	if err != nil {
 		return "", err
 	}

image/image.go

@@ -4,9 +4,13 @@ import (
 	"encoding/json"
 	"errors"
 	"io"
+	"runtime"
+	"strings"
 	"time"
 
 	"github.com/docker/docker/api/types/container"
+	"github.com/docker/docker/dockerversion"
+	"github.com/docker/docker/layer"
 	"github.com/opencontainers/go-digest"
 )
 
@@ -110,6 +114,51 @@ func (img *Image) MarshalJSON() ([]byte, error) {
 	return json.Marshal(c)
 }
 
+// ChildConfig is the configuration to apply to an Image to create a new
+// Child image. Other properties of the image are copied from the parent.
+type ChildConfig struct {
+	ContainerID     string
+	Author          string
+	Comment         string
+	DiffID          layer.DiffID
+	ContainerConfig *container.Config
+	Config          *container.Config
+}
+
+// NewChildImage creates a new Image as a child of this image.
+func NewChildImage(img *Image, child ChildConfig) *Image {
+	isEmptyLayer := layer.IsEmpty(child.DiffID)
+	rootFS := img.RootFS
+	if rootFS == nil {
+		rootFS = NewRootFS()
+	}
+	if !isEmptyLayer {
+		rootFS.Append(child.DiffID)
+	}
+	imgHistory := NewHistory(
+		child.Author,
+		child.Comment,
+		strings.Join(child.ContainerConfig.Cmd, " "),
+		isEmptyLayer)
+
+	return &Image{
+		V1Image: V1Image{
+			DockerVersion:   dockerversion.Version,
+			Config:          child.Config,
+			Architecture:    runtime.GOARCH,
+			OS:              runtime.GOOS,
+			Container:       child.ContainerID,
+			ContainerConfig: *child.ContainerConfig,
+			Author:          child.Author,
+			Created:         imgHistory.Created,
+		},
+		RootFS:     rootFS,
+		History:    append(img.History, imgHistory),
+		OSFeatures: img.OSFeatures,
+		OSVersion:  img.OSVersion,
+	}
+}
+
 // History stores build commands that were used to create an image
 type History struct {
 	// Created is the timestamp at which the image was created
@@ -126,6 +175,18 @@ type History struct {
 	EmptyLayer bool `json:"empty_layer,omitempty"`
 }
 
+// NewHistory creates a new history struct from arguments, and sets the created
+// time to the current time in UTC
+func NewHistory(author, comment, createdBy string, isEmptyLayer bool) History {
+	return History{
+		Author:     author,
+		Created:    time.Now().UTC(),
+		CreatedBy:  createdBy,
+		Comment:    comment,
+		EmptyLayer: isEmptyLayer,
+	}
+}
+
 // Exporter provides interface for loading and saving images
 type Exporter interface {
 	Load(io.ReadCloser, io.Writer, bool) error

image/store.go

@@ -2,7 +2,6 @@ package image
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"sync"
 
@@ -10,6 +9,7 @@ import (
 	"github.com/docker/distribution/digestset"
 	"github.com/docker/docker/layer"
 	"github.com/opencontainers/go-digest"
+	"github.com/pkg/errors"
 )
 
 // Store is an interface for creating and accessing images
@@ -147,7 +147,7 @@ func (is *store) Create(config []byte) (ID, error) {
 	if layerID != "" {
 		l, err = is.ls.Get(layerID)
 		if err != nil {
-			return "", err
+			return "", errors.Wrapf(err, "failed to get layer %s", layerID)
 		}
 	}
 

integration-cli/docker_cli_build_test.go

@@ -5870,15 +5870,17 @@ func (s *DockerSuite) TestBuildCopyFromPreviousRootFS(c *check.C) {
 	dockerfile := `
 		FROM busybox AS first
 		COPY foo bar
+
 		FROM busybox
-    %s
-    COPY baz baz
-    RUN echo mno > baz/cc
+		%s
+		COPY baz baz
+		RUN echo mno > baz/cc
+
 		FROM busybox
-    COPY bar /
-    COPY --from=1 baz sub/
-    COPY --from=0 bar baz
-    COPY --from=first bar bay`
+		COPY bar /
+		COPY --from=1 baz sub/
+		COPY --from=0 bar baz
+		COPY --from=first bar bay`
 
 	ctx := fakecontext.New(c, "",
 		fakecontext.WithDockerfile(fmt.Sprintf(dockerfile, "")),
@@ -5892,32 +5894,25 @@ func (s *DockerSuite) TestBuildCopyFromPreviousRootFS(c *check.C) {
 
 	cli.BuildCmd(c, "build1", build.WithExternalBuildContext(ctx))
 
-	out := cli.DockerCmd(c, "run", "build1", "cat", "bar").Combined()
-	c.Assert(strings.TrimSpace(out), check.Equals, "def")
-	out = cli.DockerCmd(c, "run", "build1", "cat", "sub/aa").Combined()
-	c.Assert(strings.TrimSpace(out), check.Equals, "ghi")
-	out = cli.DockerCmd(c, "run", "build1", "cat", "sub/cc").Combined()
-	c.Assert(strings.TrimSpace(out), check.Equals, "mno")
-	out = cli.DockerCmd(c, "run", "build1", "cat", "baz").Combined()
-	c.Assert(strings.TrimSpace(out), check.Equals, "abc")
-	out = cli.DockerCmd(c, "run", "build1", "cat", "bay").Combined()
-	c.Assert(strings.TrimSpace(out), check.Equals, "abc")
+	cli.DockerCmd(c, "run", "build1", "cat", "bar").Assert(c, icmd.Expected{Out: "def"})
+	cli.DockerCmd(c, "run", "build1", "cat", "sub/aa").Assert(c, icmd.Expected{Out: "ghi"})
+	cli.DockerCmd(c, "run", "build1", "cat", "sub/cc").Assert(c, icmd.Expected{Out: "mno"})
+	cli.DockerCmd(c, "run", "build1", "cat", "baz").Assert(c, icmd.Expected{Out: "abc"})
+	cli.DockerCmd(c, "run", "build1", "cat", "bay").Assert(c, icmd.Expected{Out: "abc"})
 
 	result := cli.BuildCmd(c, "build2", build.WithExternalBuildContext(ctx))
 
 	// all commands should be cached
 	c.Assert(strings.Count(result.Combined(), "Using cache"), checker.Equals, 7)
+	c.Assert(getIDByName(c, "build1"), checker.Equals, getIDByName(c, "build2"))
 
 	err := ioutil.WriteFile(filepath.Join(ctx.Dir, "Dockerfile"), []byte(fmt.Sprintf(dockerfile, "COPY baz/aa foo")), 0644)
 	c.Assert(err, checker.IsNil)
 
 	// changing file in parent block should not affect last block
 	result = cli.BuildCmd(c, "build3", build.WithExternalBuildContext(ctx))
-
 	c.Assert(strings.Count(result.Combined(), "Using cache"), checker.Equals, 5)
 
-	c.Assert(getIDByName(c, "build1"), checker.Equals, getIDByName(c, "build2"))
-
 	err = ioutil.WriteFile(filepath.Join(ctx.Dir, "foo"), []byte("pqr"), 0644)
 	c.Assert(err, checker.IsNil)
 
@@ -5925,10 +5920,8 @@ func (s *DockerSuite) TestBuildCopyFromPreviousRootFS(c *check.C) {
 	result = cli.BuildCmd(c, "build4", build.WithExternalBuildContext(ctx))
 	c.Assert(strings.Count(result.Combined(), "Using cache"), checker.Equals, 5)
 
-	out = cli.DockerCmd(c, "run", "build4", "cat", "bay").Combined()
-	c.Assert(strings.TrimSpace(out), check.Equals, "pqr")
-	out = cli.DockerCmd(c, "run", "build4", "cat", "baz").Combined()
-	c.Assert(strings.TrimSpace(out), check.Equals, "pqr")
+	cli.DockerCmd(c, "run", "build4", "cat", "bay").Assert(c, icmd.Expected{Out: "pqr"})
+	cli.DockerCmd(c, "run", "build4", "cat", "baz").Assert(c, icmd.Expected{Out: "pqr"})
 }
 
 func (s *DockerSuite) TestBuildCopyFromPreviousRootFSErrors(c *check.C) {

layer/empty.go

@@ -54,3 +54,8 @@ func (el *emptyLayer) DiffSize() (size int64, err error) {
 func (el *emptyLayer) Metadata() (map[string]string, error) {
 	return make(map[string]string), nil
 }
+
+// IsEmpty returns true if the layer is an EmptyLayer
+func IsEmpty(diffID DiffID) bool {
+	return diffID == DigestSHA256EmptyTar
+}

pkg/testutil/tempfile/tempfile.go

@@ -34,3 +34,23 @@ func (f *TempFile) Name() string {
 func (f *TempFile) Remove() {
 	os.Remove(f.Name())
 }
+
+// TempDir is a temporary directory that can be used with unit tests. TempDir
+// reduces the boilerplate setup required in each test case by handling
+// setup errors.
+type TempDir struct {
+	Path string
+}
+
+// NewTempDir returns a new temp file with contents
+func NewTempDir(t require.TestingT, prefix string) *TempDir {
+	path, err := ioutil.TempDir("", prefix+"-")
+	require.NoError(t, err)
+
+	return &TempDir{Path: path}
+}
+
+// Remove removes the file
+func (f *TempDir) Remove() {
+	os.Remove(f.Path)
+}