Merge pull request #8748 from duglin/Issue8330

Have .dockerignore support Dockerfile/.dockerignore

api/client/commands.go

@@ -14,7 +14,6 @@ import (
 	"os"
 	"os/exec"
 	"path"
-	"path/filepath"
 	"runtime"
 	"strconv"
 	"strings"
@@ -30,6 +29,7 @@ import (
 	"github.com/docker/docker/nat"
 	"github.com/docker/docker/opts"
 	"github.com/docker/docker/pkg/archive"
+	"github.com/docker/docker/pkg/fileutils"
 	flag "github.com/docker/docker/pkg/mflag"
 	"github.com/docker/docker/pkg/parsers"
 	"github.com/docker/docker/pkg/parsers/filters"
@@ -140,32 +140,32 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
 		if _, err = os.Stat(filename); os.IsNotExist(err) {
 			return fmt.Errorf("no Dockerfile found in %s", cmd.Arg(0))
 		}
-		var excludes []string
-		ignore, err := ioutil.ReadFile(path.Join(root, ".dockerignore"))
-		if err != nil && !os.IsNotExist(err) {
-			return fmt.Errorf("Error reading .dockerignore: '%s'", err)
+		var includes []string = []string{"."}
+
+		excludes, err := utils.ReadDockerIgnore(path.Join(root, ".dockerignore"))
+		if err != nil {
+			return err
 		}
-		for _, pattern := range strings.Split(string(ignore), "\n") {
-			pattern = strings.TrimSpace(pattern)
-			if pattern == "" {
-				continue
-			}
-			pattern = filepath.Clean(pattern)
-			ok, err := filepath.Match(pattern, "Dockerfile")
-			if err != nil {
-				return fmt.Errorf("Bad .dockerignore pattern: '%s', error: %s", pattern, err)
-			}
-			if ok {
-				return fmt.Errorf("Dockerfile was excluded by .dockerignore pattern '%s'", pattern)
-			}
-			excludes = append(excludes, pattern)
+
+		// If .dockerignore mentions .dockerignore or Dockerfile
+		// then make sure we send both files over to the daemon
+		// because Dockerfile is, obviously, needed no matter what, and
+		// .dockerignore is needed to know if either one needs to be
+		// removed.  The deamon will remove them for us, if needed, after it
+		// parses the Dockerfile.
+		keepThem1, _ := fileutils.Matches(".dockerignore", excludes)
+		keepThem2, _ := fileutils.Matches("Dockerfile", excludes)
+		if keepThem1 || keepThem2 {
+			includes = append(includes, ".dockerignore", "Dockerfile")
 		}
+
 		if err = utils.ValidateContextDirectory(root, excludes); err != nil {
 			return fmt.Errorf("Error checking context is accessible: '%s'. Please check permissions and try again.", err)
 		}
 		options := &archive.TarOptions{
-			Compression: archive.Uncompressed,
-			Excludes:    excludes,
+			Compression:     archive.Uncompressed,
+			ExcludePatterns: excludes,
+			IncludeFiles:    includes,
 		}
 		context, err = archive.TarWithOptions(root, options)
 		if err != nil {

builder/evaluator.go

@@ -31,6 +31,7 @@ import (
 	"github.com/docker/docker/builder/parser"
 	"github.com/docker/docker/daemon"
 	"github.com/docker/docker/engine"
+	"github.com/docker/docker/pkg/fileutils"
 	"github.com/docker/docker/pkg/tarsum"
 	"github.com/docker/docker/registry"
 	"github.com/docker/docker/runconfig"
@@ -136,30 +137,10 @@ func (b *Builder) Run(context io.Reader) (string, error) {
 		}
 	}()
 
-	filename := path.Join(b.contextPath, "Dockerfile")
-
-	fi, err := os.Stat(filename)
-	if os.IsNotExist(err) {
-		return "", fmt.Errorf("Cannot build a directory without a Dockerfile")
-	}
-	if fi.Size() == 0 {
-		return "", ErrDockerfileEmpty
-	}
-
-	f, err := os.Open(filename)
-	if err != nil {
+	if err := b.readDockerfile("Dockerfile"); err != nil {
 		return "", err
 	}
 
-	defer f.Close()
-
-	ast, err := parser.Parse(f)
-	if err != nil {
-		return "", err
-	}
-
-	b.dockerfile = ast
-
 	// some initializations that would not have been supplied by the caller.
 	b.Config = &runconfig.Config{}
 	b.TmpContainers = map[string]struct{}{}
@@ -185,6 +166,53 @@ func (b *Builder) Run(context io.Reader) (string, error) {
 	return b.image, nil
 }
 
+// Reads a Dockerfile from the current context. It assumes that the
+// 'filename' is a relative path from the root of the context
+func (b *Builder) readDockerfile(filename string) error {
+	filename = path.Join(b.contextPath, filename)
+
+	fi, err := os.Stat(filename)
+	if os.IsNotExist(err) {
+		return fmt.Errorf("Cannot build a directory without a Dockerfile")
+	}
+	if fi.Size() == 0 {
+		return ErrDockerfileEmpty
+	}
+
+	f, err := os.Open(filename)
+	if err != nil {
+		return err
+	}
+
+	b.dockerfile, err = parser.Parse(f)
+	f.Close()
+
+	if err != nil {
+		return err
+	}
+
+	// After the Dockerfile has been parsed, we need to check the .dockerignore
+	// file for either "Dockerfile" or ".dockerignore", and if either are
+	// present then erase them from the build context. These files should never
+	// have been sent from the client but we did send them to make sure that
+	// we had the Dockerfile to actually parse, and then we also need the
+	// .dockerignore file to know whether either file should be removed.
+	// Note that this assumes the Dockerfile has been read into memory and
+	// is now safe to be removed.
+
+	excludes, _ := utils.ReadDockerIgnore(path.Join(b.contextPath, ".dockerignore"))
+	if rm, _ := fileutils.Matches(".dockerignore", excludes); rm == true {
+		os.Remove(path.Join(b.contextPath, ".dockerignore"))
+		b.context.(tarsum.BuilderContext).Remove(".dockerignore")
+	}
+	if rm, _ := fileutils.Matches("Dockerfile", excludes); rm == true {
+		os.Remove(path.Join(b.contextPath, "Dockerfile"))
+		b.context.(tarsum.BuilderContext).Remove("Dockerfile")
+	}
+
+	return nil
+}
+
 // This method is the entrypoint to all statement handling routines.
 //
 // Almost all nodes will have this structure:

builder/internals.go

@@ -390,7 +390,15 @@ func calcCopyInfo(b *Builder, cmdName string, cInfos *[]*copyInfo, origPath stri
 
 	for _, fileInfo := range b.context.GetSums() {
 		absFile := path.Join(b.contextPath, fileInfo.Name())
-		if strings.HasPrefix(absFile, absOrigPath) || absFile == absOrigPathNoSlash {
+		// Any file in the context that starts with the given path will be
+		// picked up and its hashcode used.  However, we'll exclude the
+		// root dir itself.  We do this for a coupel of reasons:
+		// 1 - ADD/COPY will not copy the dir itself, just its children
+		//     so there's no reason to include it in the hash calc
+		// 2 - the metadata on the dir will change when any child file
+		//     changes.  This will lead to a miss in the cache check if that
+		//     child file is in the .dockerignore list.
+		if strings.HasPrefix(absFile, absOrigPath) && absFile != absOrigPathNoSlash {
 			subfiles = append(subfiles, fileInfo.Sum())
 		}
 	}

daemon/container.go

@@ -888,8 +888,8 @@ func (container *Container) Copy(resource string) (io.ReadCloser, error) {
 	}
 
 	archive, err := archive.TarWithOptions(basePath, &archive.TarOptions{
-		Compression: archive.Uncompressed,
-		Includes:    filter,
+		Compression:  archive.Uncompressed,
+		IncludeFiles: filter,
 	})
 	if err != nil {
 		container.Unmount()

daemon/graphdriver/aufs/aufs.go

@@ -300,8 +300,8 @@ func (a *Driver) Put(id string) {
 func (a *Driver) Diff(id, parent string) (archive.Archive, error) {
 	// AUFS doesn't need the parent layer to produce a diff.
 	return archive.TarWithOptions(path.Join(a.rootPath(), "diff", id), &archive.TarOptions{
-		Compression: archive.Uncompressed,
-		Excludes:    []string{".wh..wh.*"},
+		Compression:     archive.Uncompressed,
+		ExcludePatterns: []string{".wh..wh.*"},
 	})
 }
 

docs/sources/reference/builder.md

@@ -154,6 +154,12 @@ Exclusion patterns match files or directories relative to the source repository
 that will be excluded from the context. Globbing is done using Go's
 [filepath.Match](http://golang.org/pkg/path/filepath#Match) rules.
 
+> **Note**:
+> The `.dockerignore` file can even be used to ignore the `Dockerfile` and
+> `.dockerignore` files. This might be useful if you are copying files from
+> the root of the build context into your new containter but do not want to 
+> include the `Dockerfile` or `.dockerignore` files (e.g. `ADD . /someDir/`).
+
 The following example shows the use of the `.dockerignore` file to exclude the
 `.git` directory from the context. Its effect can be seen in the changed size of
 the uploaded context.

graph/load.go

@@ -57,7 +57,7 @@ func (s *TagStore) CmdLoad(job *engine.Job) engine.Status {
 		excludes[i] = k
 		i++
 	}
-	if err := chrootarchive.Untar(repoFile, repoDir, &archive.TarOptions{Excludes: excludes}); err != nil {
+	if err := chrootarchive.Untar(repoFile, repoDir, &archive.TarOptions{ExcludePatterns: excludes}); err != nil {
 		return job.Error(err)
 	}
 

integration-cli/docker_cli_build_test.go

@@ -3131,28 +3131,106 @@ func TestBuildDockerignoringDockerfile(t *testing.T) {
 	name := "testbuilddockerignoredockerfile"
 	defer deleteImages(name)
 	dockerfile := `
-        FROM scratch`
+        FROM busybox
+		ADD . /tmp/
+		RUN ! ls /tmp/Dockerfile
+		RUN ls /tmp/.dockerignore`
 	ctx, err := fakeContext(dockerfile, map[string]string{
-		"Dockerfile":    "FROM scratch",
+		"Dockerfile":    dockerfile,
 		".dockerignore": "Dockerfile\n",
 	})
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer ctx.Close()
-	if _, err = buildImageFromContext(name, ctx, true); err == nil {
-		t.Fatalf("Didn't get expected error from ignoring Dockerfile")
+	if _, err = buildImageFromContext(name, ctx, true); err != nil {
+		t.Fatalf("Didn't ignore Dockerfile correctly:%s", err)
 	}
 
 	// now try it with ./Dockerfile
 	ctx.Add(".dockerignore", "./Dockerfile\n")
-	if _, err = buildImageFromContext(name, ctx, true); err == nil {
-		t.Fatalf("Didn't get expected error from ignoring ./Dockerfile")
+	if _, err = buildImageFromContext(name, ctx, true); err != nil {
+		t.Fatalf("Didn't ignore ./Dockerfile correctly:%s", err)
 	}
 
 	logDone("build - test .dockerignore of Dockerfile")
 }
 
+func TestBuildDockerignoringDockerignore(t *testing.T) {
+	name := "testbuilddockerignoredockerignore"
+	defer deleteImages(name)
+	dockerfile := `
+        FROM busybox
+		ADD . /tmp/
+		RUN ! ls /tmp/.dockerignore
+		RUN ls /tmp/Dockerfile`
+	ctx, err := fakeContext(dockerfile, map[string]string{
+		"Dockerfile":    dockerfile,
+		".dockerignore": ".dockerignore\n",
+	})
+	defer ctx.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err = buildImageFromContext(name, ctx, true); err != nil {
+		t.Fatalf("Didn't ignore .dockerignore correctly:%s", err)
+	}
+	logDone("build - test .dockerignore of .dockerignore")
+}
+
+func TestBuildDockerignoreTouchDockerfile(t *testing.T) {
+	var id1 string
+	var id2 string
+
+	name := "testbuilddockerignoretouchdockerfile"
+	defer deleteImages(name)
+	dockerfile := `
+        FROM busybox
+		ADD . /tmp/`
+	ctx, err := fakeContext(dockerfile, map[string]string{
+		"Dockerfile":    dockerfile,
+		".dockerignore": "Dockerfile\n",
+	})
+	defer ctx.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if id1, err = buildImageFromContext(name, ctx, true); err != nil {
+		t.Fatalf("Didn't build it correctly:%s", err)
+	}
+
+	if id2, err = buildImageFromContext(name, ctx, true); err != nil {
+		t.Fatalf("Didn't build it correctly:%s", err)
+	}
+	if id1 != id2 {
+		t.Fatalf("Didn't use the cache - 1")
+	}
+
+	// Now make sure touching Dockerfile doesn't invalidate the cache
+	if err = ctx.Add("Dockerfile", dockerfile+"\n# hi"); err != nil {
+		t.Fatalf("Didn't add Dockerfile: %s", err)
+	}
+	if id2, err = buildImageFromContext(name, ctx, true); err != nil {
+		t.Fatalf("Didn't build it correctly:%s", err)
+	}
+	if id1 != id2 {
+		t.Fatalf("Didn't use the cache - 2")
+	}
+
+	// One more time but just 'touch' it instead of changing the content
+	if err = ctx.Add("Dockerfile", dockerfile+"\n# hi"); err != nil {
+		t.Fatalf("Didn't add Dockerfile: %s", err)
+	}
+	if id2, err = buildImageFromContext(name, ctx, true); err != nil {
+		t.Fatalf("Didn't build it correctly:%s", err)
+	}
+	if id1 != id2 {
+		t.Fatalf("Didn't use the cache - 3")
+	}
+
+	logDone("build - test .dockerignore touch dockerfile")
+}
+
 func TestBuildDockerignoringWholeDir(t *testing.T) {
 	name := "testbuilddockerignorewholedir"
 	defer deleteImages(name)

integration-cli/docker_cli_events_test.go

@@ -10,14 +10,13 @@ import (
 )
 
 func TestEventsUntag(t *testing.T) {
-	out, _, _ := dockerCmd(t, "images", "-q")
-	image := strings.Split(out, "\n")[0]
+	image := "busybox"
 	dockerCmd(t, "tag", image, "utest:tag1")
 	dockerCmd(t, "tag", image, "utest:tag2")
 	dockerCmd(t, "rmi", "utest:tag1")
 	dockerCmd(t, "rmi", "utest:tag2")
 	eventsCmd := exec.Command("timeout", "0.2", dockerBinary, "events", "--since=1")
-	out, _, _ = runCommandWithOutput(eventsCmd)
+	out, _, _ := runCommandWithOutput(eventsCmd)
 	events := strings.Split(out, "\n")
 	nEvents := len(events)
 	// The last element after the split above will be an empty string, so we

pkg/archive/archive.go

@@ -30,11 +30,11 @@ type (
 	ArchiveReader io.Reader
 	Compression   int
 	TarOptions    struct {
-		Includes    []string
-		Excludes    []string
-		Compression Compression
-		NoLchown    bool
-		Name        string
+		IncludeFiles    []string
+		ExcludePatterns []string
+		Compression     Compression
+		NoLchown        bool
+		Name            string
 	}
 
 	// Archiver allows the reuse of most utility functions of this package
@@ -378,7 +378,7 @@ func escapeName(name string) string {
 }
 
 // TarWithOptions creates an archive from the directory at `path`, only including files whose relative
-// paths are included in `options.Includes` (if non-nil) or not in `options.Excludes`.
+// paths are included in `options.IncludeFiles` (if non-nil) or not in `options.ExcludePatterns`.
 func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error) {
 	pipeReader, pipeWriter := io.Pipe()
 
@@ -401,12 +401,14 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error)
 		// mutating the filesystem and we can see transient errors
 		// from this
 
-		if options.Includes == nil {
-			options.Includes = []string{"."}
+		if options.IncludeFiles == nil {
+			options.IncludeFiles = []string{"."}
 		}
 
+		seen := make(map[string]bool)
+
 		var renamedRelFilePath string // For when tar.Options.Name is set
-		for _, include := range options.Includes {
+		for _, include := range options.IncludeFiles {
 			filepath.Walk(filepath.Join(srcPath, include), func(filePath string, f os.FileInfo, err error) error {
 				if err != nil {
 					log.Debugf("Tar: Can't stat file %s to tar: %s", srcPath, err)
@@ -420,10 +422,19 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error)
 					return nil
 				}
 
-				skip, err := fileutils.Matches(relFilePath, options.Excludes)
-				if err != nil {
-					log.Debugf("Error matching %s", relFilePath, err)
-					return err
+				skip := false
+
+				// If "include" is an exact match for the current file
+				// then even if there's an "excludePatterns" pattern that
+				// matches it, don't skip it. IOW, assume an explicit 'include'
+				// is asking for that file no matter what - which is true
+				// for some files, like .dockerignore and Dockerfile (sometimes)
+				if include != relFilePath {
+					skip, err = fileutils.Matches(relFilePath, options.ExcludePatterns)
+					if err != nil {
+						log.Debugf("Error matching %s", relFilePath, err)
+						return err
+					}
 				}
 
 				if skip {
@@ -433,6 +444,11 @@ func TarWithOptions(srcPath string, options *TarOptions) (io.ReadCloser, error)
 					return nil
 				}
 
+				if seen[relFilePath] {
+					return nil
+				}
+				seen[relFilePath] = true
+
 				// Rename the base resource
 				if options.Name != "" && filePath == srcPath+"/"+filepath.Base(relFilePath) {
 					renamedRelFilePath = relFilePath
@@ -487,7 +503,7 @@ loop:
 		// This keeps "../" as-is, but normalizes "/../" to "/"
 		hdr.Name = filepath.Clean(hdr.Name)
 
-		for _, exclude := range options.Excludes {
+		for _, exclude := range options.ExcludePatterns {
 			if strings.HasPrefix(hdr.Name, exclude) {
 				continue loop
 			}
@@ -563,8 +579,8 @@ func Untar(archive io.Reader, dest string, options *TarOptions) error {
 	if options == nil {
 		options = &TarOptions{}
 	}
-	if options.Excludes == nil {
-		options.Excludes = []string{}
+	if options.ExcludePatterns == nil {
+		options.ExcludePatterns = []string{}
 	}
 	decompressedArchive, err := DecompressStream(archive)
 	if err != nil {

pkg/archive/archive_test.go

@@ -165,8 +165,8 @@ func TestTarUntar(t *testing.T) {
 		Gzip,
 	} {
 		changes, err := tarUntar(t, origin, &TarOptions{
-			Compression: c,
-			Excludes:    []string{"3"},
+			Compression:     c,
+			ExcludePatterns: []string{"3"},
 		})
 
 		if err != nil {
@@ -196,8 +196,8 @@ func TestTarWithOptions(t *testing.T) {
 		opts       *TarOptions
 		numChanges int
 	}{
-		{&TarOptions{Includes: []string{"1"}}, 1},
-		{&TarOptions{Excludes: []string{"2"}}, 1},
+		{&TarOptions{IncludeFiles: []string{"1"}}, 1},
+		{&TarOptions{ExcludePatterns: []string{"2"}}, 1},
 	}
 	for _, testCase := range cases {
 		changes, err := tarUntar(t, origin, testCase.opts)

pkg/chrootarchive/archive.go

@@ -50,8 +50,8 @@ func Untar(tarArchive io.Reader, dest string, options *archive.TarOptions) error
 	if options == nil {
 		options = &archive.TarOptions{}
 	}
-	if options.Excludes == nil {
-		options.Excludes = []string{}
+	if options.ExcludePatterns == nil {
+		options.ExcludePatterns = []string{}
 	}
 
 	var (

pkg/chrootarchive/archive_test.go

@@ -40,7 +40,7 @@ func TestChrootTarUntar(t *testing.T) {
 	if err := os.MkdirAll(dest, 0700); err != nil {
 		t.Fatal(err)
 	}
-	if err := Untar(stream, dest, &archive.TarOptions{Excludes: []string{"lolo"}}); err != nil {
+	if err := Untar(stream, dest, &archive.TarOptions{ExcludePatterns: []string{"lolo"}}); err != nil {
 		t.Fatal(err)
 	}
 }

pkg/tarsum/builder_context.go

@@ -0,0 +1,20 @@
+package tarsum
+
+// This interface extends TarSum by adding the Remove method.  In general
+// there was concern about adding this method to TarSum itself so instead
+// it is being added just to "BuilderContext" which will then only be used
+// during the .dockerignore file processing - see builder/evaluator.go
+type BuilderContext interface {
+	TarSum
+	Remove(string)
+}
+
+func (bc *tarSum) Remove(filename string) {
+	for i, fis := range bc.sums {
+		if fis.Name() == filename {
+			bc.sums = append(bc.sums[:i], bc.sums[i+1:]...)
+			// Note, we don't just return because there could be
+			// more than one with this name
+		}
+	}
+}

utils/utils.go

@@ -1,6 +1,7 @@
 package utils
 
 import (
+	"bufio"
 	"bytes"
 	"crypto/rand"
 	"crypto/sha1"
@@ -492,3 +493,34 @@ func StringsContainsNoCase(slice []string, s string) bool {
 	}
 	return false
 }
+
+// Reads a .dockerignore file and returns the list of file patterns
+// to ignore. Note this will trim whitespace from each line as well
+// as use GO's "clean" func to get the shortest/cleanest path for each.
+func ReadDockerIgnore(path string) ([]string, error) {
+	// Note that a missing .dockerignore file isn't treated as an error
+	reader, err := os.Open(path)
+	if err != nil {
+		if !os.IsNotExist(err) {
+			return nil, fmt.Errorf("Error reading '%s': %v", path, err)
+		}
+		return nil, nil
+	}
+	defer reader.Close()
+
+	scanner := bufio.NewScanner(reader)
+	var excludes []string
+
+	for scanner.Scan() {
+		pattern := strings.TrimSpace(scanner.Text())
+		if pattern == "" {
+			continue
+		}
+		pattern = filepath.Clean(pattern)
+		excludes = append(excludes, pattern)
+	}
+	if err = scanner.Err(); err != nil {
+		return nil, fmt.Errorf("Error reading '%s': %v", path, err)
+	}
+	return excludes, nil
+}

volumes/volume.go

@@ -47,9 +47,9 @@ func (v *Volume) Export(resource, name string) (io.ReadCloser, error) {
 		basePath = path.Dir(basePath)
 	}
 	return archive.TarWithOptions(basePath, &archive.TarOptions{
-		Compression: archive.Uncompressed,
-		Name:        name,
-		Includes:    filter,
+		Compression:  archive.Uncompressed,
+		Name:         name,
+		IncludeFiles: filter,
 	})
 }