Do not modify/create .gitignore in bundle root (#2429)

denik · web-flow · commit a26461c5a396 · 2025-03-05T10:57:05.000Z
## Changes
- Do not modify or edit .gitignore in bundle root.
- Instead create .databricks/.gitignore with content set to "*"

## Why
Merging our changes into existing .gitignore is complicated and adding
.gitignore where it's not expected adds to the noise. Other tools also
use the approach in this PR (e.g. ruff creates .ruff_cache/.gitignore).

## Tests
- Modified templates/default-sql to capture this new file.
diff --git a/acceptance/bundle/templates/dbt-sql/script b/acceptance/bundle/templates/dbt-sql/script
@@ -6,3 +6,4 @@ trace $CLI bundle validate -t prod
 
 # Do not affect this repository's git behaviour #2318
 mv .gitignore out.gitignore
+rm .databricks/.gitignore
diff --git a/acceptance/bundle/templates/default-python/classic/script b/acceptance/bundle/templates/default-python/classic/script
@@ -6,6 +6,7 @@ trace $CLI bundle validate -t prod
 
 # Do not affect this repository's git behaviour #2318
 mv .gitignore out.gitignore
+rm .databricks/.gitignore
 
 cd ../../
 
diff --git a/acceptance/bundle/templates/default-python/serverless/script b/acceptance/bundle/templates/default-python/serverless/script
@@ -6,3 +6,4 @@ trace $CLI bundle validate -t prod
 
 # Do not affect this repository's git behaviour #2318
 mv .gitignore out.gitignore
+rm .databricks/.gitignore
diff --git a/acceptance/bundle/templates/default-sql/output/my_default_sql/.databricks/out.gitignore b/acceptance/bundle/templates/default-sql/output/my_default_sql/.databricks/out.gitignore
@@ -0,0 +1 @@
+*
diff --git a/acceptance/bundle/templates/default-sql/script b/acceptance/bundle/templates/default-sql/script
@@ -6,3 +6,6 @@ trace $CLI bundle validate -t prod
 
 # Do not affect this repository's git behaviour #2318
 mv .gitignore out.gitignore
+
+# Only for this test (default-sql), record .databricks/.gitignore in the output
+mv .databricks/.gitignore .databricks/out.gitignore
diff --git a/acceptance/bundle/templates/experimental-jobs-as-code/script b/acceptance/bundle/templates/experimental-jobs-as-code/script
@@ -11,3 +11,4 @@ rm -fr .venv resources/__pycache__ uv.lock my_jobs_as_code.egg-info
 
 # Do not affect this repository's git behaviour #2318
 mv .gitignore out.gitignore
+rm .databricks/.gitignore
diff --git a/bundle/bundle.go b/bundle/bundle.go
@@ -21,6 +21,7 @@ import (
 	"github.com/databricks/cli/libs/fileset"
 	"github.com/databricks/cli/libs/locker"
 	"github.com/databricks/cli/libs/log"
+	libsync "github.com/databricks/cli/libs/sync"
 	"github.com/databricks/cli/libs/tags"
 	"github.com/databricks/cli/libs/terraform"
 	"github.com/databricks/cli/libs/vfs"
@@ -198,6 +199,7 @@ func (b *Bundle) CacheDir(ctx context.Context, paths ...string) (string, error)
 		return "", err
 	}
 
+	libsync.WriteGitIgnore(ctx, b.BundleRootPath)
 	return dir, nil
 }
 
diff --git a/integration/cmd/sync/sync_test.go b/integration/cmd/sync/sync_test.go
@@ -224,22 +224,21 @@ func (a *syncTest) snapshotContains(files []string) {
 		_, ok := s.LastModifiedTimes[filePath]
 		assert.True(a.t, ok, "%s not in snapshot file: %v", filePath, s.LastModifiedTimes)
 	}
-	assert.Equal(a.t, len(files), len(s.LastModifiedTimes))
+	assert.Equal(a.t, len(files), len(s.LastModifiedTimes), "files=%s s.LastModifiedTimes=%s", files, s.LastModifiedTimes)
 }
 
 func TestSyncFullFileSync(t *testing.T) {
 	ctx, assertSync := setupSyncTest(t, "--full", "--watch")
 
-	// .gitignore is created by the sync process to enforce .databricks is not synced
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore"))
+	assertSync.remoteDirContent(ctx, "", repoFiles)
 
 	// New file
 	localFilePath := filepath.Join(assertSync.localRoot, "foo.txt")
 	f := testfile.CreateFile(t, localFilePath)
 	defer f.Close(t)
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo.txt", ".gitignore"))
+	assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo.txt"))
 	assertSync.remoteFileContent(ctx, "foo.txt", "")
 
 	// Write to file
@@ -255,24 +254,23 @@ func TestSyncFullFileSync(t *testing.T) {
 	// delete
 	f.Remove(t)
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore"))
+	assertSync.remoteDirContent(ctx, "", repoFiles)
 }
 
 func TestSyncIncrementalFileSync(t *testing.T) {
 	ctx, assertSync := setupSyncTest(t, "--watch")
 
-	// .gitignore is created by the sync process to enforce .databricks is not synced
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore"))
+	assertSync.remoteDirContent(ctx, "", repoFiles)
 
 	// New file
 	localFilePath := filepath.Join(assertSync.localRoot, "foo.txt")
 	f := testfile.CreateFile(t, localFilePath)
 	defer f.Close(t)
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo.txt", ".gitignore"))
+	assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo.txt"))
 	assertSync.remoteFileContent(ctx, "foo.txt", "")
-	assertSync.snapshotContains(append(repoFiles, "foo.txt", ".gitignore"))
+	assertSync.snapshotContains(append(repoFiles, "foo.txt"))
 
 	// Write to file
 	f.Overwrite(t, `{"statement": "Mi Gente"}`)
@@ -287,16 +285,15 @@ func TestSyncIncrementalFileSync(t *testing.T) {
 	// delete
 	f.Remove(t)
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore"))
-	assertSync.snapshotContains(append(repoFiles, ".gitignore"))
+	assertSync.remoteDirContent(ctx, "", repoFiles)
+	assertSync.snapshotContains(repoFiles)
 }
 
 func TestSyncNestedFolderSync(t *testing.T) {
 	ctx, assertSync := setupSyncTest(t, "--watch")
 
-	// .gitignore is created by the sync process to enforce .databricks is not synced
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore"))
+	assertSync.remoteDirContent(ctx, "", repoFiles)
 
 	// New file
 	localFilePath := filepath.Join(assertSync.localRoot, "dir1/dir2/dir3/foo.txt")
@@ -305,25 +302,24 @@ func TestSyncNestedFolderSync(t *testing.T) {
 	f := testfile.CreateFile(t, localFilePath)
 	defer f.Close(t)
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "dir1"))
+	assertSync.remoteDirContent(ctx, "", append(repoFiles, "dir1"))
 	assertSync.remoteDirContent(ctx, "dir1", []string{"dir2"})
 	assertSync.remoteDirContent(ctx, "dir1/dir2", []string{"dir3"})
 	assertSync.remoteDirContent(ctx, "dir1/dir2/dir3", []string{"foo.txt"})
-	assertSync.snapshotContains(append(repoFiles, ".gitignore", "dir1/dir2/dir3/foo.txt"))
+	assertSync.snapshotContains(append(repoFiles, "dir1/dir2/dir3/foo.txt"))
 
 	// delete
 	f.Remove(t)
 	assertSync.waitForCompletionMarker()
 	assertSync.remoteNotExist(ctx, "dir1")
-	assertSync.snapshotContains(append(repoFiles, ".gitignore"))
+	assertSync.snapshotContains(repoFiles)
 }
 
 func TestSyncNestedFolderDoesntFailOnNonEmptyDirectory(t *testing.T) {
 	ctx, assertSync := setupSyncTest(t, "--watch")
 
-	// .gitignore is created by the sync process to enforce .databricks is not synced
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore"))
+	assertSync.remoteDirContent(ctx, "", repoFiles)
 
 	// New file
 	localFilePath := filepath.Join(assertSync.localRoot, "dir1/dir2/dir3/foo.txt")
@@ -353,9 +349,8 @@ func TestSyncNestedFolderDoesntFailOnNonEmptyDirectory(t *testing.T) {
 func TestSyncNestedSpacePlusAndHashAreEscapedSync(t *testing.T) {
 	ctx, assertSync := setupSyncTest(t, "--watch")
 
-	// .gitignore is created by the sync process to enforce .databricks is not synced
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore"))
+	assertSync.remoteDirContent(ctx, "", repoFiles)
 
 	// New file
 	localFilePath := filepath.Join(assertSync.localRoot, "dir1/a b+c/c+d e/e+f g#i.txt")
@@ -364,17 +359,17 @@ func TestSyncNestedSpacePlusAndHashAreEscapedSync(t *testing.T) {
 	f := testfile.CreateFile(t, localFilePath)
 	defer f.Close(t)
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "dir1"))
+	assertSync.remoteDirContent(ctx, "", append(repoFiles, "dir1"))
 	assertSync.remoteDirContent(ctx, "dir1", []string{"a b+c"})
 	assertSync.remoteDirContent(ctx, "dir1/a b+c", []string{"c+d e"})
 	assertSync.remoteDirContent(ctx, "dir1/a b+c/c+d e", []string{"e+f g#i.txt"})
-	assertSync.snapshotContains(append(repoFiles, ".gitignore", "dir1/a b+c/c+d e/e+f g#i.txt"))
+	assertSync.snapshotContains(append(repoFiles, "dir1/a b+c/c+d e/e+f g#i.txt"))
 
 	// delete
 	f.Remove(t)
 	assertSync.waitForCompletionMarker()
 	assertSync.remoteNotExist(ctx, "dir1/a b+c/c+d e")
-	assertSync.snapshotContains(append(repoFiles, ".gitignore"))
+	assertSync.snapshotContains(repoFiles)
 }
 
 // This is a check for the edge case when a user does the following:
@@ -395,23 +390,23 @@ func TestSyncIncrementalFileOverwritesFolder(t *testing.T) {
 	f := testfile.CreateFile(t, localFilePath)
 	defer f.Close(t)
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo"))
+	assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo"))
 	assertSync.remoteDirContent(ctx, "foo", []string{"bar.txt"})
-	assertSync.snapshotContains(append(repoFiles, ".gitignore", "foo/bar.txt"))
+	assertSync.snapshotContains(append(repoFiles, "foo/bar.txt"))
 
 	// delete foo/bar.txt
 	f.Remove(t)
 	os.Remove(filepath.Join(assertSync.localRoot, "foo"))
 	assertSync.waitForCompletionMarker()
 	assertSync.remoteNotExist(ctx, "foo")
-	assertSync.snapshotContains(append(repoFiles, ".gitignore"))
+	assertSync.snapshotContains(repoFiles)
 
 	f2 := testfile.CreateFile(t, filepath.Join(assertSync.localRoot, "foo"))
 	defer f2.Close(t)
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo"))
+	assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo"))
 	assertSync.objectType(ctx, "foo", "FILE")
-	assertSync.snapshotContains(append(repoFiles, ".gitignore", "foo"))
+	assertSync.snapshotContains(append(repoFiles, "foo"))
 }
 
 func TestSyncIncrementalSyncPythonNotebookToFile(t *testing.T) {
@@ -425,23 +420,23 @@ func TestSyncIncrementalSyncPythonNotebookToFile(t *testing.T) {
 
 	// notebook was uploaded properly
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo"))
+	assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo"))
 	assertSync.objectType(ctx, "foo", "NOTEBOOK")
 	assertSync.language(ctx, "foo", "PYTHON")
-	assertSync.snapshotContains(append(repoFiles, ".gitignore", "foo.py"))
+	assertSync.snapshotContains(append(repoFiles, "foo.py"))
 
 	// convert to vanilla python file
 	f.Overwrite(t, "# No longer a python notebook")
 	assertSync.waitForCompletionMarker()
 	assertSync.objectType(ctx, "foo.py", "FILE")
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo.py"))
-	assertSync.snapshotContains(append(repoFiles, ".gitignore", "foo.py"))
+	assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo.py"))
+	assertSync.snapshotContains(append(repoFiles, "foo.py"))
 
 	// delete the vanilla python file
 	f.Remove(t)
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore"))
-	assertSync.snapshotContains(append(repoFiles, ".gitignore"))
+	assertSync.remoteDirContent(ctx, "", repoFiles)
+	assertSync.snapshotContains(repoFiles)
 }
 
 func TestSyncIncrementalSyncFileToPythonNotebook(t *testing.T) {
@@ -454,17 +449,17 @@ func TestSyncIncrementalSyncFileToPythonNotebook(t *testing.T) {
 	assertSync.waitForCompletionMarker()
 
 	// assert file upload
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo.py"))
+	assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo.py"))
 	assertSync.objectType(ctx, "foo.py", "FILE")
-	assertSync.snapshotContains(append(repoFiles, ".gitignore", "foo.py"))
+	assertSync.snapshotContains(append(repoFiles, "foo.py"))
 
 	// convert to notebook
 	f.Overwrite(t, "# Databricks notebook source")
 	assertSync.waitForCompletionMarker()
 	assertSync.objectType(ctx, "foo", "NOTEBOOK")
 	assertSync.language(ctx, "foo", "PYTHON")
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo"))
-	assertSync.snapshotContains(append(repoFiles, ".gitignore", "foo.py"))
+	assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo"))
+	assertSync.snapshotContains(append(repoFiles, "foo.py"))
 }
 
 func TestSyncIncrementalSyncPythonNotebookDelete(t *testing.T) {
@@ -478,14 +473,14 @@ func TestSyncIncrementalSyncPythonNotebookDelete(t *testing.T) {
 	assertSync.waitForCompletionMarker()
 
 	// notebook was uploaded properly
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore", "foo"))
+	assertSync.remoteDirContent(ctx, "", append(repoFiles, "foo"))
 	assertSync.objectType(ctx, "foo", "NOTEBOOK")
 	assertSync.language(ctx, "foo", "PYTHON")
 
 	// Delete notebook
 	f.Remove(t)
 	assertSync.waitForCompletionMarker()
-	assertSync.remoteDirContent(ctx, "", append(repoFiles, ".gitignore"))
+	assertSync.remoteDirContent(ctx, "", repoFiles)
 }
 
 func TestSyncEnsureRemotePathIsUsableIfRepoDoesntExist(t *testing.T) {
diff --git a/libs/git/fileset.go b/libs/git/fileset.go
@@ -43,7 +43,3 @@ func (f *FileSet) Files() ([]fileset.File, error) {
 	f.view.repo.taintIgnoreRules()
 	return f.fileset.Files()
 }
-
-func (f *FileSet) EnsureValidGitIgnoreExists() error {
-	return f.view.EnsureValidGitIgnoreExists()
-}
diff --git a/libs/git/fileset_test.go b/libs/git/fileset_test.go
@@ -1,10 +1,8 @@
 package git
 
 import (
-	"os"
 	"path"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/databricks/cli/libs/vfs"
@@ -51,34 +49,3 @@ func TestFileSetNonCleanRoot(t *testing.T) {
 	require.NoError(t, err)
 	assert.Len(t, files, 3)
 }
-
-func TestFileSetAddsCacheDirToGitIgnore(t *testing.T) {
-	projectDir := t.TempDir()
-	fileSet, err := NewFileSetAtRoot(vfs.MustNew(projectDir))
-	require.NoError(t, err)
-	err = fileSet.EnsureValidGitIgnoreExists()
-	require.NoError(t, err)
-
-	gitIgnorePath := filepath.Join(projectDir, ".gitignore")
-	assert.FileExists(t, gitIgnorePath)
-	fileBytes, err := os.ReadFile(gitIgnorePath)
-	assert.NoError(t, err)
-	assert.Contains(t, string(fileBytes), ".databricks")
-}
-
-func TestFileSetDoesNotCacheDirToGitIgnoreIfAlreadyPresent(t *testing.T) {
-	projectDir := t.TempDir()
-	gitIgnorePath := filepath.Join(projectDir, ".gitignore")
-
-	fileSet, err := NewFileSetAtRoot(vfs.MustNew(projectDir))
-	require.NoError(t, err)
-	err = os.WriteFile(gitIgnorePath, []byte(".databricks"), 0o644)
-	require.NoError(t, err)
-
-	err = fileSet.EnsureValidGitIgnoreExists()
-	require.NoError(t, err)
-
-	b, err := os.ReadFile(gitIgnorePath)
-	require.NoError(t, err)
-	assert.Equal(t, 1, strings.Count(string(b), ".databricks"))
-}
diff --git a/libs/git/view.go b/libs/git/view.go
@@ -90,46 +90,25 @@ func NewView(worktreeRoot, root vfs.Path) (*View, error) {
 	target = strings.TrimPrefix(target, string(os.PathSeparator))
 	target = path.Clean(filepath.ToSlash(target))
 
-	return &View{
+	result := &View{
 		repo:       repo,
 		targetPath: target,
-	}, nil
+	}
+
+	result.SetupDefaults()
+	return result, nil
 }
 
 func NewViewAtRoot(root vfs.Path) (*View, error) {
 	return NewView(root, root)
 }
 
-func (v *View) EnsureValidGitIgnoreExists() error {
-	ign, err := v.IgnoreDirectory(".databricks")
-	if err != nil {
-		return err
-	}
-
-	// return early if .databricks is already being ignored
-	if ign {
-		return nil
-	}
-
-	// Create .gitignore with .databricks entry
-	gitIgnorePath := filepath.Join(v.repo.Root(), v.targetPath, ".gitignore")
-	file, err := os.OpenFile(gitIgnorePath, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0o644)
-	if err != nil {
-		return err
-	}
-	defer file.Close()
-
+func (v *View) SetupDefaults() {
 	// Hard code .databricks ignore pattern so that we never sync it (irrespective)
 	// of .gitignore patterns
 	v.repo.addIgnoreRule(newStringIgnoreRules([]string{
 		".databricks",
 	}))
 
-	_, err = file.WriteString("\n.databricks\n")
-	if err != nil {
-		return err
-	}
-
 	v.repo.taintIgnoreRules()
-	return nil
 }
diff --git a/libs/git/view_test.go b/libs/git/view_test.go
diff --git a/libs/sync/gitignore.go b/libs/sync/gitignore.go
diff --git a/libs/sync/sync.go b/libs/sync/sync.go
diff --git a/libs/sync/sync_test.go b/libs/sync/sync_test.go

Original file line number	Diff line number	Diff line change
`@@ -6,3 +6,4 @@ trace $CLI bundle validate -t prod`
`6`	`6`
`7`	`7`	`# Do not affect this repository's git behaviour #2318`
`8`	`8`	`mv .gitignore out.gitignore`
	`9`	`+rm .databricks/.gitignore`
Original file line number	Diff line number	Diff line change
`@@ -11,3 +11,4 @@ rm -fr .venv resources/__pycache__ uv.lock my_jobs_as_code.egg-info`
`11`	`11`
`12`	`12`	`# Do not affect this repository's git behaviour #2318`
`13`	`13`	`mv .gitignore out.gitignore`
	`14`	`+rm .databricks/.gitignore`
Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,7 @@ import (`
`21`	`21`	`"github.com/databricks/cli/libs/fileset"`
`22`	`22`	`"github.com/databricks/cli/libs/locker"`
`23`	`23`	`"github.com/databricks/cli/libs/log"`
	`24`	`+ libsync "github.com/databricks/cli/libs/sync"`
`24`	`25`	`"github.com/databricks/cli/libs/tags"`
`25`	`26`	`"github.com/databricks/cli/libs/terraform"`
`26`	`27`	`"github.com/databricks/cli/libs/vfs"`
`@@ -198,6 +199,7 @@ func (b *Bundle) CacheDir(ctx context.Context, paths ...string) (string, error)`
`198`	`199`	`return "", err`
`199`	`200`	`}`
`200`	`201`
	`202`	`+ libsync.WriteGitIgnore(ctx, b.BundleRootPath)`
`201`	`203`	`return dir, nil`
`202`	`204`	`}`
`203`	`205`
Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,3 @@ func (f *FileSet) Files() ([]fileset.File, error) {`
`43`	`43`	`f.view.repo.taintIgnoreRules()`
`44`	`44`	`return f.fileset.Files()`
`45`	`45`	`}`
`46`		`-`
`47`		`-func (f *FileSet) EnsureValidGitIgnoreExists() error {`
`48`		`- return f.view.EnsureValidGitIgnoreExists()`
`49`		`-}`