diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 1f46a0a57c6..6343eb4a1f4 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -5,6 +5,7 @@ ### Notable Changes ### CLI +* `workspace export-dir` no longer aborts when a workspace object's name is not a legal local filename (e.g. a notebook named `New Notebook 2026-05-04 13:54:24` whose `:` is illegal on Windows). Such files are now skipped with a warning and the export completes ([#5171](https://github.com/databricks/cli/issues/5171)). ### Bundles * `bundle run` now prints the modern job run URL (`/jobs//runs/`) so that non-admin users permitted to view the run are taken to the run instead of the workspace homepage. diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index 2d3326b23d6..09bd0fd973a 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -1633,18 +1633,38 @@ type pathFilter struct { // contains substrings from the variants other than current. // E.g. if EnvVaryOutput is DATABRICKS_BUNDLE_ENGINE and current test running DATABRICKS_BUNDLE_ENGINE="terraform" then // notSelected contains ".direct." meaning if filename contains that (e.g. out.deploy.direct.txt) then we ignore it here. + // It also contains the infixes of every GOOS other than runtime.GOOS, so out..txt files for other OSes are ignored. notSelected []string } -// preparePathFilter builds filter based on EnvVaryOutput and current variant env. +// osVariants are the GOOS values that can tag an output file (e.g. +// out.windows.txt). Files tagged with an OS other than runtime.GOOS are skipped +// during comparison, so a single test can capture per-OS output differences +// (see cmd/workspace/export-dir-illegal-filename, databricks/cli#5171). +var osVariants = []string{"darwin", "linux", "windows"} + +// preparePathFilter builds filter based on the current GOOS and EnvVaryOutput. func preparePathFilter(config internal.TestConfig, customEnv []string) pathFilter { + var notSelected []string + for _, goos := range osVariants { + if goos != runtime.GOOS { + notSelected = append(notSelected, "."+goos+".") + } + } + notSelected = append(notSelected, envVaryNotSelected(config, customEnv)...) + return pathFilter{notSelected: notSelected} +} + +// envVaryNotSelected returns the infixes of EnvVaryOutput variants other than +// the one selected for the current test run. +func envVaryNotSelected(config internal.TestConfig, customEnv []string) []string { if config.EnvVaryOutput == nil { - return pathFilter{} + return nil } key := *config.EnvVaryOutput vals := config.EnvMatrix[key] if len(vals) <= 1 { - return pathFilter{} + return nil } selected := "" for _, kv := range customEnv { @@ -1655,7 +1675,7 @@ func preparePathFilter(config internal.TestConfig, customEnv []string) pathFilte } } if selected == "" { - return pathFilter{} + return nil } var others []string for _, v := range vals { @@ -1664,9 +1684,7 @@ func preparePathFilter(config internal.TestConfig, customEnv []string) pathFilte } others = append(others, "."+v+".") } - return pathFilter{ - notSelected: others, - } + return others } // shouldSkip returns true if the given file belongs to a non-selected variant. diff --git a/acceptance/cmd/workspace/export-dir-illegal-filename/notebook.py b/acceptance/cmd/workspace/export-dir-illegal-filename/notebook.py new file mode 100644 index 00000000000..4914a7436d9 --- /dev/null +++ b/acceptance/cmd/workspace/export-dir-illegal-filename/notebook.py @@ -0,0 +1,2 @@ +# Databricks notebook source +print("hello") diff --git a/acceptance/cmd/workspace/export-dir-illegal-filename/out.darwin.txt b/acceptance/cmd/workspace/export-dir-illegal-filename/out.darwin.txt new file mode 100644 index 00000000000..846fd8cab36 --- /dev/null +++ b/acceptance/cmd/workspace/export-dir-illegal-filename/out.darwin.txt @@ -0,0 +1,5 @@ + +>>> [CLI] workspace export-dir /test-dir [TEST_TMP_DIR]/export +Exporting files from /test-dir +/test-dir/New Notebook [TIMESTAMP] -> [TEST_TMP_DIR]/export/New Notebook [TIMESTAMP].py +Export complete diff --git a/acceptance/cmd/workspace/export-dir-illegal-filename/out.linux.txt b/acceptance/cmd/workspace/export-dir-illegal-filename/out.linux.txt new file mode 100644 index 00000000000..846fd8cab36 --- /dev/null +++ b/acceptance/cmd/workspace/export-dir-illegal-filename/out.linux.txt @@ -0,0 +1,5 @@ + +>>> [CLI] workspace export-dir /test-dir [TEST_TMP_DIR]/export +Exporting files from /test-dir +/test-dir/New Notebook [TIMESTAMP] -> [TEST_TMP_DIR]/export/New Notebook [TIMESTAMP].py +Export complete diff --git a/acceptance/cmd/workspace/export-dir-illegal-filename/out.test.toml b/acceptance/cmd/workspace/export-dir-illegal-filename/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/cmd/workspace/export-dir-illegal-filename/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/workspace/export-dir-illegal-filename/out.windows.txt b/acceptance/cmd/workspace/export-dir-illegal-filename/out.windows.txt new file mode 100644 index 00000000000..c234e37825d --- /dev/null +++ b/acceptance/cmd/workspace/export-dir-illegal-filename/out.windows.txt @@ -0,0 +1,5 @@ + +>>> [CLI] workspace export-dir /test-dir [TEST_TMP_DIR]/export +Exporting files from /test-dir +/test-dir/New Notebook [TIMESTAMP] (skipped; invalid name for local file system) +Export complete diff --git a/acceptance/cmd/workspace/export-dir-illegal-filename/output.txt b/acceptance/cmd/workspace/export-dir-illegal-filename/output.txt new file mode 100644 index 00000000000..3ba158aa4b1 --- /dev/null +++ b/acceptance/cmd/workspace/export-dir-illegal-filename/output.txt @@ -0,0 +1,2 @@ + +=== export-dir output is OS-specific; see out..txt diff --git a/acceptance/cmd/workspace/export-dir-illegal-filename/script b/acceptance/cmd/workspace/export-dir-illegal-filename/script new file mode 100644 index 00000000000..9b5070885a2 --- /dev/null +++ b/acceptance/cmd/workspace/export-dir-illegal-filename/script @@ -0,0 +1,16 @@ +$CLI workspace import "/test-dir/New Notebook 2026-05-04 13:54:24.py" --file notebook.py --format AUTO --language PYTHON + +mkdir -p "$TEST_TMP_DIR/export" + +# The notebook name contains ':', which is illegal in a local filename on Windows +# but legal on Linux/macOS. export-dir skips it with a warning on Windows and +# exports it normally elsewhere (databricks/cli#5171). The output therefore +# differs per OS, so capture it in out..txt. +case "$OSTYPE" in + msys | cygwin | win32) goos=windows ;; + darwin*) goos=darwin ;; + *) goos=linux ;; +esac + +title "export-dir output is OS-specific; see out..txt\n" +trace $CLI workspace export-dir /test-dir "$TEST_TMP_DIR/export" >"out.$goos.txt" 2>&1 diff --git a/acceptance/cmd/workspace/export-dir-illegal-filename/test.toml b/acceptance/cmd/workspace/export-dir-illegal-filename/test.toml new file mode 100644 index 00000000000..3dfbea904a5 --- /dev/null +++ b/acceptance/cmd/workspace/export-dir-illegal-filename/test.toml @@ -0,0 +1,15 @@ +Local = true +Cloud = false + +# This reproduces databricks/cli#5171. The notebook name contains a ':', which is +# a legal filename character on Linux/macOS but illegal on Windows. The export is +# therefore skipped with a warning on Windows and succeeds elsewhere, so the test +# runs on every OS and captures the divergent output in out..txt. + +# output.txt only holds an OS-independent header; the captured output lives in +# out..txt. The exported notebook lands under the temp dir on Linux/macOS; +# it is not part of the comparison, only the captured output is. +Ignore = ["export/"] + +[Env] +MSYS_NO_PATHCONV = "1" diff --git a/cmd/workspace/workspace/export_dir.go b/cmd/workspace/workspace/export_dir.go index e7965bcda91..f06d7d16b2f 100644 --- a/cmd/workspace/workspace/export_dir.go +++ b/cmd/workspace/workspace/export_dir.go @@ -125,6 +125,13 @@ func (opts *exportDirOptions) callback(ctx context.Context, workspaceFiler filer // create the file f, err := os.Create(targetPath) if err != nil { + // A workspace name can be illegal as a local filename (e.g. a ':' + // in "New Notebook 2026-05-04 13:54:24" on Windows). Skip those with + // a warning rather than aborting the whole export (#5171). + if isInvalidLocalNameError(err) { + cmdio.LogString(ctx, sourcePath+" (skipped; invalid name for local file system)") + return nil + } return err } defer f.Close() diff --git a/cmd/workspace/workspace/export_dir_other.go b/cmd/workspace/workspace/export_dir_other.go new file mode 100644 index 00000000000..e2cd1741d17 --- /dev/null +++ b/cmd/workspace/workspace/export_dir_other.go @@ -0,0 +1,11 @@ +//go:build !windows + +package workspace + +// isInvalidLocalNameError reports whether err means the workspace object could +// not be written because its name is not a legal filename on the local OS. On +// non-Windows platforms the only bytes illegal in a filename are '/' and NUL, +// neither of which can appear in a workspace object name, so this never fires. +func isInvalidLocalNameError(err error) bool { + return false +} diff --git a/cmd/workspace/workspace/export_dir_windows.go b/cmd/workspace/workspace/export_dir_windows.go new file mode 100644 index 00000000000..d1276ce2b3f --- /dev/null +++ b/cmd/workspace/workspace/export_dir_windows.go @@ -0,0 +1,21 @@ +//go:build windows + +package workspace + +import ( + "errors" + "syscall" +) + +// errorInvalidName is the Windows ERROR_INVALID_NAME code. The file APIs return +// it when a path contains characters that are illegal in a local filename, such +// as the ':' in a notebook named "New Notebook 2026-05-04 13:54:24". It is not +// declared in the standard syscall package, so we use the well-known code. +// https://learn.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499- +const errorInvalidName = syscall.Errno(0x7b) + +// isInvalidLocalNameError reports whether err means the workspace object could +// not be written because its name is not a legal filename on the local OS. +func isInvalidLocalNameError(err error) bool { + return errors.Is(err, errorInvalidName) +} diff --git a/cmd/workspace/workspace/export_dir_windows_test.go b/cmd/workspace/workspace/export_dir_windows_test.go new file mode 100644 index 00000000000..aba1e735aa4 --- /dev/null +++ b/cmd/workspace/workspace/export_dir_windows_test.go @@ -0,0 +1,56 @@ +//go:build windows + +package workspace + +import ( + "errors" + "io/fs" + "os" + "syscall" + "testing" + + "github.com/stretchr/testify/assert" +) + +// The narrow contract: only the Windows "invalid file name" error is treated as +// skippable. Genuine failures (permission, missing path, anything else) must not +// be swallowed, otherwise export-dir would silently drop files on real errors. +func TestIsInvalidLocalNameError(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + { + name: "invalid name wrapped in PathError", + err: &os.PathError{Op: "open", Path: `C:\tmp\New Notebook 13:54:24.py`, Err: syscall.Errno(0x7b)}, + want: true, + }, + { + name: "permission denied is not skipped", + err: fs.ErrPermission, + want: false, + }, + { + name: "not exist is not skipped", + err: fs.ErrNotExist, + want: false, + }, + { + name: "generic error is not skipped", + err: errors.New("boom"), + want: false, + }, + { + name: "nil is not skipped", + err: nil, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, isInvalidLocalNameError(tt.err)) + }) + } +} diff --git a/libs/testserver/fake_workspace.go b/libs/testserver/fake_workspace.go index d2925fb1e38..9b7a6fcc91c 100644 --- a/libs/testserver/fake_workspace.go +++ b/libs/testserver/fake_workspace.go @@ -8,6 +8,7 @@ import ( "os" "path" "path/filepath" + "slices" "strings" "sync" "time" @@ -370,6 +371,31 @@ func (s *FakeWorkspace) WorkspaceGetStatus(path string) Response { } } +func (s *FakeWorkspace) WorkspaceList(listPath string) Response { + defer s.LockUnlock()() + + var objects []workspace.ObjectInfo + + for filePath, entry := range s.files { + if path.Dir(filePath) == listPath { + objects = append(objects, entry.Info) + } + } + for dirPath, dirInfo := range s.directories { + if dirPath != listPath && path.Dir(dirPath) == listPath { + objects = append(objects, dirInfo) + } + } + + slices.SortFunc(objects, func(a, b workspace.ObjectInfo) int { + return strings.Compare(a.Path, b.Path) + }) + + return Response{ + Body: workspace.ListResponse{Objects: objects}, + } +} + func (s *FakeWorkspace) WorkspaceMkdirs(request workspace.Mkdirs) { defer s.LockUnlock()() s.directories[request.Path] = workspace.ObjectInfo{ diff --git a/libs/testserver/handlers.go b/libs/testserver/handlers.go index 8f611a7c7c9..0756af5d0ff 100644 --- a/libs/testserver/handlers.go +++ b/libs/testserver/handlers.go @@ -79,6 +79,11 @@ func AddDefaultHandlers(server *Server) { return req.Workspace.WorkspaceGetStatus(path) }) + server.Handle("GET", "/api/2.0/workspace/list", func(req Request) any { + path := req.URL.Query().Get("path") + return req.Workspace.WorkspaceList(path) + }) + server.Handle("POST", "/api/2.0/workspace/mkdirs", func(req Request) any { var request workspace.Mkdirs if err := json.Unmarshal(req.Body, &request); err != nil {