Merge pull request #5997 from crosbymichael/add-hang

Fix add hang when dest is .

integration-cli/build_tests/TestAdd/SingleFileToWorkdir/Dockerfile

@@ -0,0 +1,2 @@
+FROM busybox
+ADD test_file .

integration-cli/docker_cli_build_test.go

@@ -7,6 +7,7 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+	"time"
 )
 
 func TestBuildCacheADD(t *testing.T) {
@@ -77,6 +78,42 @@ func TestAddSingleFileToRoot(t *testing.T) {
 	logDone("build - add single file to root")
 }
 
+// Issue #3960: "ADD src ." hangs
+func TestAddSingleFileToWorkdir(t *testing.T) {
+	buildDirectory := filepath.Join(workingDirectory, "build_tests", "TestAdd", "SingleFileToWorkdir")
+	f, err := os.OpenFile(filepath.Join(buildDirectory, "test_file"), os.O_CREATE, 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+	f.Close()
+	buildCmd := exec.Command(dockerBinary, "build", "-t", "testaddimg", ".")
+	buildCmd.Dir = buildDirectory
+	done := make(chan error)
+	go func() {
+		out, exitCode, err := runCommandWithOutput(buildCmd)
+		if err != nil || exitCode != 0 {
+			done <- fmt.Errorf("build failed to complete: %s %v", out, err)
+			return
+		}
+		done <- nil
+	}()
+	select {
+	case <-time.After(5 * time.Second):
+		if err := buildCmd.Process.Kill(); err != nil {
+			fmt.Printf("could not kill build (pid=%d): %v\n", buildCmd.Process.Pid, err)
+		}
+		t.Fatal("build timed out")
+	case err := <-done:
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	deleteImages("testaddimg")
+
+	logDone("build - add single file to workdir")
+}
+
 func TestAddSingleFileToExistDir(t *testing.T) {
 	buildDirectory := filepath.Join(workingDirectory, "build_tests", "TestAdd")
 	buildCmd := exec.Command(dockerBinary, "build", "-t", "testaddimg", "SingleFileToExistDir")

server/buildfile.go

@@ -415,17 +415,18 @@ func (b *buildFile) addContext(container *daemon.Container, orig, dest string, r
 	}
 
 	// Preserve the trailing '/'
-	if strings.HasSuffix(dest, "/") {
+	if strings.HasSuffix(dest, "/") || dest == "." {
 		destPath = destPath + "/"
 	}
+
 	destStat, err := os.Stat(destPath)
 	if err != nil {
-		if os.IsNotExist(err) {
-			destExists = false
-		} else {
+		if !os.IsNotExist(err) {
 			return err
 		}
+		destExists = false
 	}
+
 	fi, err := os.Stat(origPath)
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -434,57 +435,29 @@ func (b *buildFile) addContext(container *daemon.Container, orig, dest string, r
 		return err
 	}
 
-	fixPermsR := func(destPath string, uid, gid int) error {
-		return filepath.Walk(destPath, func(path string, info os.FileInfo, err error) error {
-			if err := os.Lchown(path, uid, gid); err != nil && !os.IsNotExist(err) {
-				return err
-			}
-			return nil
-		})
-	}
-
 	if fi.IsDir() {
-		if err := archive.CopyWithTar(origPath, destPath); err != nil {
-			return err
-		}
-		if destExists {
-			files, err := ioutil.ReadDir(origPath)
-			if err != nil {
-				return err
-			}
-			for _, file := range files {
-				if err := fixPermsR(filepath.Join(destPath, file.Name()), 0, 0); err != nil {
-					return err
-				}
-			}
-		} else {
-			if err := fixPermsR(destPath, 0, 0); err != nil {
-				return err
-			}
-		}
-		return nil
-	}
-
-	// 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 / .
-	tarDest := destPath
-	if strings.HasSuffix(tarDest, "/") {
-		tarDest = filepath.Dir(destPath)
+		return copyAsDirectory(origPath, destPath, destExists)
 	}
 
 	// If we are adding a remote file, do not try to untar it
 	if !remote {
+		// 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 / .
+		tarDest := destPath
+		if strings.HasSuffix(tarDest, "/") {
+			tarDest = filepath.Dir(destPath)
+		}
+
 		// try to successfully untar the orig
 		if err := archive.UntarPath(origPath, tarDest); err == nil {
 			return nil
+		} else if err != io.EOF {
+			utils.Debugf("Couldn't untar %s to %s: %s", origPath, tarDest, err)
 		}
-		utils.Debugf("Couldn't untar %s to %s: %s", origPath, destPath, err)
 	}
 
-	// If that fails, just copy it as a regular file
-	// but do not use all the magic path handling for the tar path
 	if err := os.MkdirAll(path.Dir(destPath), 0755); err != nil {
 		return err
 	}
@@ -497,10 +470,7 @@ func (b *buildFile) addContext(container *daemon.Container, orig, dest string, r
 		resPath = path.Join(destPath, path.Base(origPath))
 	}
 
-	if err := fixPermsR(resPath, 0, 0); err != nil {
-		return err
-	}
-	return nil
+	return fixPermissions(resPath, 0, 0)
 }
 
 func (b *buildFile) CmdAdd(args string) error {
@@ -873,6 +843,37 @@ func stripComments(raw []byte) string {
 	return strings.Join(out, "\n")
 }
 
+func copyAsDirectory(source, destination string, destinationExists bool) error {
+	if err := archive.CopyWithTar(source, destination); err != nil {
+		return err
+	}
+
+	if destinationExists {
+		files, err := ioutil.ReadDir(source)
+		if err != nil {
+			return err
+		}
+
+		for _, file := range files {
+			if err := fixPermissions(filepath.Join(destination, file.Name()), 0, 0); err != nil {
+				return err
+			}
+		}
+		return nil
+	}
+
+	return fixPermissions(destination, 0, 0)
+}
+
+func fixPermissions(destination string, uid, gid int) error {
+	return filepath.Walk(destination, func(path string, info os.FileInfo, err error) error {
+		if err := os.Lchown(path, uid, gid); err != nil && !os.IsNotExist(err) {
+			return err
+		}
+		return nil
+	})
+}
+
 func NewBuildFile(srv *Server, outStream, errStream io.Writer, verbose, utilizeCache, rm bool, forceRm bool, outOld io.Writer, sf *utils.StreamFormatter, auth *registry.AuthConfig, authConfigFile *registry.ConfigFile) BuildFile {
 	return &buildFile{
 		daemon:        srv.daemon,