Skip to content

Commit

Permalink
dockerignore: convert ignore patterns to absolute paths [ch9237] (#3743)
Browse files Browse the repository at this point in the history
In most places in Tilt, we try to use absolute paths everywhere.
So this makes things more consistent with the rest of Tilt, and lets us be
a bit more flexible in how we handle subdirs and parent dirs in ignores.

Fixes #3740
  • Loading branch information
nicks committed Sep 2, 2020
1 parent 780867f commit 65794cd
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 19 deletions.
43 changes: 36 additions & 7 deletions internal/dockerignore/ignore.go
@@ -1,6 +1,7 @@
package dockerignore

import (
"fmt"
"os"
"path/filepath"
"strings"
Expand All @@ -17,11 +18,10 @@ type dockerPathMatcher struct {
}

func (i dockerPathMatcher) Matches(f string) (bool, error) {
rp, err := filepath.Rel(i.repoRoot, f)
if err != nil {
return false, err
if !filepath.IsAbs(f) {
f = filepath.Join(i.repoRoot, f)
}
return i.matcher.Matches(rp)
return i.matcher.Matches(f)
}

func (i dockerPathMatcher) MatchesEntireDir(f string) (bool, error) {
Expand All @@ -36,8 +36,7 @@ func (i dockerPathMatcher) MatchesEntireDir(f string) (bool, error) {
if !pattern.Exclusion() {
continue
}
absPattern := filepath.Join(i.repoRoot, pattern.String())
if ospath.IsChild(f, absPattern) {
if ospath.IsChild(f, pattern.String()) {
// Found an exclusion match -- we don't match this whole dir
return false, nil
}
Expand All @@ -61,13 +60,43 @@ func NewDockerIgnoreTester(repoRoot string) (*dockerPathMatcher, error) {
return NewDockerPatternMatcher(absRoot, patterns)
}

// Make all the patterns use absolute paths.
func absPatterns(absRoot string, patterns []string) []string {
absPatterns := make([]string, 0, len(patterns))
for _, p := range patterns {
// The pattern parsing here is loosely adapted from fileutils' NewPatternMatcher
p = strings.TrimSpace(p)
if p == "" {
continue
}
p = filepath.Clean(p)

pPath := p
isExclusion := false
if p[0] == '!' {
pPath = p[1:]
isExclusion = true
}

if !filepath.IsAbs(pPath) {
pPath = filepath.Join(absRoot, pPath)
}
absPattern := pPath
if isExclusion {
absPattern = fmt.Sprintf("!%s", pPath)
}
absPatterns = append(absPatterns, absPattern)
}
return absPatterns
}

func NewDockerPatternMatcher(repoRoot string, patterns []string) (*dockerPathMatcher, error) {
absRoot, err := filepath.Abs(repoRoot)
if err != nil {
return nil, err
}

pm, err := fileutils.NewPatternMatcher(patterns)
pm, err := fileutils.NewPatternMatcher(absPatterns(absRoot, patterns))
if err != nil {
return nil, err
}
Expand Down
35 changes: 35 additions & 0 deletions internal/dockerignore/ignore_test.go
Expand Up @@ -34,6 +34,40 @@ func TestGlob(t *testing.T) {
tf.AssertResult(tf.JoinPath("somedir", "temp"), true)
}

func TestCurrentDirDoubleGlob(t *testing.T) {
tf := newTestFixture(t, "**/temp*")
defer tf.TearDown()
tf.AssertResult(tf.JoinPath("a", "temporary.txt"), true)
tf.AssertResult(tf.JoinPath("a", "b", "temporary.txt"), true)
tf.AssertResult(tf.JoinPath("a", "b", "c", "temporary.txt"), true)
tf.AssertResult(tf.JoinPath("b", "c", "temporary.txt"), true)
tf.AssertResult(tf.JoinPath("..", "a", "b", "temporary.txt"), false)
}

func TestInnerDirDoubleGlob(t *testing.T) {
tf := newTestFixture(t, "a/**/temp*")
defer tf.TearDown()
tf.AssertResult(tf.JoinPath("a", "temporary.txt"), true)
tf.AssertResult(tf.JoinPath("a", "b", "temporary.txt"), true)
tf.AssertResult(tf.JoinPath("a", "b", "c", "temporary.txt"), true)
tf.AssertResult(tf.JoinPath("b", "c", "temporary.txt"), false)
tf.AssertResult(tf.JoinPath("..", "a", "b", "temporary.txt"), false)
}

func TestUplevel(t *testing.T) {
tf := newTestFixture(t, "../a/b.txt")
defer tf.TearDown()
tf.AssertResult(tf.JoinPath("..", "a", "b.txt"), true)
tf.AssertResult(tf.JoinPath("a", "b.txt"), false)
}

func TestUplevelDoubleGlob(t *testing.T) {
tf := newTestFixture(t, "../**/b.txt")
defer tf.TearDown()
tf.AssertResult(tf.JoinPath("..", "a", "b.txt"), true)
tf.AssertResult(tf.JoinPath("a", "b.txt"), true)
}

func TestOneCharacterExtension(t *testing.T) {
tf := newTestFixture(t, "temp?")
defer tf.TearDown()
Expand Down Expand Up @@ -106,6 +140,7 @@ func (tf *testFixture) JoinPath(path ...string) string {
}

func (tf *testFixture) AssertResult(path string, expectedMatches bool) {
tf.t.Helper()
isIgnored, err := tf.tester.Matches(path)
if err != nil {
tf.t.Fatal(err)
Expand Down
15 changes: 3 additions & 12 deletions internal/watch/notify_test.go
Expand Up @@ -421,9 +421,6 @@ func TestWatchNonexistentDirectory(t *testing.T) {
f := newNotifyFixture(t)
defer f.tearDown()

ignore, _ := dockerignore.NewDockerPatternMatcher(f.paths[0], []string{"./"})
f.setIgnore(ignore)

root := f.JoinPath("root")
err := os.Mkdir(root, 0777)
if err != nil {
Expand All @@ -441,12 +438,9 @@ func TestWatchNonexistentDirectory(t *testing.T) {
t.Fatal(err)
}

if runtime.GOOS == "darwin" {
// for directories that were the root of an Add, we don't report creation, cf. watcher_darwin.go
f.assertEvents()
} else {
f.assertEvents(parent)
}
// for directories that were the root of an Add, we don't report creation, cf. watcher_darwin.go
f.assertEvents()

f.events = nil
f.WriteFile(file, "hello")

Expand All @@ -457,9 +451,6 @@ func TestWatchNonexistentFileInNonexistentDirectory(t *testing.T) {
f := newNotifyFixture(t)
defer f.tearDown()

ignore, _ := dockerignore.NewDockerPatternMatcher(f.paths[0], []string{"./"})
f.setIgnore(ignore)

root := f.JoinPath("root")
err := os.Mkdir(root, 0777)
if err != nil {
Expand Down
6 changes: 6 additions & 0 deletions internal/watch/watcher_naive.go
Expand Up @@ -214,6 +214,12 @@ func (d *naiveNotify) shouldNotify(path string) bool {
}

if _, ok := d.notifyList[path]; ok {
// We generally don't care when directories change at the root of an ADD
stat, err := os.Lstat(path)
isDir := err == nil && stat.IsDir()
if isDir {
return false
}
return true
}
// TODO(dmiller): maybe use a prefix tree here?
Expand Down

0 comments on commit 65794cd

Please sign in to comment.