Never ignore root directory when enumerating files in a repository (#683)

## Changes

The pattern `.*` in a `.gitignore` file can match `.` when walking all
files in a repository. If it does, then the walker immediately aborts
and no files are returned. The root directory (an unnamed directory)
must never be ignored.

Reported in https://github.com/databricks/databricks-vscode/issues/837.

## Tests

New tests pass.
This commit is contained in:
Pieter Noordhuis 2023-08-21 09:35:02 +02:00 committed by GitHub
parent ffc78b4b8b
commit c25bc041b1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 37 additions and 22 deletions

View File

@ -39,14 +39,14 @@ func (w *FileSet) Root() string {
// Return all tracked files for Repo // Return all tracked files for Repo
func (w *FileSet) All() ([]File, error) { func (w *FileSet) All() ([]File, error) {
return w.RecursiveListFiles(w.root) return w.recursiveListFiles()
} }
// Recursively traverses dir in a depth first manner and returns a list of all files // Recursively traverses dir in a depth first manner and returns a list of all files
// that are being tracked in the FileSet (ie not being ignored for matching one of the // that are being tracked in the FileSet (ie not being ignored for matching one of the
// patterns in w.ignore) // patterns in w.ignore)
func (w *FileSet) RecursiveListFiles(dir string) (fileList []File, err error) { func (w *FileSet) recursiveListFiles() (fileList []File, err error) {
err = filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error { err = filepath.WalkDir(w.root, func(path string, d fs.DirEntry, err error) error {
if err != nil { if err != nil {
return err return err
} }

View File

@ -6,7 +6,7 @@ import (
// FileSet is Git repository aware implementation of [fileset.FileSet]. // FileSet is Git repository aware implementation of [fileset.FileSet].
// It forces checking if gitignore files have been modified every // It forces checking if gitignore files have been modified every
// time a call to [FileSet.All] or [FileSet.RecursiveListFiles] is made. // time a call to [FileSet.All] is made.
type FileSet struct { type FileSet struct {
fileset *fileset.FileSet fileset *fileset.FileSet
view *View view *View
@ -43,11 +43,6 @@ func (f *FileSet) All() ([]fileset.File, error) {
return f.fileset.All() return f.fileset.All()
} }
func (f *FileSet) RecursiveListFiles(dir string) ([]fileset.File, error) {
f.view.repo.taintIgnoreRules()
return f.fileset.RecursiveListFiles(dir)
}
func (f *FileSet) EnsureValidGitIgnoreExists() error { func (f *FileSet) EnsureValidGitIgnoreExists() error {
return f.view.EnsureValidGitIgnoreExists() return f.view.EnsureValidGitIgnoreExists()
} }

View File

@ -10,18 +10,23 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
func TestFileSetRecursiveListFiles(t *testing.T) { func testFileSetAll(t *testing.T, path string) {
fileSet, err := NewFileSet("./testdata") fileSet, err := NewFileSet(path)
require.NoError(t, err) require.NoError(t, err)
files, err := fileSet.RecursiveListFiles("./testdata") files, err := fileSet.All()
require.NoError(t, err) require.NoError(t, err)
require.Len(t, files, 6) require.Len(t, files, 3)
assert.Equal(t, filepath.Join(".gitignore"), files[0].Relative) assert.Equal(t, filepath.Join("a", "b", "world.txt"), files[0].Relative)
assert.Equal(t, filepath.Join("a", ".gitignore"), files[1].Relative) assert.Equal(t, filepath.Join("a", "hello.txt"), files[1].Relative)
assert.Equal(t, filepath.Join("a", "b", ".gitignore"), files[2].Relative) assert.Equal(t, filepath.Join("databricks.yml"), files[2].Relative)
assert.Equal(t, filepath.Join("a", "b", "world.txt"), files[3].Relative) }
assert.Equal(t, filepath.Join("a", "hello.txt"), files[4].Relative)
assert.Equal(t, filepath.Join("databricks.yml"), files[5].Relative) func TestFileSetListAllInRepo(t *testing.T) {
testFileSetAll(t, "./testdata")
}
func TestFileSetListAllInTempDir(t *testing.T) {
testFileSetAll(t, copyTestdata(t, "./testdata"))
} }
func TestFileSetNonCleanRoot(t *testing.T) { func TestFileSetNonCleanRoot(t *testing.T) {
@ -32,10 +37,10 @@ func TestFileSetNonCleanRoot(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
files, err := fileSet.All() files, err := fileSet.All()
require.NoError(t, err) require.NoError(t, err)
assert.Len(t, files, 6) assert.Len(t, files, 3)
} }
func TestFilesetAddsCacheDirToGitIgnore(t *testing.T) { func TestFileSetAddsCacheDirToGitIgnore(t *testing.T) {
projectDir := t.TempDir() projectDir := t.TempDir()
fileSet, err := NewFileSet(projectDir) fileSet, err := NewFileSet(projectDir)
require.NoError(t, err) require.NoError(t, err)
@ -48,7 +53,7 @@ func TestFilesetAddsCacheDirToGitIgnore(t *testing.T) {
assert.Contains(t, string(fileBytes), ".databricks") assert.Contains(t, string(fileBytes), ".databricks")
} }
func TestFilesetDoesNotCacheDirToGitIgnoreIfAlreadyPresent(t *testing.T) { func TestFileSetDoesNotCacheDirToGitIgnoreIfAlreadyPresent(t *testing.T) {
projectDir := t.TempDir() projectDir := t.TempDir()
gitIgnorePath := filepath.Join(projectDir, ".gitignore") gitIgnorePath := filepath.Join(projectDir, ".gitignore")

View File

@ -160,6 +160,11 @@ func (r *Repository) Ignore(relPath string) (bool, error) {
trailingSlash = "/" trailingSlash = "/"
} }
// Never ignore the root path (an unnamed path)
if len(parts) == 1 && parts[0] == "." {
return false, nil
}
// Walk over path prefixes to check applicable gitignore files. // Walk over path prefixes to check applicable gitignore files.
for i := range parts { for i := range parts {
prefix := path.Clean(strings.Join(parts[:i], "/")) prefix := path.Clean(strings.Join(parts[:i], "/"))

View File

@ -7,3 +7,6 @@ root.*
# Directory pattern. # Directory pattern.
ignoredirectory/ ignoredirectory/
# Ignore dotfiles
.*

View File

@ -68,8 +68,15 @@ func testViewAtRoot(t *testing.T, tv testView) {
assert.True(t, tv.Ignore("root/foo")) assert.True(t, tv.Ignore("root/foo"))
assert.True(t, tv.Ignore("root_double")) assert.True(t, tv.Ignore("root_double"))
assert.False(t, tv.Ignore("newfile")) assert.False(t, tv.Ignore("newfile"))
assert.True(t, tv.Ignore(".gitignore"))
assert.False(t, tv.Ignore("newfile.py"))
assert.True(t, tv.Ignore("ignoredirectory/")) assert.True(t, tv.Ignore("ignoredirectory/"))
// Never ignore the root directory.
// This is the only path that may be checked as `.`,
// and would match the `.*` ignore pattern if specified.
assert.False(t, tv.Ignore("."))
// Nested .gitignores should not affect root. // Nested .gitignores should not affect root.
assert.False(t, tv.Ignore("a.sh")) assert.False(t, tv.Ignore("a.sh"))