diff --git a/.nextchanges/air/7639.md b/.nextchanges/air/7639.md new file mode 100644 index 00000000000..6649b534fdd --- /dev/null +++ b/.nextchanges/air/7639.md @@ -0,0 +1 @@ +* `air run` now rejects a `code_source.include_paths` entry that matches no files instead of uploading a snapshot without it. ([#7639](https://github.com/databricks/cli/pull/7639)) diff --git a/cmd/air/snapshot_package.go b/cmd/air/snapshot_package.go index c1dc9c5f89d..174c4b17806 100644 --- a/cmd/air/snapshot_package.go +++ b/cmd/air/snapshot_package.go @@ -8,6 +8,7 @@ import ( "io" "os" "os/exec" + "path" "path/filepath" "strings" @@ -193,9 +194,39 @@ func snapshotFiles(ctx context.Context, repoPath string, includePaths []string, if len(files) == 0 { return nil, emptySnapshotError(ctx, repoPath, includePaths, isGitRepo) } + if unmatched := unmatchedIncludePaths(includePaths, files); len(unmatched) > 0 { + return nil, fmt.Errorf("code_source include_paths select no files under %s: %s. Check that each path exists under root_path and is not ignored by git (`git check-ignore -v `)", filepath.ToSlash(repoPath), strings.Join(unmatched, ", ")) + } return files, nil } +// unmatchedIncludePaths returns the include_paths entries that select none of files. +// git ls-files ignores a pathspec that matches nothing, so a mistyped entry would be +// left out of the snapshot and the run would fail on the node after compute was +// provisioned. Entries with glob characters are skipped because git matches them as +// patterns rather than as path prefixes. +func unmatchedIncludePaths(includePaths []string, files []snapshotFile) []string { + var unmatched []string + for _, p := range includePaths { + if strings.ContainsAny(p, "*?[") { + continue + } + prefix := path.Clean(filepath.ToSlash(p)) + matched := prefix == "." + for _, file := range files { + rel := filepath.ToSlash(file.rel) + if rel == prefix || strings.HasPrefix(rel, prefix+"/") { + matched = true + break + } + } + if !matched { + unmatched = append(unmatched, p) + } + } + return unmatched +} + // emptySnapshotError explains why no file was selected for the snapshot. An empty // tarball is accepted by the upload but fails on the node, after compute was // provisioned, so it is rejected here with the most likely cause. diff --git a/cmd/air/snapshot_package_test.go b/cmd/air/snapshot_package_test.go index 068997e2671..cad2a6a5bba 100644 --- a/cmd/air/snapshot_package_test.go +++ b/cmd/air/snapshot_package_test.go @@ -324,3 +324,42 @@ func TestSnapshotFiles_RejectsIncludePathsMatchingNothing(t *testing.T) { assert.Contains(t, err.Error(), "contains no files") assert.Contains(t, err.Error(), "include_paths") } + +func TestSnapshotFiles_RejectsIncludePathMatchingNothingAlongsideOthers(t *testing.T) { + repo := t.TempDir() + writeRepoFile(t, repo, "ray_bootstrap.sh", "#!/bin/bash\n") + writeRepoFile(t, repo, "my-job/main.py", "print(1)\n") + + _, err := snapshotFiles(t.Context(), repo, []string{"ray_bootstrap.sh", "my_job"}, false) + require.Error(t, err) + assert.Contains(t, err.Error(), "include_paths select no files") + assert.Contains(t, err.Error(), ": my_job.") + assert.NotContains(t, err.Error(), "ray_bootstrap.sh") +} + +func TestSnapshotFiles_RejectsIncludePathWhoseFilesAreIgnored(t *testing.T) { + repo := newTestRepo(t) + writeRepoFile(t, repo, ".gitignore", "build/\n") + writeRepoFile(t, repo, "main.py", "print(1)\n") + writeRepoFile(t, repo, "build/out.bin", "x") + + _, err := snapshotFiles(t.Context(), repo, []string{"main.py", "build"}, true) + require.Error(t, err) + assert.Contains(t, err.Error(), ": build.") + assert.Contains(t, err.Error(), "check-ignore -v") +} + +func TestSnapshotFiles_AcceptsIncludePathForms(t *testing.T) { + repo := t.TempDir() + writeRepoFile(t, repo, "src/model.py", "print()") + writeRepoFile(t, repo, "README.md", "readme") + writeRepoFile(t, repo, "notes.txt", "notes") + + files, err := snapshotFiles(t.Context(), repo, []string{"./src/", "README.md", "*.txt"}, false) + require.NoError(t, err) + var rels []string + for _, file := range files { + rels = append(rels, filepath.ToSlash(file.rel)) + } + assert.ElementsMatch(t, []string{"src/model.py", "README.md", "notes.txt"}, rels) +}