diff --git a/api/client/commands.go b/api/client/commands.go index e666d43205..14b01f5603 100644 --- a/api/client/commands.go +++ b/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" @@ -143,32 +143,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 { diff --git a/builder/evaluator.go b/builder/evaluator.go index eef222b943..43fb419fce 100644 --- a/builder/evaluator.go +++ b/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: diff --git a/builder/internals.go b/builder/internals.go index 1caa33141c..909e7a8d10 100644 --- a/builder/internals.go +++ b/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()) } } diff --git a/daemon/container.go b/daemon/container.go index 75cd133fec..45aaa8be59 100644 --- a/daemon/container.go +++ b/daemon/container.go @@ -891,8 +891,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() diff --git a/daemon/graphdriver/aufs/aufs.go b/daemon/graphdriver/aufs/aufs.go index 82a5c89051..220be2de55 100644 --- a/daemon/graphdriver/aufs/aufs.go +++ b/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.*"}, }) } diff --git a/docs/sources/reference/builder.md b/docs/sources/reference/builder.md index fa7393efa3..90862334ef 100644 --- a/docs/sources/reference/builder.md +++ b/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. diff --git a/graph/load.go b/graph/load.go index 6ef219c077..4399da25d9 100644 --- a/graph/load.go +++ b/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) } diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index bb53b68acc..c4d7ff7c54 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/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) diff --git a/integration-cli/docker_cli_events_test.go b/integration-cli/docker_cli_events_test.go index be6f1202c4..322d622b55 100644 --- a/integration-cli/docker_cli_events_test.go +++ b/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 diff --git a/pkg/archive/archive.go b/pkg/archive/archive.go index ec45d8546d..35566520b1 100644 --- a/pkg/archive/archive.go +++ b/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 { diff --git a/pkg/archive/archive_test.go b/pkg/archive/archive_test.go index fdba6fb87c..6cd95d5ad5 100644 --- a/pkg/archive/archive_test.go +++ b/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) diff --git a/pkg/chrootarchive/archive.go b/pkg/chrootarchive/archive.go index 66f837314a..ae15a2a54d 100644 --- a/pkg/chrootarchive/archive.go +++ b/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 ( diff --git a/pkg/chrootarchive/archive_test.go b/pkg/chrootarchive/archive_test.go index bb8a22dc70..b3f7d57688 100644 --- a/pkg/chrootarchive/archive_test.go +++ b/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) } } diff --git a/pkg/tarsum/builder_context.go b/pkg/tarsum/builder_context.go new file mode 100644 index 0000000000..06a42825e7 --- /dev/null +++ b/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 + } + } +} diff --git a/utils/utils.go b/utils/utils.go index 8d3b3eb73e..4e36b4061e 100644 --- a/utils/utils.go +++ b/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 +} diff --git a/volumes/volume.go b/volumes/volume.go index d718b07d70..db99aed5dc 100644 --- a/volumes/volume.go +++ b/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, }) }