diff --git a/builder/builder.go b/builder/builder.go index 06dfd3697e..b25a5cd043 100644 --- a/builder/builder.go +++ b/builder/builder.go @@ -129,6 +129,8 @@ type Backend interface { ContainerWait(containerID string, timeout time.Duration) (int, error) // ContainerUpdateCmdOnBuild updates container.Path and container.Args ContainerUpdateCmdOnBuild(containerID string, cmd []string) error + // ContainerCreateWorkdir creates the workdir (currently only used on Windows) + ContainerCreateWorkdir(containerID string) error // ContainerCopy copies/extracts a source FileInfo to a destination path inside a container // specified by a container object. diff --git a/builder/dockerfile/dispatchers.go b/builder/dockerfile/dispatchers.go index ab9d3145f4..93598882f6 100644 --- a/builder/dockerfile/dispatchers.go +++ b/builder/dockerfile/dispatchers.go @@ -18,6 +18,7 @@ import ( "github.com/Sirupsen/logrus" "github.com/docker/docker/api" + "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/strslice" "github.com/docker/docker/builder" @@ -279,12 +280,26 @@ func workdir(b *Builder, args []string, attributes map[string]bool, original str return err } - // NOTE: You won't find the "mkdir" for the directory in here. Rather we - // just set the value in the image's runConfig.WorkingDir property - // and container.SetupWorkingDirectory() will create it automatically - // for us the next time the image is used to create a container. + // For performance reasons, we explicitly do a create/mkdir now + // This avoids having an unnecessary expensive mount/unmount calls + // (on Windows in particular) during each container create. + // Prior to 1.13, the mkdir was deferred and not executed at this step. + if b.disableCommit { + // Don't call back into the daemon if we're going through docker commit --change "WORKDIR /foo". + // We've already updated the runConfig and that's enough. + return nil + } + b.runConfig.Image = b.image + container, err := b.docker.ContainerCreate(types.ContainerCreateConfig{Config: b.runConfig}, true) + if err != nil { + return err + } + b.tmpContainers[container.ID] = struct{}{} + if err := b.docker.ContainerCreateWorkdir(container.ID); err != nil { + return err + } - return b.commit("", b.runConfig.Cmd, fmt.Sprintf("WORKDIR %v", b.runConfig.WorkingDir)) + return b.commit(container.ID, b.runConfig.Cmd, "WORKDIR "+b.runConfig.WorkingDir) } // RUN some command yo diff --git a/daemon/workdir.go b/daemon/workdir.go new file mode 100644 index 0000000000..5bd0d0caca --- /dev/null +++ b/daemon/workdir.go @@ -0,0 +1,21 @@ +package daemon + +// ContainerCreateWorkdir creates the working directory. This is solves the +// issue arising from https://github.com/docker/docker/issues/27545, +// which was initially fixed by https://github.com/docker/docker/pull/27884. But that fix +// was too expensive in terms of performance on Windows. Instead, +// https://github.com/docker/docker/pull/28514 introduces this new functionality +// where the builder calls into the backend here to create the working directory. +func (daemon *Daemon) ContainerCreateWorkdir(cID string) error { + container, err := daemon.GetContainer(cID) + if err != nil { + return err + } + err = daemon.Mount(container) + if err != nil { + return err + } + defer daemon.Unmount(container) + rootUID, rootGID := daemon.GetRemappedUIDGID() + return container.SetupWorkingDirectory(rootUID, rootGID) +} diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index fd0fd1130e..eea4712fe7 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -1878,8 +1878,8 @@ func (s *DockerSuite) TestBuildWindowsAddCopyPathProcessing(c *check.C) { WORKDIR /wc2 ADD wc2 c:/wc2 WORKDIR c:/ - RUN sh -c "[ $(cat c:/wc1) = 'hellowc1' ]" - RUN sh -c "[ $(cat c:/wc2) = 'worldwc2' ]" + RUN sh -c "[ $(cat c:/wc1/wc1) = 'hellowc1' ]" + RUN sh -c "[ $(cat c:/wc2/wc2) = 'worldwc2' ]" # Trailing slash on COPY/ADD, Windows-style path. WORKDIR /wd1 @@ -7283,3 +7283,31 @@ func (s *DockerSuite) TestBuildWindowsUser(c *check.C) { } c.Assert(strings.ToLower(out), checker.Contains, "username=user") } + +// Verifies if COPY file . when WORKDIR is set to a non-existing directory, +// the directory is created and the file is copied into the directory, +// as opposed to the file being copied as a file with the name of the +// directory. Fix for 27545 (found on Windows, but regression good for Linux too). +// Note 27545 was reverted in 28505, but a new fix was added subsequently in 28514. +func (s *DockerSuite) TestBuildCopyFileDotWithWorkdir(c *check.C) { + name := "testbuildcopyfiledotwithworkdir" + ctx, err := fakeContext(`FROM busybox +WORKDIR /foo +COPY file . +RUN ["cat", "/foo/file"] +`, + map[string]string{}) + + if err != nil { + c.Fatal(err) + } + defer ctx.Close() + + if err := ctx.Add("file", "content"); err != nil { + c.Fatal(err) + } + + if _, err = buildImageFromContext(name, ctx, true); err != nil { + c.Fatal(err) + } +} diff --git a/integration-cli/docker_cli_commit_test.go b/integration-cli/docker_cli_commit_test.go index 423314126a..72ff89f3dc 100644 --- a/integration-cli/docker_cli_commit_test.go +++ b/integration-cli/docker_cli_commit_test.go @@ -104,7 +104,6 @@ func (s *DockerSuite) TestCommitWithHostBindMount(c *check.C) { } func (s *DockerSuite) TestCommitChange(c *check.C) { - testRequires(c, DaemonIsLinux) dockerCmd(c, "run", "--name", "test", "busybox", "true") imageID, _ := dockerCmd(c, "commit", @@ -122,12 +121,14 @@ func (s *DockerSuite) TestCommitChange(c *check.C) { "test", "test-commit") imageID = strings.TrimSpace(imageID) + prefix, slash := getPrefixAndSlashFromDaemonPlatform() + prefix = strings.ToUpper(prefix) // Force C: as that's how WORKDIR is normalised on Windows expected := map[string]string{ "Config.ExposedPorts": "map[8080/tcp:{}]", "Config.Env": "[DEBUG=true test=1 PATH=/foo]", "Config.Labels": "map[foo:bar]", "Config.Cmd": "[/bin/sh]", - "Config.WorkingDir": "/opt", + "Config.WorkingDir": prefix + slash + "opt", "Config.Entrypoint": "[/bin/sh]", "Config.User": "testuser", "Config.Volumes": "map[/var/lib/docker:{}]",