From d587531cbd2f960be49d1617a7a1fd17e769eacb Mon Sep 17 00:00:00 2001
From: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com>
Date: Tue, 8 Nov 2022 13:51:08 +0100
Subject: [PATCH] Added creation of .gitignore for bricks project with cache
 dir path (#88)

It works, please trust me
---
 git/fileset.go              | 55 ++++++++++++++++++++++++++++---------
 go.mod                      |  9 ++++--
 go.sum                      |  9 ++++--
 internal/sync_test.go       | 55 +++++++++++++++++++------------------
 project/project.go          |  6 +++-
 project/project_test.go     | 44 +++++++++++++++++++++++++----
 project/testdata/.gitignore |  2 ++
 7 files changed, 128 insertions(+), 52 deletions(-)
 create mode 100644 project/testdata/.gitignore

diff --git a/git/fileset.go b/git/fileset.go
index a3734cf7..04d1bea7 100644
--- a/git/fileset.go
+++ b/git/fileset.go
@@ -9,6 +9,7 @@ import (
 	"time"
 
 	ignore "github.com/sabhiram/go-gitignore"
+	"golang.org/x/exp/slices"
 )
 
 type File struct {
@@ -36,29 +37,57 @@ type FileSet struct {
 	ignore *ignore.GitIgnore
 }
 
-// GetFileSet retrieves FileSet from Git repository checkout root
-// or panics if no root is detected.
-func GetFileSet() (FileSet, error) {
-	root, err := Root()
-	return NewFileSet(root), err
+// Retuns FileSet for the git repo located at `root`
+func NewFileSet(root string) *FileSet {
+	syncIgnoreLines := append(getGitIgnoreLines(root), getSyncIgnoreLines()...)
+	return &FileSet{
+		root:   root,
+		ignore: ignore.CompileIgnoreLines(syncIgnoreLines...),
+	}
 }
 
-// Retuns FileSet for the repository located at `root`
-func NewFileSet(root string) FileSet {
-	lines := []string{".git", ".bricks"}
-	rawIgnore, err := os.ReadFile(fmt.Sprintf("%s/.gitignore", root))
+func getGitIgnoreLines(root string) []string {
+	gitIgnoreLines := []string{}
+	gitIgnorePath := fmt.Sprintf("%s/.gitignore", root)
+	rawIgnore, err := os.ReadFile(gitIgnorePath)
+
 	if err == nil {
 		// add entries from .gitignore if the file exists (did read correctly)
 		for _, line := range strings.Split(string(rawIgnore), "\n") {
 			// underlying library doesn't behave well with Rule 5 of .gitignore,
 			// hence this workaround
-			lines = append(lines, strings.Trim(line, "/"))
+			gitIgnoreLines = append(gitIgnoreLines, strings.Trim(line, "/"))
 		}
 	}
-	return FileSet{
-		root:   root,
-		ignore: ignore.CompileIgnoreLines(lines...),
+	return gitIgnoreLines
+}
+
+// This contains additional files/dirs to be ignored while syncing that are
+// not mentioned in .gitignore
+func getSyncIgnoreLines() []string {
+	return []string{".git"}
+}
+
+// Only call this function for a bricks project root
+// since it will create a .gitignore file if missing
+func (w *FileSet) EnsureValidGitIgnoreExists() error {
+	gitIgnoreLines := getGitIgnoreLines(w.root)
+	gitIgnorePath := fmt.Sprintf("%s/.gitignore", w.root)
+	if slices.Contains(gitIgnoreLines, ".databricks") {
+		return nil
 	}
+	f, err := os.OpenFile(gitIgnorePath, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0644)
+	if err != nil {
+		return err
+	}
+	defer f.Close()
+	_, err = f.WriteString("\n.databricks\n")
+	if err != nil {
+		return err
+	}
+	gitIgnoreLines = append(gitIgnoreLines, ".databricks")
+	w.ignore = ignore.CompileIgnoreLines(append(gitIgnoreLines, getSyncIgnoreLines()...)...)
+	return nil
 }
 
 // Return root for fileset.
diff --git a/go.mod b/go.mod
index 7dea5c30..2914c540 100644
--- a/go.mod
+++ b/go.mod
@@ -13,11 +13,14 @@ require (
 	github.com/spf13/cobra v1.5.0 // Apache 2.0
 	github.com/stretchr/testify v1.8.0 // MIT
 	github.com/whilp/git-urls v1.0.0 // MIT
-	golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 // BSD-3-Clause
+	golang.org/x/mod v0.6.0 // BSD-3-Clause
 	gopkg.in/ini.v1 v1.67.0 // Apache 2.0
 )
 
-require golang.org/x/sync v0.0.0-20220513210516-0976fa681c29
+require (
+	golang.org/x/exp v0.0.0-20221031165847-c99f073a8326
+	golang.org/x/sync v0.0.0-20220513210516-0976fa681c29
+)
 
 require (
 	cloud.google.com/go/compute v1.6.1 // indirect
@@ -33,7 +36,7 @@ require (
 	go.opencensus.io v0.23.0 // indirect
 	golang.org/x/net v0.0.0-20220526153639-5463443f8c37 // indirect
 	golang.org/x/oauth2 v0.0.0-20220628200809-02e64fa58f26 // indirect
-	golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a // indirect
+	golang.org/x/sys v0.1.0 // indirect
 	golang.org/x/text v0.3.7 // indirect
 	golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac // indirect
 	google.golang.org/api v0.82.0 // indirect
diff --git a/go.sum b/go.sum
index 31e98125..5bebab36 100644
--- a/go.sum
+++ b/go.sum
@@ -252,6 +252,8 @@ golang.org/x/exp v0.0.0-20191227195350-da58074b4299/go.mod h1:2RIsYlXP63K8oxa1u0
 golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u096TMicItID8zy7Y6sNkU49FU4=
 golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM=
 golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU=
+golang.org/x/exp v0.0.0-20221031165847-c99f073a8326 h1:QfTh0HpN6hlw6D3vu8DAwC8pBIwikq0AI1evdm+FksE=
+golang.org/x/exp v0.0.0-20221031165847-c99f073a8326/go.mod h1:CxIveKay+FTh1D0yPZemJVgC/95VzuuOLq5Qi4xnoYc=
 golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
 golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=
 golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE=
@@ -277,8 +279,8 @@ golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.2/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
-golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3 h1:kQgndtyPBW/JIYERgdxfwMYh3AVStj88WQTlNDi2a+o=
-golang.org/x/mod v0.6.0-dev.0.20220106191415-9b9b3d81d5e3/go.mod h1:3p9vT2HGsQu2K1YbXdKPJLVgG5VJdoTa1poYQBtP1AY=
+golang.org/x/mod v0.6.0 h1:b9gGHsz9/HhJ3HF5DHQytPpuwocVTChQJK3AvoLRD5I=
+golang.org/x/mod v0.6.0/go.mod h1:4mET923SAdbXp2ki8ey+zGs1SLqsuM2Y0uvdZR/fUNI=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -413,8 +415,9 @@ golang.org/x/sys v0.0.0-20220227234510-4e6760a101f9/go.mod h1:oPkhp1MJrh7nUepCBc
 golang.org/x/sys v0.0.0-20220328115105-d36c6a25d886/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220412211240-33da011f77ad/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220502124256-b6088ccd6cba/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a h1:dGzPydgVsqGcTRVwiLJ1jVbufYwmzD3LfVPLKsKg+0k=
 golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.1.0 h1:kunALQeHf1/185U1i0GOB/fy1IPRDDpuoOOqRReG57U=
+golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
 golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
diff --git a/internal/sync_test.go b/internal/sync_test.go
index 3949e579..accf487e 100644
--- a/internal/sync_test.go
+++ b/internal/sync_test.go
@@ -108,7 +108,7 @@ func TestAccFullSync(t *testing.T) {
 			Path: repoPath,
 		})
 		assert.NoError(t, err)
-		return len(repoContent.Objects) == 2
+		return len(repoContent.Objects) == 3
 	}, 30*time.Second, 5*time.Second)
 	repoContent, err := wsc.Workspace.List(ctx, workspace.ListRequest{
 		Path: repoPath,
@@ -118,13 +118,38 @@ func TestAccFullSync(t *testing.T) {
 	for _, v := range repoContent.Objects {
 		files1 = append(files1, filepath.Base(v.Path))
 	}
-	assert.Len(t, files1, 2)
+	assert.Len(t, files1, 3)
 	assert.Contains(t, files1, "amsterdam.txt")
 	assert.Contains(t, files1, ".gitkeep")
+	assert.Contains(t, files1, ".gitignore")
 
 	// Create new files and assert
 	os.Create(filepath.Join(projectDir, "hello.txt"))
 	os.Create(filepath.Join(projectDir, "world.txt"))
+	assert.Eventually(t, func() bool {
+		repoContent, err := wsc.Workspace.List(ctx, workspace.ListRequest{
+			Path: repoPath,
+		})
+		assert.NoError(t, err)
+		return len(repoContent.Objects) == 5
+	}, 30*time.Second, 5*time.Second)
+	repoContent, err = wsc.Workspace.List(ctx, workspace.ListRequest{
+		Path: repoPath,
+	})
+	assert.NoError(t, err)
+	var files2 []string
+	for _, v := range repoContent.Objects {
+		files2 = append(files2, filepath.Base(v.Path))
+	}
+	assert.Len(t, files2, 5)
+	assert.Contains(t, files2, "amsterdam.txt")
+	assert.Contains(t, files2, ".gitkeep")
+	assert.Contains(t, files2, "hello.txt")
+	assert.Contains(t, files2, "world.txt")
+	assert.Contains(t, files2, ".gitignore")
+
+	// delete a file and assert
+	os.Remove(filepath.Join(projectDir, "hello.txt"))
 	assert.Eventually(t, func() bool {
 		repoContent, err := wsc.Workspace.List(ctx, workspace.ListRequest{
 			Path: repoPath,
@@ -136,37 +161,15 @@ func TestAccFullSync(t *testing.T) {
 		Path: repoPath,
 	})
 	assert.NoError(t, err)
-	var files2 []string
-	for _, v := range repoContent.Objects {
-		files2 = append(files2, filepath.Base(v.Path))
-	}
-	assert.Len(t, files2, 4)
-	assert.Contains(t, files2, "amsterdam.txt")
-	assert.Contains(t, files2, ".gitkeep")
-	assert.Contains(t, files2, "hello.txt")
-	assert.Contains(t, files2, "world.txt")
-
-	// delete a file and assert
-	os.Remove(filepath.Join(projectDir, "hello.txt"))
-	assert.Eventually(t, func() bool {
-		repoContent, err := wsc.Workspace.List(ctx, workspace.ListRequest{
-			Path: repoPath,
-		})
-		assert.NoError(t, err)
-		return len(repoContent.Objects) == 3
-	}, 30*time.Second, 5*time.Second)
-	repoContent, err = wsc.Workspace.List(ctx, workspace.ListRequest{
-		Path: repoPath,
-	})
-	assert.NoError(t, err)
 	var files3 []string
 	for _, v := range repoContent.Objects {
 		files3 = append(files3, filepath.Base(v.Path))
 	}
-	assert.Len(t, files3, 3)
+	assert.Len(t, files3, 4)
 	assert.Contains(t, files3, "amsterdam.txt")
 	assert.Contains(t, files3, ".gitkeep")
 	assert.Contains(t, files3, "world.txt")
+	assert.Contains(t, files3, ".gitignore")
 }
 
 func assertSnapshotContents(t *testing.T, host, repoPath, projectDir string, listOfSyncedFiles []string) {
diff --git a/project/project.go b/project/project.go
index 9b7747af..f6103ce9 100644
--- a/project/project.go
+++ b/project/project.go
@@ -68,6 +68,10 @@ func Initialize(ctx context.Context, root, env string) (context.Context, error)
 	}
 
 	fileSet := git.NewFileSet(root)
+	err = fileSet.EnsureValidGitIgnoreExists()
+	if err != nil {
+		return ctx, nil
+	}
 
 	p := project{
 		root: root,
@@ -75,7 +79,7 @@ func Initialize(ctx context.Context, root, env string) (context.Context, error)
 
 		config:      &config,
 		environment: &environment,
-		fileSet:     &fileSet,
+		fileSet:     fileSet,
 	}
 
 	p.initializeWorkspacesClient(ctx)
diff --git a/project/project_test.go b/project/project_test.go
index 3bee240b..2ecff5d8 100644
--- a/project/project_test.go
+++ b/project/project_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -16,7 +17,7 @@ func TestProjectInitialize(t *testing.T) {
 	assert.Equal(t, Get(ctx).config.Name, "dev")
 }
 
-func TestProjectCacheDirErrorsIfNoGitIgnoreFile(t *testing.T) {
+func TestProjectInitializationCreatesGitIgnoreIfAbsent(t *testing.T) {
 	// create project root with databricks.yml
 	projectDir := t.TempDir()
 	f1, err := os.Create(filepath.Join(projectDir, "databricks.yml"))
@@ -26,29 +27,60 @@ func TestProjectCacheDirErrorsIfNoGitIgnoreFile(t *testing.T) {
 	ctx, err := Initialize(context.Background(), projectDir, DefaultEnvironment)
 	assert.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")
+
 	prj := Get(ctx)
 	_, err = prj.CacheDir()
-	assert.Error(t, err, "please add /.databricks/ to .gitignore")
+	assert.NoError(t, err)
 }
 
-func TestProjectCacheDirErrorsIfGitIgnoreEntryAbsent(t *testing.T) {
+func TestProjectInitializationAddsCacheDirToGitIgnore(t *testing.T) {
 	// create project root with databricks.yml
 	projectDir := t.TempDir()
 	f1, err := os.Create(filepath.Join(projectDir, "databricks.yml"))
 	assert.NoError(t, err)
 	defer f1.Close()
 
-	// create empty .gitignore
-	f2, err := os.Create(filepath.Join(projectDir, ".gitignore"))
+	gitIgnorePath := filepath.Join(projectDir, ".gitignore")
+	f2, err := os.Create(gitIgnorePath)
 	assert.NoError(t, err)
 	defer f2.Close()
 
 	ctx, err := Initialize(context.Background(), projectDir, DefaultEnvironment)
 	assert.NoError(t, err)
 
+	fileBytes, err := os.ReadFile(gitIgnorePath)
+	assert.NoError(t, err)
+	assert.Contains(t, string(fileBytes), ".databricks")
+
 	prj := Get(ctx)
 	_, err = prj.CacheDir()
-	assert.Error(t, err, "please add /.databricks/ to .gitignore")
+	assert.NoError(t, err)
+}
+
+func TestProjectInitializationDoesNotAddCacheDirToGitIgnoreIfAlreadyPresent(t *testing.T) {
+	// create project root with databricks.yml
+	projectDir := t.TempDir()
+	f1, err := os.Create(filepath.Join(projectDir, "databricks.yml"))
+	assert.NoError(t, err)
+	defer f1.Close()
+
+	gitIgnorePath := filepath.Join(projectDir, ".gitignore")
+
+	err = os.WriteFile(gitIgnorePath, []byte("/.databricks/"), 0o644)
+	assert.NoError(t, err)
+
+	_, err = Initialize(context.Background(), projectDir, DefaultEnvironment)
+	assert.NoError(t, err)
+
+	fileBytes, err := os.ReadFile(gitIgnorePath)
+	assert.NoError(t, err)
+
+	assert.Equal(t, 1, strings.Count(string(fileBytes), ".databricks"))
 }
 
 func TestProjectCacheDir(t *testing.T) {
diff --git a/project/testdata/.gitignore b/project/testdata/.gitignore
new file mode 100644
index 00000000..ddb36430
--- /dev/null
+++ b/project/testdata/.gitignore
@@ -0,0 +1,2 @@
+
+/.databricks/
\ No newline at end of file