libs/sync: extract FileList to decouple file listing from syncing

bundle/deploy/snapshot/path.go

@@ -74,7 +74,11 @@ func addSyncRootToZip(ctx context.Context, zw *zip.Writer, b *bundle.Bundle) (in
 	if err != nil {
 		return 0, err
 	}
-	fileList, err := libsync.GetFileList(ctx, *opts)
+	fl, err := libsync.NewFileList(ctx, opts.WorktreeRoot, opts.LocalRoot, opts.Paths, opts.Include, opts.Exclude)
+	if err != nil {
+		return 0, fmt.Errorf("build file set: %w", err)
+	}
+	fileList, err := fl.Files(ctx)
 	if err != nil {
 		return 0, err
 	}

bundle/deploy/snapshot/path_test.go

@@ -33,6 +33,10 @@ func makeBundleWithFiles(t *testing.T, files map[string]string) *bundle.Bundle {
 		WorktreeRoot: root,
 		Config: config.Root{
 			Bundle: config.Bundle{Target: "default"},
+			// The SyncDefaultPath mutator sets this to ["."] during initialize;
+			// set it here since these tests bypass the mutator pipeline. Empty
+			// sync paths select no files.
+			Sync: config.Sync{Paths: []string{"."}},
 		},
 	}
 }

bundle/deploy/snapshot/upload_warning_test.go

@@ -38,6 +38,10 @@ func makeBundle(t *testing.T, nFiles int) *bundle.Bundle {
 		WorktreeRoot:   root,
 		Config: config.Root{
 			Bundle: config.Bundle{Target: "default"},
+			// The SyncDefaultPath mutator sets this to ["."] during initialize;
+			// set it here since these tests bypass the mutator pipeline. Empty
+			// sync paths select no files.
+			Sync: config.Sync{Paths: []string{"."}},
 			Workspace: config.Workspace{
 				CurrentUser: &config.User{
 					User: &iam.User{UserName: "test@example.test"},

libs/sync/filelist.go

@@ -0,0 +1,83 @@
+package sync
+
+import (
+	"context"
+
+	"github.com/databricks/cli/libs/fileset"
+	"github.com/databricks/cli/libs/git"
+	"github.com/databricks/cli/libs/log"
+	"github.com/databricks/cli/libs/set"
+	"github.com/databricks/cli/libs/vfs"
+)
+
+// FileList enumerates the local files selected for sync: git-tracked files,
+// plus explicit includes, minus excludes. It is the "what would be synced"
+// listing concern, independent of the sync snapshot and remote upload.
+type FileList struct {
+	fileSet        *git.FileSet
+	includeFileSet *fileset.FileSet
+	excludeFileSet *fileset.FileSet
+}
+
+// NewFileList builds a FileList for the directory tree at root, located within
+// the Git worktree at worktreeRoot so gitignore rules are honored. paths selects
+// which entries under root to list; empty paths select nothing, so callers that
+// want the whole root must pass ["."] explicitly.
+func NewFileList(ctx context.Context, worktreeRoot, root vfs.Path, paths, include, exclude []string) (*FileList, error) {
+	fileSet, err := git.NewFileSet(ctx, worktreeRoot, root, paths)
+	if err != nil {
+		return nil, err
+	}
+
+	includeFileSet, err := fileset.NewGlobSet(root, include)
+	if err != nil {
+		return nil, err
+	}
+
+	excludeFileSet, err := fileset.NewGlobSet(root, exclude)
+	if err != nil {
+		return nil, err
+	}
+
+	return &FileList{
+		fileSet:        fileSet,
+		includeFileSet: includeFileSet,
+		excludeFileSet: excludeFileSet,
+	}, nil
+}
+
+// Files returns the deduped set of files (git ∪ include) − exclude.
+// Files are deduped by their relative path; the order is whatever the
+// underlying set yields.
+func (l *FileList) Files(ctx context.Context) ([]fileset.File, error) {
+	all := set.NewSetF(func(f fileset.File) string {
+		return f.Relative
+	})
+	gitFiles, err := l.fileSet.Files()
+	if err != nil {
+		log.Errorf(ctx, "cannot list files: %s", err)
+		return nil, err
+	}
+
+	all.Add(gitFiles...)
+
+	include, err := l.includeFileSet.Files()
+	if err != nil {
+		log.Errorf(ctx, "cannot list include files: %s", err)
+		return nil, err
+	}
+
+	all.Add(include...)
+
+	exclude, err := l.excludeFileSet.Files()
+	if err != nil {
+		log.Errorf(ctx, "cannot list exclude files: %s", err)
+		return nil, err
+	}
+
+	for _, f := range exclude {
+		all.Remove(f)
+	}
+
+	return all.Iter(), nil
+}

libs/sync/filelist_test.go

@@ -0,0 +1,137 @@
+package sync
+
+import (
+	"path/filepath"
+	"slices"
+	"testing"
+
+	"github.com/databricks/cli/internal/testutil"
+	"github.com/databricks/cli/libs/fileset"
+	"github.com/databricks/cli/libs/vfs"
+	"github.com/stretchr/testify/require"
+)
+
+func setupFiles(t *testing.T) string {
+	dir := t.TempDir()
+
+	for _, f := range []([]string){
+		[]string{dir, "a.go"},
+		[]string{dir, "b.go"},
+		[]string{dir, "ab.go"},
+		[]string{dir, "abc.go"},
+		[]string{dir, "c.go"},
+		[]string{dir, "d.go"},
+		[]string{dir, ".databricks", "e.go"},
+		[]string{dir, "test", "sub1", "f.go"},
+		[]string{dir, "test", "sub1", "sub2", "g.go"},
+		[]string{dir, "test", "sub1", "sub2", "h.txt"},
+	} {
+		testutil.Touch(t, f...)
+	}
+
+	return dir
+}
+
+func relativePaths(files []fileset.File) []string {
+	paths := make([]string, 0, len(files))
+	for _, f := range files {
+		paths = append(paths, f.Relative)
+	}
+	slices.Sort(paths)
+	return paths
+}
+
+func TestFileListFiles(t *testing.T) {
+	ctx := t.Context()
+
+	dir := setupFiles(t)
+	root := vfs.MustNew(dir)
+
+	l, err := NewFileList(ctx, root, root, []string{"."}, []string{}, []string{})
+	require.NoError(t, err)
+
+	fileList, err := l.Files(ctx)
+	require.NoError(t, err)
+	require.Len(t, fileList, 9)
+
+	l, err = NewFileList(ctx, root, root, []string{"."}, []string{}, []string{"*.go"})
+	require.NoError(t, err)
+
+	fileList, err = l.Files(ctx)
+	require.NoError(t, err)
+	require.Len(t, fileList, 1)
+
+	l, err = NewFileList(ctx, root, root, []string{"."}, []string{"./.databricks/*.go"}, []string{})
+	require.NoError(t, err)
+
+	fileList, err = l.Files(ctx)
+	require.NoError(t, err)
+	require.Len(t, fileList, 10)
+}
+
+func TestRecursiveExclude(t *testing.T) {
+	ctx := t.Context()
+
+	dir := setupFiles(t)
+	root := vfs.MustNew(dir)
+
+	l, err := NewFileList(ctx, root, root, []string{"."}, []string{}, []string{"test/**"})
+	require.NoError(t, err)
+
+	fileList, err := l.Files(ctx)
+	require.NoError(t, err)
+	require.Len(t, fileList, 6)
+}
+
+func TestNegateExclude(t *testing.T) {
+	ctx := t.Context()
+
+	dir := setupFiles(t)
+	root := vfs.MustNew(dir)
+
+	l, err := NewFileList(ctx, root, root, []string{"."}, []string{}, []string{"./*", "!*.txt"})
+	require.NoError(t, err)
+
+	fileList, err := l.Files(ctx)
+	require.NoError(t, err)
+	require.Len(t, fileList, 1)
+	require.Equal(t, "test/sub1/sub2/h.txt", fileList[0].Relative)
+}
+
+// TestFileListNestedRoot pins the two-root semantics: gitignore rules at the
+// worktree root must govern a sync root nested below it. If NewFileList treated
+// the sync root as the worktree root, the parent .gitignore would not be loaded
+// and ignored.go would leak into the listing.
+func TestFileListNestedRoot(t *testing.T) {
+	ctx := t.Context()
+
+	dir := t.TempDir()
+	testutil.WriteFile(t, filepath.Join(dir, ".gitignore"), "ignored.go\n")
+	testutil.Touch(t, dir, "sub", "keep.go")
+	testutil.Touch(t, dir, "sub", "ignored.go")
+
+	worktreeRoot := vfs.MustNew(dir)
+	syncRoot := vfs.MustNew(filepath.Join(dir, "sub"))
+
+	l, err := NewFileList(ctx, worktreeRoot, syncRoot, []string{"."}, nil, nil)
+	require.NoError(t, err)
+
+	fileList, err := l.Files(ctx)
+	require.NoError(t, err)
+	require.Equal(t, []string{"keep.go"}, relativePaths(fileList))
+}
+
+// TestFileListEmptyPathsIsNop pins that empty paths select no files: callers
+// that want the whole root must say so with ["."]. There is no implicit default.
+func TestFileListEmptyPathsIsNop(t *testing.T) {
+	ctx := t.Context()
+
+	dir := setupFiles(t)
+	root := vfs.MustNew(dir)
+
+	l, err := NewFileList(ctx, root, root, nil, nil, nil)
+	require.NoError(t, err)
+	files, err := l.Files(ctx)
+	require.NoError(t, err)
+	require.Empty(t, files)
+}

libs/sync/sync.go

@@ -9,9 +9,7 @@ import (
 
 	"github.com/databricks/cli/libs/filer"
 	"github.com/databricks/cli/libs/fileset"
-	"github.com/databricks/cli/libs/git"
 	"github.com/databricks/cli/libs/log"
-	"github.com/databricks/cli/libs/set"
 	"github.com/databricks/cli/libs/vfs"
 	"github.com/databricks/databricks-sdk-go"
 	"github.com/databricks/databricks-sdk-go/service/iam"
@@ -48,9 +46,7 @@ type SyncOptions struct {
 type Sync struct {
 	*SyncOptions
 
-	fileSet        *git.FileSet
-	includeFileSet *fileset.FileSet
-	excludeFileSet *fileset.FileSet
+	fileList *FileList
 
 	snapshot *Snapshot
 	filer    filer.Filer
@@ -66,23 +62,13 @@ type Sync struct {
 
 // New initializes and returns a new [Sync] instance.
 func New(ctx context.Context, opts SyncOptions) (*Sync, error) {
-	fileSet, err := git.NewFileSet(ctx, opts.WorktreeRoot, opts.LocalRoot, opts.Paths)
+	fileList, err := NewFileList(ctx, opts.WorktreeRoot, opts.LocalRoot, opts.Paths, opts.Include, opts.Exclude)
 	if err != nil {
 		return nil, err
 	}
 
 	WriteGitIgnore(ctx, opts.LocalRoot.Native())
 
-	includeFileSet, err := fileset.NewGlobSet(opts.LocalRoot, opts.Include)
-	if err != nil {
-		return nil, err
-	}
-
-	excludeFileSet, err := fileset.NewGlobSet(opts.LocalRoot, opts.Exclude)
-	if err != nil {
-		return nil, err
-	}
-
 	// Verify that the remote path we're about to synchronize to is valid and allowed.
 	err = EnsureRemotePathIsUsable(ctx, opts.WorkspaceClient, opts.RemotePath, opts.CurrentUser, opts.DryRun)
 	if err != nil {
@@ -131,9 +117,7 @@ func New(ctx context.Context, opts SyncOptions) (*Sync, error) {
 	return &Sync{
 		SyncOptions: &opts,
 
-		fileSet:         fileSet,
-		includeFileSet:  includeFileSet,
-		excludeFileSet:  excludeFileSet,
+		fileList:        fileList,
 		snapshot:        snapshot,
 		filer:           filer,
 		notifier:        notifier,
@@ -211,67 +195,9 @@ func (s *Sync) RunOnce(ctx context.Context) ([]fileset.File, error) {
 	return files, nil
 }
 
+// GetFileList returns the local files selected for sync, without syncing them.
 func (s *Sync) GetFileList(ctx context.Context) ([]fileset.File, error) {
-	// tradeoff: doing portable monitoring only due to macOS max descriptor manual ulimit setting requirement
-	// https://github.com/gorakhargosh/watchdog/blob/master/src/watchdog/observers/kqueue.py#L394-L418
-	all := set.NewSetF(func(f fileset.File) string {
-		return f.Relative
-	})
-	gitFiles, err := s.fileSet.Files()
-	if err != nil {
-		log.Errorf(ctx, "cannot list files: %s", err)
-		return nil, err
-	}
-	all.Add(gitFiles...)
-
-	include, err := s.includeFileSet.Files()
-	if err != nil {
-		log.Errorf(ctx, "cannot list include files: %s", err)
-		return nil, err
-	}
-
-	all.Add(include...)
-
-	exclude, err := s.excludeFileSet.Files()
-	if err != nil {
-		log.Errorf(ctx, "cannot list exclude files: %s", err)
-		return nil, err
-	}
-
-	for _, f := range exclude {
-		all.Remove(f)
-	}
-
-	return all.Iter(), nil
-}
-
-// GetFileList returns the list of files that would be synced given opts,
-// applying the same git-aware include/exclude logic as RunOnce.
-// Unlike New, it does not verify the remote path or load a sync snapshot.
-func GetFileList(ctx context.Context, opts SyncOptions) ([]fileset.File, error) {
-	paths := opts.Paths
-	if len(paths) == 0 {
-		paths = []string{"."}
-	}
-	fileSet, err := git.NewFileSet(ctx, opts.WorktreeRoot, opts.LocalRoot, paths)
-	if err != nil {
-		return nil, fmt.Errorf("build file set: %w", err)
-	}
-	includeFileSet, err := fileset.NewGlobSet(opts.LocalRoot, opts.Include)
-	if err != nil {
-		return nil, err
-	}
-	excludeFileSet, err := fileset.NewGlobSet(opts.LocalRoot, opts.Exclude)
-	if err != nil {
-		return nil, err
-	}
-	s := &Sync{
-		SyncOptions:    &opts,
-		fileSet:        fileSet,
-		includeFileSet: includeFileSet,
-		excludeFileSet: excludeFileSet,
-	}
-	return s.GetFileList(ctx)
+	return s.fileList.Files(ctx)
 }
 
 func (s *Sync) RunContinuous(ctx context.Context) error {