Merge pull request moby#3474 from creack/fix_add_cache_issue

Fix ADD caching issue with . prefixed path
rjnagal · Jan 8, 2014 · bb76985 · bb76985
2 parents 28ec47c + f3103e5
commit bb76985
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 14 deletions.
diff --git a/buildfile.go b/buildfile.go
@@ -407,6 +407,15 @@ func (b *buildFile) CmdAdd(args string) error {
 			hash string
 			sums = b.context.GetSums()
 		)
+
+		// Has tarsum strips the '.' and './', we put it back for comparaison.
+		for file, sum := range sums {
+			if len(file) == 0 || file[0] != '.' && file[0] != '/' {
+				delete(sums, file)
+				sums["./"+file] = sum
+			}
+		}
+
 		if fi, err := os.Stat(path.Join(b.contextPath, origPath)); err != nil {
 			return err
 		} else if fi.IsDir() {
@@ -428,7 +437,8 @@ func (b *buildFile) CmdAdd(args string) error {
 		if err != nil {
 			return err
 		}
-		if hit {
+		// If we do not have a hash, never use the cache
+		if hit && hash != "" {
 			return nil
 		}
 	}

diff --git a/graphdriver/vfs/driver.go b/graphdriver/vfs/driver.go
@@ -36,9 +36,8 @@ func (d *Driver) Cleanup() error {
 }
 
 func copyDir(src, dst string) error {
-	cmd := exec.Command("cp", "-aT", "--reflink=auto", src, dst)
-	if err := cmd.Run(); err != nil {
-		return err
+	if output, err := exec.Command("cp", "-aT", "--reflink=auto", src, dst).CombinedOutput(); err != nil {
+		return fmt.Errorf("Error VFS copying directory: %s (%s)", err, output)
 	}
 	return nil
 }

diff --git a/integration/buildfile_test.go b/integration/buildfile_test.go
@@ -457,7 +457,7 @@ func TestBuildEntrypointRunCleanup(t *testing.T) {
 	}
 }
 
-func checkCacheBehavior(t *testing.T, template testContextTemplate, expectHit bool) {
+func checkCacheBehavior(t *testing.T, template testContextTemplate, expectHit bool) (imageId string) {
 	eng := NewTestEngine(t)
 	defer nuke(mkRuntimeFromEngine(eng, t))
 
@@ -466,20 +466,36 @@ func checkCacheBehavior(t *testing.T, template testContextTemplate, expectHit bo
 		t.Fatal(err)
 	}
 
-	imageId := img.ID
+	imageId = img.ID
 
-	img = nil
 	img, err = buildImage(template, t, eng, expectHit)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	hit := imageId == img.ID
-	if hit != expectHit {
-		t.Logf("Cache misbehavior, got hit=%t, expected hit=%t: (first: %s, second %s)",
-			hit, expectHit, imageId, img.ID)
-		t.Fail()
+	if hit := imageId == img.ID; hit != expectHit {
+		t.Fatalf("Cache misbehavior, got hit=%t, expected hit=%t: (first: %s, second %s)", hit, expectHit, imageId, img.ID)
 	}
+	return
+}
+
+func checkCacheBehaviorFromEngime(t *testing.T, template testContextTemplate, expectHit bool, eng *engine.Engine) (imageId string) {
+	img, err := buildImage(template, t, eng, true)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	imageId = img.ID
+
+	img, err = buildImage(template, t, eng, expectHit)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if hit := imageId == img.ID; hit != expectHit {
+		t.Fatalf("Cache misbehavior, got hit=%t, expected hit=%t: (first: %s, second %s)", hit, expectHit, imageId, img.ID)
+	}
+	return
 }
 
 func TestBuildImageWithCache(t *testing.T) {
@@ -506,11 +522,61 @@ func TestBuildADDLocalFileWithCache(t *testing.T) {
         maintainer dockerio
         run echo "first"
         add foo /usr/lib/bla/bar
+	run [ "$(cat /usr/lib/bla/bar)" = "hello" ]
         run echo "second"
+	add . /src/
+	run [ "$(cat /src/foo)" = "hello" ]
         `,
-		[][2]string{{"foo", "hello"}},
+		[][2]string{
+			{"foo", "hello"},
+		},
 		nil}
-	checkCacheBehavior(t, template, true)
+	eng := NewTestEngine(t)
+	defer nuke(mkRuntimeFromEngine(eng, t))
+
+	id1 := checkCacheBehaviorFromEngime(t, template, true, eng)
+	template.files = append(template.files, [2]string{"bar", "hello2"})
+	id2 := checkCacheBehaviorFromEngime(t, template, true, eng)
+	if id1 == id2 {
+		t.Fatal("The cache should have been invalided but hasn't.")
+	}
+	id3 := checkCacheBehaviorFromEngime(t, template, true, eng)
+	if id2 != id3 {
+		t.Fatal("The cache should have been used but hasn't.")
+	}
+	template.files[1][1] = "hello3"
+	id4 := checkCacheBehaviorFromEngime(t, template, true, eng)
+	if id3 == id4 {
+		t.Fatal("The cache should have been invalided but hasn't.")
+	}
+	template.dockerfile += `
+	add ./bar /src2/
+	run ls /src2/bar
+	`
+	id5 := checkCacheBehaviorFromEngime(t, template, true, eng)
+	if id4 == id5 {
+		t.Fatal("The cache should have been invalided but hasn't.")
+	}
+	template.files[1][1] = "hello4"
+	id6 := checkCacheBehaviorFromEngime(t, template, true, eng)
+	if id5 == id6 {
+		t.Fatal("The cache should have been invalided but hasn't.")
+	}
+
+	template.dockerfile += `
+	add bar /src2/bar2
+	add /bar /src2/bar3
+	run ls /src2/bar2 /src2/bar3
+	`
+	id7 := checkCacheBehaviorFromEngime(t, template, true, eng)
+	if id6 == id7 {
+		t.Fatal("The cache should have been invalided but hasn't.")
+	}
+	template.files[1][1] = "hello5"
+	id8 := checkCacheBehaviorFromEngime(t, template, true, eng)
+	if id7 == id8 {
+		t.Fatal("The cache should have been invalided but hasn't.")
+	}
 }
 
 func TestBuildADDLocalFileWithoutCache(t *testing.T) {