diff --git a/.nextchanges/cli/air-snapshot-profile-parity.md b/.nextchanges/cli/air-snapshot-profile-parity.md new file mode 100644 index 00000000000..48075356125 --- /dev/null +++ b/.nextchanges/cli/air-snapshot-profile-parity.md @@ -0,0 +1 @@ +Improved AIR snapshots to honor Git ignore rules and added selected-profile workspace directory overrides. diff --git a/experimental/air/cmd/runlaunch.go b/experimental/air/cmd/runlaunch.go index b2a7215e66a..ee667e4d8a6 100644 --- a/experimental/air/cmd/runlaunch.go +++ b/experimental/air/cmd/runlaunch.go @@ -7,6 +7,7 @@ import ( "path" "strings" + "github.com/databricks/cli/libs/databrickscfg" "github.com/databricks/cli/libs/env" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/apierr" @@ -33,6 +34,13 @@ func userWorkspaceDir(ctx context.Context, w *databricks.WorkspaceClient) (strin if override := env.Get(ctx, userWorkspaceDirEnv); override != "" { return override, nil } + override, err := databrickscfg.ProfileValue(w.Config, "databricks_internal_user_workspace_dir") + if err != nil { + return "", fmt.Errorf("failed to read selected profile: %w", err) + } + if override != "" { + return override, nil + } email, err := currentUserEmail(ctx, w) if err != nil { return "", err diff --git a/experimental/air/cmd/runlaunch_test.go b/experimental/air/cmd/runlaunch_test.go index af6f0f70d31..7654e544964 100644 --- a/experimental/air/cmd/runlaunch_test.go +++ b/experimental/air/cmd/runlaunch_test.go @@ -1,6 +1,8 @@ package aircmd import ( + "os" + "path/filepath" "strings" "testing" @@ -31,6 +33,7 @@ func newFakeWorkspaceClient(t *testing.T) *databricks.WorkspaceClient { func TestUserWorkspaceDir(t *testing.T) { w := newFakeWorkspaceClient(t) + t.Setenv(userWorkspaceDirEnv, "") dir, err := userWorkspaceDir(t.Context(), w) require.NoError(t, err) assert.True(t, strings.HasPrefix(dir, "/Workspace/Users/"), dir) @@ -42,6 +45,52 @@ func TestUserWorkspaceDir(t *testing.T) { assert.Equal(t, "/Workspace/custom", dir) } +func TestUserWorkspaceDirProfilePrecedence(t *testing.T) { + configPath := filepath.Join(t.TempDir(), ".databrickscfg") + require.NoError(t, os.WriteFile(configPath, []byte(` +[__settings__] +default_profile = second + +[first] +host = https://first.test +databricks_internal_user_workspace_dir = /Workspace/first + +[second] +host = https://second.test +databricks_internal_user_workspace_dir = /Workspace/second +`), 0o600)) + + t.Run("explicit profile", func(t *testing.T) { + w := newFakeWorkspaceClient(t) + w.Config.ConfigFile = configPath + w.Config.Profile = "first" + t.Setenv(userWorkspaceDirEnv, "") + dir, err := userWorkspaceDir(t.Context(), w) + require.NoError(t, err) + assert.Equal(t, "/Workspace/first", dir) + }) + + t.Run("default profile", func(t *testing.T) { + w := newFakeWorkspaceClient(t) + w.Config.ConfigFile = configPath + w.Config.Profile = "" + t.Setenv(userWorkspaceDirEnv, "") + dir, err := userWorkspaceDir(t.Context(), w) + require.NoError(t, err) + assert.Equal(t, "/Workspace/second", dir) + }) + + t.Run("environment wins", func(t *testing.T) { + w := newFakeWorkspaceClient(t) + w.Config.ConfigFile = configPath + w.Config.Profile = "first" + t.Setenv(userWorkspaceDirEnv, "/Workspace/env") + dir, err := userWorkspaceDir(t.Context(), w) + require.NoError(t, err) + assert.Equal(t, "/Workspace/env", dir) + }) +} + func TestEnsureExperimentDirectory(t *testing.T) { ctx := t.Context() w := newFakeWorkspaceClient(t) diff --git a/experimental/air/cmd/snapshot_package.go b/experimental/air/cmd/snapshot_package.go index 4b95b698dc6..5556e2bf4c2 100644 --- a/experimental/air/cmd/snapshot_package.go +++ b/experimental/air/cmd/snapshot_package.go @@ -3,6 +3,7 @@ package aircmd import ( "bytes" "context" + "errors" "fmt" "os" "os/exec" @@ -57,45 +58,21 @@ func createPlainTarball(ctx context.Context, repoPath, outputTarball string, inc } outName := filepath.Base(outputTarball) - args := []string{"-czf", outName} - - // Exclude macOS AppleDouble files: they sort before the real top-level dir and - // hijack a remote `head -1` parse. No-op on Linux. - args = append(args, "--exclude=._*") - - // Never ship .git — provenance flows via the git_state.json sidecar. - args = append(args, "--exclude=.git") - - // Honor .gitignore if present. - gitignorePath := filepath.Join(repoPath, ".gitignore") - if patterns, err := parseGitignore(gitignorePath); err == nil { - for _, p := range patterns { - if strings.Contains(p, "/") { - // Anchor path-relative patterns to the archive root so they don't - // match identically-named paths in subdirectories. - args = append(args, "--exclude="+dirName+"/"+strings.TrimPrefix(p, "/")) - } else { - args = append(args, "--exclude="+p) - } - } - } - - // Archive from the parent so the directory name is preserved; with include_paths, - // prefix each so entries nest under it (matching git archive --prefix). -C only - // affects the file operands that follow it, not the -f archive path (which - // resolves against tar's working dir, set to outDirAbs below). - args = append(args, "-C", parent) - if len(includePaths) > 0 { - for _, p := range includePaths { - args = append(args, dirName+"/"+p) - } - } else { - args = append(args, dirName) + files, err := snapshotFiles(ctx, repoPath, includePaths) + if err != nil { + return err } + args := []string{"-czf", outName, "-C", parent, "--null", "--no-recursion", "-T", "-"} cmd := exec.CommandContext(ctx, "tar", args...) // Run tar in the output directory so the bare -f basename lands there. cmd.Dir = outDirAbs + var stdin bytes.Buffer + for _, file := range files { + stdin.WriteString(filepath.ToSlash(filepath.Join(dirName, file))) + stdin.WriteByte(0) + } + cmd.Stdin = &stdin var stderr bytes.Buffer cmd.Stderr = &stderr if err := cmd.Run(); err != nil { @@ -107,43 +84,48 @@ func createPlainTarball(ctx context.Context, repoPath, outputTarball string, inc return nil } -// parseGitignore reads a .gitignore and returns tar --exclude patterns. It mirrors -// the Python CLI's lossy normalization so plain-tar snapshots exclude the same set: -// -// - comments (#…) and blank lines are skipped; -// - negation patterns (!…) are unsupported by tar --exclude and skipped; -// - a trailing "/" (directory marker) is stripped; -// - "**" is not a path-separator-agnostic wildcard in tar, so "**/foo" → "foo" -// and "foo/**" → "foo"; a mid-path "**" has no tar equivalent and is skipped. -// -// A missing file returns (nil, error); callers treat any error as "no patterns". -func parseGitignore(path string) ([]string, error) { - data, err := os.ReadFile(path) +func snapshotFiles(ctx context.Context, repoPath string, includePaths []string) ([]string, error) { + args := []string{"-C", repoPath, "ls-files", "-z", "--cached", "--others", "--exclude-standard"} + if !newGitRepo(repoPath).isRepository(ctx) { + gitDir, err := os.MkdirTemp("", "air-snapshot-git-") + if err != nil { + return nil, fmt.Errorf("failed to create temporary git metadata: %w", err) + } + defer os.RemoveAll(gitDir) + cmd := exec.CommandContext(ctx, "git", "--git-dir", gitDir, "--work-tree", repoPath, "init", "--quiet") + if output, err := cmd.CombinedOutput(); err != nil { + return nil, fmt.Errorf("failed to initialize temporary git metadata: %w: %s", err, strings.TrimSpace(string(output))) + } + args = []string{"--git-dir", gitDir, "--work-tree", repoPath, "ls-files", "-z", "--cached", "--others", "--exclude-standard"} + } + + if len(includePaths) > 0 { + args = append(args, "--") + args = append(args, includePaths...) + } + output, err := exec.CommandContext(ctx, "git", args...).Output() if err != nil { - return nil, err + return nil, fmt.Errorf("failed to evaluate git ignore rules: %w", err) } - var patterns []string - for raw := range strings.SplitSeq(string(data), "\n") { - line := strings.TrimRight(raw, " \t\r") - if line == "" || strings.HasPrefix(line, "#") { + var files []string + for raw := range bytes.SplitSeq(output, []byte{0}) { + if len(raw) == 0 { + continue + } + name := filepath.ToSlash(string(raw)) + base := filepath.Base(name) + if name == ".git" || strings.HasPrefix(name, ".git/") || strings.HasPrefix(base, "._") { continue } - if strings.HasPrefix(line, "!") { + _, err := os.Lstat(filepath.Join(repoPath, filepath.FromSlash(name))) + if errors.Is(err, os.ErrNotExist) { continue } - line = strings.TrimRight(line, "/") - if strings.Contains(line, "**") { - switch { - case strings.HasPrefix(line, "**/"): - line = line[len("**/"):] - case strings.HasSuffix(line, "/**"): - line = line[:len(line)-len("/**")] - default: - continue - } + if err != nil { + return nil, fmt.Errorf("failed to inspect snapshot path %q: %w", name, err) } - patterns = append(patterns, line) + files = append(files, filepath.FromSlash(name)) } - return patterns, nil + return files, nil } diff --git a/experimental/air/cmd/snapshot_package_test.go b/experimental/air/cmd/snapshot_package_test.go index 22561f4e2ce..360592466a4 100644 --- a/experimental/air/cmd/snapshot_package_test.go +++ b/experimental/air/cmd/snapshot_package_test.go @@ -130,32 +130,42 @@ func TestCreatePlainTarball_IncludePaths(t *testing.T) { assert.NotContains(t, entries, dirName+"/a.txt") } -func TestParseGitignore(t *testing.T) { - dir := t.TempDir() - path := filepath.Join(dir, ".gitignore") - content := "# comment\n" + - "\n" + - "*.log\n" + - "!keep.log\n" + // negation: skipped - "build/\n" + // trailing slash stripped - "**/node_modules\n" + // **/foo -> foo - "dist/**\n" + // foo/** -> foo - "a/**/b\n" + // mid ** : skipped - "src/config\n" // path-relative kept as-is - require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) - - patterns, err := parseGitignore(path) - require.NoError(t, err) - assert.Equal(t, []string{ - "*.log", - "build", - "node_modules", - "dist", - "src/config", - }, patterns) +func TestCreatePlainTarball_HonorsNestedGitignoreAndNegation(t *testing.T) { + repo := t.TempDir() + writeRepoFile(t, repo, ".gitignore", "*.log\n!keep.log\n/root-only.txt\n") + writeRepoFile(t, repo, "drop.log", "drop") + writeRepoFile(t, repo, "keep.log", "keep") + writeRepoFile(t, repo, "root-only.txt", "drop") + writeRepoFile(t, repo, "nested/root-only.txt", "keep") + writeRepoFile(t, repo, "nested/.gitignore", "*.tmp\n!keep.tmp\n") + writeRepoFile(t, repo, "nested/drop.tmp", "drop") + writeRepoFile(t, repo, "nested/keep.tmp", "keep") + + out := filepath.Join(t.TempDir(), "snap.tar.gz") + require.NoError(t, createPlainTarball(t.Context(), repo, out, nil)) + + dirName := filepath.Base(repo) + entries := tarballEntries(t, out) + assert.NotContains(t, entries, dirName+"/drop.log") + assert.Contains(t, entries, dirName+"/keep.log") + assert.NotContains(t, entries, dirName+"/root-only.txt") + assert.Contains(t, entries, dirName+"/nested/root-only.txt") + assert.NotContains(t, entries, dirName+"/nested/drop.tmp") + assert.Contains(t, entries, dirName+"/nested/keep.tmp") } -func TestParseGitignore_Missing(t *testing.T) { - _, err := parseGitignore(filepath.Join(t.TempDir(), "nope")) - require.Error(t, err) +func TestCreatePlainTarball_SkipsDeletedTrackedFiles(t *testing.T) { + repo := newTestRepo(t) + writeRepoFile(t, repo, "keep.txt", "keep") + writeRepoFile(t, repo, "deleted.txt", "deleted") + commitAll(t, repo, "init") + require.NoError(t, os.Remove(filepath.Join(repo, "deleted.txt"))) + + out := filepath.Join(t.TempDir(), "snap.tar.gz") + require.NoError(t, createPlainTarball(t.Context(), repo, out, nil)) + + dirName := filepath.Base(repo) + entries := tarballEntries(t, out) + assert.Contains(t, entries, dirName+"/keep.txt") + assert.NotContains(t, entries, dirName+"/deleted.txt") } diff --git a/libs/databrickscfg/profile_value.go b/libs/databrickscfg/profile_value.go new file mode 100644 index 00000000000..b45cc20274e --- /dev/null +++ b/libs/databrickscfg/profile_value.go @@ -0,0 +1,35 @@ +package databrickscfg + +import ( + "errors" + "io/fs" + + "github.com/databricks/databricks-sdk-go/config" +) + +// ProfileValue returns a property from the selected configuration profile. An +// empty profile name uses the configured default-profile resolution order. +func ProfileValue(cfg *config.Config, key string) (string, error) { + file, err := config.LoadFile(cfg.ConfigFile) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return "", nil + } + return "", err + } + profile := cfg.Profile + if profile == "" { + profile = GetDefaultProfileFrom(file) + } + if profile == "" { + return "", nil + } + if !file.HasSection(profile) { + return "", nil + } + section := file.Section(profile) + if !section.HasKey(key) { + return "", nil + } + return section.Key(key).Value(), nil +} diff --git a/libs/databrickscfg/profile_value_test.go b/libs/databrickscfg/profile_value_test.go new file mode 100644 index 00000000000..b20c8595d54 --- /dev/null +++ b/libs/databrickscfg/profile_value_test.go @@ -0,0 +1,56 @@ +package databrickscfg + +import ( + "os" + "path/filepath" + "testing" + + "github.com/databricks/databricks-sdk-go/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestProfileValue(t *testing.T) { + path := filepath.Join(t.TempDir(), ".databrickscfg") + require.NoError(t, os.WriteFile(path, []byte(` +[__settings__] +default_profile = second + +[first] +host = https://first.test +databricks_internal_user_workspace_dir = /Workspace/first + +[second] +host = https://second.test +databricks_internal_user_workspace_dir = /Workspace/second +`), 0o600)) + + value, err := ProfileValue(&config.Config{ConfigFile: path, Profile: "first"}, "databricks_internal_user_workspace_dir") + require.NoError(t, err) + assert.Equal(t, "/Workspace/first", value) + + value, err = ProfileValue(&config.Config{ConfigFile: path}, "databricks_internal_user_workspace_dir") + require.NoError(t, err) + assert.Equal(t, "/Workspace/second", value) + + value, err = ProfileValue(&config.Config{ConfigFile: path, Profile: "missing"}, "databricks_internal_user_workspace_dir") + require.NoError(t, err) + assert.Empty(t, value) + + value, err = ProfileValue(&config.Config{ConfigFile: path, Profile: "first"}, "missing") + require.NoError(t, err) + assert.Empty(t, value) +} + +func TestProfileValueSingleProfileDefault(t *testing.T) { + path := filepath.Join(t.TempDir(), ".databrickscfg") + require.NoError(t, os.WriteFile(path, []byte(` +[only] +host = https://only.test +databricks_internal_user_workspace_dir = /Workspace/only +`), 0o600)) + + value, err := ProfileValue(&config.Config{ConfigFile: path}, "databricks_internal_user_workspace_dir") + require.NoError(t, err) + assert.Equal(t, "/Workspace/only", value) +}