Browse code

[builder] Make build cache ignore mtime

Build cache uses pgk/tarsum to get a digest of content which is
ADD'd or COPY'd during a build. The builder has always used v0 of
the tarsum algorithm which includes mtimes however since the whole
file is hashed anyway, the mtime doesn't really provide any extra
information about whether the file has changed and many version
control tools like Git strip mtime from files when they are cloned.

This patch updates the build subsystem to use v1 of Tarsum which
explicitly ignores mtime when calculating a digest. Now ADD and
COPY will result in a cache hit if only the mtime and not the file
contents have changed.

NOTE: Tarsum is NOT a meant to be a cryptographically secure hash
function. It is a best-effort approach to determining if two sets of
filesystem content are different.

Docker-DCO-1.1-Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)

Josh Hawn authored on 2015/04/03 02:42:40
Showing 2 changed files
... ...
@@ -50,7 +50,7 @@ func (b *Builder) readContext(context io.Reader) error {
50 50
 		return err
51 51
 	}
52 52
 
53
-	if b.context, err = tarsum.NewTarSum(decompressedStream, true, tarsum.Version0); err != nil {
53
+	if b.context, err = tarsum.NewTarSum(decompressedStream, true, tarsum.Version1); err != nil {
54 54
 		return err
55 55
 	}
56 56
 
... ...
@@ -345,7 +345,7 @@ func calcCopyInfo(b *Builder, cmdName string, cInfos *[]*copyInfo, origPath stri
345 345
 		if err != nil {
346 346
 			return err
347 347
 		}
348
-		tarSum, err := tarsum.NewTarSum(r, true, tarsum.Version0)
348
+		tarSum, err := tarsum.NewTarSum(r, true, tarsum.Version1)
349 349
 		if err != nil {
350 350
 			return err
351 351
 		}
... ...
@@ -2768,7 +2768,6 @@ func (s *DockerSuite) TestBuildAddCurrentDirWithCache(c *check.C) {
2768 2768
 	name2 := name + "2"
2769 2769
 	name3 := name + "3"
2770 2770
 	name4 := name + "4"
2771
-	name5 := name + "5"
2772 2771
 	dockerfile := `
2773 2772
         FROM scratch
2774 2773
         MAINTAINER dockerio
... ...
@@ -2806,7 +2805,8 @@ func (s *DockerSuite) TestBuildAddCurrentDirWithCache(c *check.C) {
2806 2806
 	if id2 == id3 {
2807 2807
 		c.Fatal("The cache should have been invalided but hasn't.")
2808 2808
 	}
2809
-	// Check that changing file to same content invalidate cache of "ADD ."
2809
+	// Check that changing file to same content with different mtime does not
2810
+	// invalidate cache of "ADD ."
2810 2811
 	time.Sleep(1 * time.Second) // wait second because of mtime precision
2811 2812
 	if err := ctx.Add("foo", "hello1"); err != nil {
2812 2813
 		c.Fatal(err)
... ...
@@ -2815,14 +2815,7 @@ func (s *DockerSuite) TestBuildAddCurrentDirWithCache(c *check.C) {
2815 2815
 	if err != nil {
2816 2816
 		c.Fatal(err)
2817 2817
 	}
2818
-	if id3 == id4 {
2819
-		c.Fatal("The cache should have been invalided but hasn't.")
2820
-	}
2821
-	id5, err := buildImageFromContext(name5, ctx, true)
2822
-	if err != nil {
2823
-		c.Fatal(err)
2824
-	}
2825
-	if id4 != id5 {
2818
+	if id3 != id4 {
2826 2819
 		c.Fatal("The cache should have been used but hasn't.")
2827 2820
 	}
2828 2821
 }
... ...
@@ -2921,7 +2914,6 @@ func (s *DockerSuite) TestBuildAddRemoteFileMTime(c *check.C) {
2921 2921
 	name := "testbuildaddremotefilemtime"
2922 2922
 	name2 := name + "2"
2923 2923
 	name3 := name + "3"
2924
-	name4 := name + "4"
2925 2924
 
2926 2925
 	files := map[string]string{"baz": "hello"}
2927 2926
 	server, err := fakeStorage(files)
... ...
@@ -2951,8 +2943,8 @@ func (s *DockerSuite) TestBuildAddRemoteFileMTime(c *check.C) {
2951 2951
 		c.Fatal("The cache should have been used but wasn't - #1")
2952 2952
 	}
2953 2953
 
2954
-	// Now create a different server withsame contents (causes different mtim)
2955
-	// This time the cache should not be used
2954
+	// Now create a different server with same contents (causes different mtime)
2955
+	// The cache should still be used
2956 2956
 
2957 2957
 	// allow some time for clock to pass as mtime precision is only 1s
2958 2958
 	time.Sleep(2 * time.Second)
... ...
@@ -2974,17 +2966,8 @@ func (s *DockerSuite) TestBuildAddRemoteFileMTime(c *check.C) {
2974 2974
 	if err != nil {
2975 2975
 		c.Fatal(err)
2976 2976
 	}
2977
-	if id1 == id3 {
2978
-		c.Fatal("The cache should not have been used but was")
2979
-	}
2980
-
2981
-	// And for good measure do it again and make sure cache is used this time
2982
-	id4, err := buildImageFromContext(name4, ctx2, true)
2983
-	if err != nil {
2984
-		c.Fatal(err)
2985
-	}
2986
-	if id3 != id4 {
2987
-		c.Fatal("The cache should have been used but wasn't - #2")
2977
+	if id1 != id3 {
2978
+		c.Fatal("The cache should have been used but wasn't")
2988 2979
 	}
2989 2980
 }
2990 2981