Fix a bug in snapshotting with multi-stage images. (#546)

We previously had an optimization that would skip snapshotting mutli-stage images
when in an intermediate stage, until the very end.

This conflicted with another optimization to avoid snapshotting when no files had changed.

integration/dockerfiles/Dockerfile_test_multistage

@@ -1,18 +1,23 @@
-FROM gcr.io/distroless/base@sha256:628939ac8bf3f49571d05c6c76b8688cb4a851af6c7088e599388259875bde20 as base
+FROM gcr.io/google-appengine/debian9@sha256:f0159d14385afcb58a9b2fa8955c0cb64bd3abc365e8589f8c2dd38150fbfdbe as base
 COPY . .
 
 FROM scratch as second
 ENV foopath context/foo
 COPY --from=0 $foopath context/b* /foo/
 
-FROM second
+FROM second as third
 COPY --from=base /context/foo /new/foo
 
+FROM base as fourth
+# Make sure that we snapshot intermediate images correctly
+RUN date > /date
+ENV foo bar
+
 # This base image contains symlinks with relative paths to whitelisted directories
 # We need to test they're extracted correctly
 FROM fedora@sha256:c4cc32b09c6ae3f1353e7e33a8dda93dc41676b923d6d89afa996b421cc5aa48
 
-FROM base
+FROM fourth
 ARG file
 COPY --from=second /foo ${file}
 COPY --from=gcr.io/google-appengine/debian9@sha256:00109fa40230a081f5ecffe0e814725042ff62a03e2d1eae0563f1f82eaeae9b /etc/os-release /new

pkg/executor/build.go

@@ -309,11 +309,6 @@ func (s *stageBuilder) shouldTakeSnapshot(index int, files []string) bool {
 		return true
 	}
 
-	// We only snapshot the very end of intermediate stages.
-	if !s.stage.Final {
-		return isLastCommand
-	}
-
 	// nil means snapshot everything.
 	if files == nil {
 		return true

pkg/executor/build_test.go

@@ -147,7 +147,7 @@ func Test_stageBuilder_shouldTakeSnapshot(t *testing.T) {
 			args: args{
 				index: 0,
 			},
-			want: false,
+			want: true,
 		},
 		{
 			name: "caching enabled intermediate container",