use Join instead of +

Co-authored-by: Pieter Noordhuis <pieter.noordhuis@databricks.com>

bundle/config/mutator/load_git_details.go

@@ -7,6 +7,7 @@ import (
 	"github.com/databricks/cli/bundle"
 	"github.com/databricks/cli/libs/diag"
 	"github.com/databricks/cli/libs/git"
+	"github.com/databricks/cli/libs/vfs"
 )
 
 type loadGitDetails struct{}
@@ -26,7 +27,11 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn
 		diags = append(diags, diag.WarningFromErr(err)...)
 	}
 
-	b.WorktreeRoot = info.GuessedWorktreeRoot
+	if info.WorktreeRoot == "" {
+		b.WorktreeRoot = b.BundleRoot
+	} else {
+		b.WorktreeRoot = vfs.MustNew(info.WorktreeRoot)
+	}
 
 	b.Config.Bundle.Git.ActualBranch = info.CurrentBranch
 	if b.Config.Bundle.Git.Branch == "" {
@@ -48,8 +53,8 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn
 	relBundlePath, err := filepath.Rel(b.WorktreeRoot.Native(), b.BundleRoot.Native())
 	if err != nil {
 		diags = append(diags, diag.FromErr(err)...)
-	}
-
+	} else {
 		b.Config.Bundle.Git.BundleRootPath = filepath.ToSlash(relBundlePath)
+	}
 	return diags
 }

cmd/sync/sync.go

@@ -73,8 +73,16 @@ func (f *syncFlags) syncOptionsFromArgs(cmd *cobra.Command, args []string) (*syn
 		log.Warnf(ctx, "Failed to read git info: %s", err)
 	}
 
+	var WorktreeRoot vfs.Path
+
+	if info.WorktreeRoot == "" {
+		WorktreeRoot = localRoot
+	} else {
+		WorktreeRoot = vfs.MustNew(info.WorktreeRoot)
+	}
+
 	opts := sync.SyncOptions{
-		WorktreeRoot: info.GuessedWorktreeRoot,
+		WorktreeRoot: WorktreeRoot,
 		LocalRoot:    localRoot,
 		Paths:        []string{"."},
 		Include:      nil,

internal/git_fetch_test.go

@@ -3,6 +3,7 @@ package internal
 import (
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"testing"
 
@@ -16,18 +17,18 @@ import (
 const examplesRepoUrl = "https://github.com/databricks/bundle-examples"
 const examplesRepoProvider = "gitHub"
 
-func assertFullGitInfo(t *testing.T, expectedRoot string, info git.GitRepositoryInfo) {
+func assertFullGitInfo(t *testing.T, expectedRoot string, info git.RepositoryInfo) {
 	assert.Equal(t, "main", info.CurrentBranch)
 	assert.NotEmpty(t, info.LatestCommit)
 	assert.Equal(t, examplesRepoUrl, info.OriginURL)
 	assert.Equal(t, expectedRoot, info.WorktreeRoot)
 }
 
-func assertEmptyGitInfo(t *testing.T, info git.GitRepositoryInfo) {
+func assertEmptyGitInfo(t *testing.T, info git.RepositoryInfo) {
 	assertSparseGitInfo(t, "", info)
 }
 
-func assertSparseGitInfo(t *testing.T, expectedRoot string, info git.GitRepositoryInfo) {
+func assertSparseGitInfo(t *testing.T, expectedRoot string, info git.RepositoryInfo) {
 	assert.Equal(t, "", info.CurrentBranch)
 	assert.Equal(t, "", info.LatestCommit)
 	assert.Equal(t, "", info.OriginURL)
@@ -39,7 +40,7 @@ func TestAccFetchRepositoryInfoAPI_FromRepo(t *testing.T) {
 	me, err := wt.W.CurrentUser.Me(ctx)
 	require.NoError(t, err)
 
-	targetPath := acc.RandomName("/Workspace/Users/" + me.UserName + "/testing-clone-bundle-examples-")
+	targetPath := acc.RandomName(path.Join("/Workspace/Users", me.UserName, "/testing-clone-bundle-examples-"))
 	stdout, stderr := RequireSuccessfulRun(t, "repos", "create", examplesRepoUrl, examplesRepoProvider, "--path", targetPath)
 	t.Cleanup(func() {
 		RequireSuccessfulRun(t, "repos", "delete", targetPath)
@@ -50,7 +51,7 @@ func TestAccFetchRepositoryInfoAPI_FromRepo(t *testing.T) {
 	ctx = dbr.MockRuntime(ctx, true)
 
 	for _, inputPath := range []string{
-		targetPath + "/knowledge_base/dashboard_nyc_taxi",
+		path.Join(targetPath, "knowledge_base/dashboard_nyc_taxi"),
 		targetPath,
 	} {
 		t.Run(inputPath, func(t *testing.T) {
@@ -66,14 +67,13 @@ func TestAccFetchRepositoryInfoAPI_FromNonRepo(t *testing.T) {
 	me, err := wt.W.CurrentUser.Me(ctx)
 	require.NoError(t, err)
 
-	rootPath := acc.RandomName("/Workspace/Users/" + me.UserName + "/testing-nonrepo-")
-	_, stderr := RequireSuccessfulRun(t, "workspace", "mkdirs", rootPath+"/a/b/c")
+	rootPath := acc.RandomName(path.Join("/Workspace/Users", me.UserName, "testing-nonrepo-"))
+	_, stderr := RequireSuccessfulRun(t, "workspace", "mkdirs", path.Join(rootPath, "a/b/c"))
 	t.Cleanup(func() {
 		RequireSuccessfulRun(t, "workspace", "delete", "--recursive", rootPath)
 	})
 
 	assert.Empty(t, stderr.String())
-	//assert.NotEmpty(t, stdout.String())
 	ctx = dbr.MockRuntime(ctx, true)
 
 	tests := []struct {
@@ -81,7 +81,7 @@ func TestAccFetchRepositoryInfoAPI_FromNonRepo(t *testing.T) {
 		msg   string
 	}{
 		{
-			input: rootPath + "/a/b/c",
+			input: path.Join(rootPath, "a/b/c"),
 			msg:   "",
 		},
 		{
@@ -89,7 +89,7 @@ func TestAccFetchRepositoryInfoAPI_FromNonRepo(t *testing.T) {
 			msg:   "",
 		},
 		{
-			input: rootPath + "/non-existent",
+			input: path.Join(rootPath, "/non-existent"),
 			msg:   "doesn't exist",
 		},
 	}
@@ -114,7 +114,7 @@ func TestAccFetchRepositoryInfoDotGit_FromGitRepo(t *testing.T) {
 	repo := cloneRepoLocally(t, examplesRepoUrl)
 
 	for _, inputPath := range []string{
-		repo + "/knowledge_base/dashboard_nyc_taxi",
+		filepath.Join(repo, "knowledge_base/dashboard_nyc_taxi"),
 		repo,
 	} {
 		t.Run(inputPath, func(t *testing.T) {
@@ -140,12 +140,12 @@ func TestAccFetchRepositoryInfoDotGit_FromNonGitRepo(t *testing.T) {
 
 	tempDir := t.TempDir()
 	root := filepath.Join(tempDir, "repo")
-	require.NoError(t, os.MkdirAll(root+"/a/b/c", 0700))
+	require.NoError(t, os.MkdirAll(filepath.Join(root, "a/b/c"), 0700))
 
 	tests := []string{
-		root + "/a/b/c",
+		filepath.Join(root, "a/b/c"),
 		root,
-		root + "/non-existent",
+		filepath.Join(root, "/non-existent"),
 	}
 
 	for _, input := range tests {
@@ -162,9 +162,9 @@ func TestAccFetchRepositoryInfoDotGit_FromBrokenGitRepo(t *testing.T) {
 
 	tempDir := t.TempDir()
 	root := filepath.Join(tempDir, "repo")
-	path := root + "/a/b/c"
+	path := filepath.Join(root, "a/b/c")
 	require.NoError(t, os.MkdirAll(path, 0700))
-	require.NoError(t, os.WriteFile(root+"/.git", []byte(""), 0000))
+	require.NoError(t, os.WriteFile(filepath.Join(root, ".git"), []byte(""), 0000))
 
 	info, err := git.FetchRepositoryInfo(ctx, path, wt.W)
 	assert.NoError(t, err)

libs/git/info.go

@@ -17,7 +17,7 @@ import (
 	"github.com/databricks/databricks-sdk-go/client"
 )
 
-type GitRepositoryInfo struct {
+type RepositoryInfo struct {
 	// Various metadata about the repo. Each could be "" if it could not be read. No error is returned for such case.
 	OriginURL     string
 	LatestCommit  string
@@ -25,9 +25,6 @@ type GitRepositoryInfo struct {
 
 	// Absolute path to determined worktree root or "" if worktree root could not be determined.
 	WorktreeRoot string
-
-	// vfs.Path variant of WorktreeRoot if WorktreeRoot is set; otherwise defaults to input path.
-	GuessedWorktreeRoot vfs.Path
 }
 
 type gitInfo struct {
@@ -42,15 +39,13 @@ type response struct {
 }
 
 // Fetch repository information either by quering .git or by fetching it from API (for dabs-in-workspace case).
-//   - In case we could not find git repository, all string fields of GitRepositoryInfo will be "" and err will be nil.
+//   - In case we could not find git repository, all string fields of RepositoryInfo will be "" and err will be nil.
 //   - If there were any errors when trying to determine git root (e.g. API call returned an error or there were permission issues
-//     reading the file system), all strings fields of GitRepositoryInfo will be "" and err will be non-nil.
-//   - For convenience, GuessedWorktreeRoot parameter will be set to path in the above two cases.
+//     reading the file system), all strings fields of RepositoryInfo will be "" and err will be non-nil.
 //   - If we could determine git worktree root but there were errors when reading metadata (origin, branch, commit), those errors
-//     will be logged as warnings, GitRepositoryInfo is guaranteed to have non-empty WorktreeRoot and corresponding GuessedWorktreeRoot
-//     and other fields on best effort basis. The err will be nil.
+//     will be logged as warnings, RepositoryInfo is guaranteed to have non-empty WorktreeRoot and other fields on best effort basis.
 //   - In successful case, all fields are set to proper git repository metadata.
-func FetchRepositoryInfo(ctx context.Context, path string, w *databricks.WorkspaceClient) (GitRepositoryInfo, error) {
+func FetchRepositoryInfo(ctx context.Context, path string, w *databricks.WorkspaceClient) (RepositoryInfo, error) {
 	if strings.HasPrefix(path, "/Workspace/") && dbr.RunsOnRuntime(ctx) {
 		return fetchRepositoryInfoAPI(ctx, path, w)
 	} else {
@@ -58,11 +53,8 @@ func FetchRepositoryInfo(ctx context.Context, path string, w *databricks.Workspa
 	}
 }
 
-func fetchRepositoryInfoAPI(ctx context.Context, path string, w *databricks.WorkspaceClient) (GitRepositoryInfo, error) {
-	result := GitRepositoryInfo{
-		// For convenience, this field defaults to input path, even if err is also set.
-		GuessedWorktreeRoot: vfs.MustNew(path),
-	}
+func fetchRepositoryInfoAPI(ctx context.Context, path string, w *databricks.WorkspaceClient) (RepositoryInfo, error) {
+	result := RepositoryInfo{}
 
 	apiClient, err := client.New(w.Config)
 	if err != nil {
@@ -96,8 +88,6 @@ func fetchRepositoryInfoAPI(ctx context.Context, path string, w *databricks.Work
 		result.LatestCommit = gi.HeadCommitID
 		result.CurrentBranch = gi.Branch
 		result.WorktreeRoot = fixedPath
-		// Note, this won't work on Windows since vfs.MustNew will call filepath.Abs
-		result.GuessedWorktreeRoot = vfs.MustNew(fixedPath)
 	} else {
 		log.Warnf(ctx, "Failed to load git info from %s", apiEndpoint)
 	}
@@ -112,10 +102,8 @@ func ensureWorkspacePrefix(p string) string {
 	return p
 }
 
-func fetchRepositoryInfoDotGit(ctx context.Context, path string) (GitRepositoryInfo, error) {
-	result := GitRepositoryInfo{
-		GuessedWorktreeRoot: vfs.MustNew(path),
-	}
+func fetchRepositoryInfoDotGit(ctx context.Context, path string) (RepositoryInfo, error) {
+	result := RepositoryInfo{}
 
 	rootDir, err := findLeafInTree(path, GitDirectoryName)
 	if rootDir == "" {
@@ -123,9 +111,8 @@ func fetchRepositoryInfoDotGit(ctx context.Context, path string) (GitRepositoryI
 	}
 
 	result.WorktreeRoot = rootDir
-	result.GuessedWorktreeRoot = vfs.MustNew(rootDir)
 
-	repo, err := NewRepository(result.GuessedWorktreeRoot)
+	repo, err := NewRepository(vfs.MustNew(rootDir))
 	if err != nil {
 		log.Warnf(ctx, "failed to read .git: %s", err)
 
@@ -154,7 +141,7 @@ func findLeafInTree(p string, leafName string) (string, error) {
 		_, err = os.Stat(filepath.Join(p, leafName))
 
 		if err == nil {
-			// found .git in p
+			// Found [leafName] in p
 			return p, nil
 		}