Fixed sync include/exclude not working correctly on Windows (#2609)

andrewnester · web-flow · commit 834245b5eee3 · 2025-04-02T15:16:28.000+02:00
## Changes Fixed sync include/exclude not working correctly on Windows Fixes #1572 ## Why We need to convert `include` and `exclude` in the `sync` block to use Unix-style slashes because this is required for the ignore.GitIgnore we use in libs/fileset to work correctly. ## Tests Added acceptance test
diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md
@@ -15,6 +15,7 @@ Previously ".internal" folder under artifact_path was not cleaned up as expected
 
 ### Bundles
 * Fixed cleaning up artifact path .internal folder ([#2572](https://github.com/databricks/cli/pull/2572))
+* Fixed sync include/exclude not working correctly on Windows ([#2609](https://github.com/databricks/cli/pull/2609))
 * Added support for quality monitors in deployment bind/unbind commands ([#2583](https://github.com/databricks/cli/pull/2583))
 * Comment out email\_notifications section in builtin templates ([#2565](https://github.com/databricks/cli/pull/2565))
 * New DATABRICKS_BUNDLE_RESTRICTED_CODE_EXECUTION environment variable that rejects running scripts and Python code when it is set ([#2598](https://github.com/databricks/cli/pull/2598))
diff --git a/acceptance/bundle/validate/sync_patterns/.gitignore b/acceptance/bundle/validate/sync_patterns/.gitignore
@@ -0,0 +1 @@
+*.yml
diff --git a/acceptance/bundle/validate/sync_patterns/conf/dir/test.txt b/acceptance/bundle/validate/sync_patterns/conf/dir/test.txt
diff --git a/acceptance/bundle/validate/sync_patterns/conf/dir/test.yml b/acceptance/bundle/validate/sync_patterns/conf/dir/test.yml
diff --git a/acceptance/bundle/validate/sync_patterns/databricks.yml b/acceptance/bundle/validate/sync_patterns/databricks.yml
@@ -0,0 +1,8 @@
+bundle:
+  name: sync_patterns
+
+sync:
+  include:
+    - conf/dir/*.yml
+  exclude:
+    - conf/dir/*.txt
diff --git a/acceptance/bundle/validate/sync_patterns/out.sync.txt b/acceptance/bundle/validate/sync_patterns/out.sync.txt
@@ -0,0 +1,4 @@
+{
+  "method": "POST",
+  "path": "/api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/sync_patterns/default/files/conf/dir/test.yml"
+}
diff --git a/acceptance/bundle/validate/sync_patterns/output.txt b/acceptance/bundle/validate/sync_patterns/output.txt
@@ -0,0 +1,27 @@
+
+>>> [CLI] bundle validate
+Name: sync_patterns
+Target: default
+Workspace:
+  User: [USERNAME]
+  Path: /Workspace/Users/[USERNAME]/.bundle/sync_patterns/default
+
+Validation OK!
+
+>>> [CLI] bundle validate -o json
+{
+  "exclude": [
+    "conf/dir/*.txt"
+  ],
+  "include": [
+    "conf/dir/*.yml"
+  ],
+  "paths": [
+    "."
+  ]
+}
+
+>>> [CLI] bundle deploy
+Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/sync_patterns/default/files...
+Deploying resources...
+Deployment complete!
diff --git a/acceptance/bundle/validate/sync_patterns/script b/acceptance/bundle/validate/sync_patterns/script
@@ -0,0 +1,5 @@
+trace $CLI bundle validate
+trace $CLI bundle validate -o json | jq '.sync'
+trace $CLI bundle deploy
+jq 'select(.path | test("/api/2.0/workspace-files/import-file/Workspace/Users/.*/.bundle/sync_patterns/default/files/conf/dir/test.yml"))' out.requests.txt  > out.sync.txt
+rm out.requests.txt
diff --git a/acceptance/bundle/validate/sync_patterns/test.toml b/acceptance/bundle/validate/sync_patterns/test.toml
@@ -0,0 +1,2 @@
+Cloud = false
+RecordRequests = true
diff --git a/bundle/config/mutator/rewrite_sync_paths.go b/bundle/config/mutator/rewrite_sync_paths.go
@@ -2,6 +2,7 @@ package mutator
 
 import (
 	"context"
+	"fmt"
 	"path/filepath"
 
 	"github.com/databricks/cli/bundle"
@@ -49,11 +50,38 @@ func (m *rewriteSyncPaths) Apply(ctx context.Context, b *bundle.Bundle) diag.Dia
 			if err != nil {
 				return dyn.InvalidValue, err
 			}
-			v, err = dyn.Map(v, "include", dyn.Foreach(m.makeRelativeTo(b.BundleRootPath)))
+
+			makeRelativeFn := m.makeRelativeTo(b.BundleRootPath)
+
+			// Makes include and exclude paths relative to the bundle root first.
+			// Then converts them to use Unix-style slashes.
+			// This is required for the ignore.GitIgnore we use in libs/fileset to work correctly.
+			v, err = dyn.Map(v, "include", dyn.Foreach(func(p dyn.Path, val dyn.Value) (dyn.Value, error) {
+				relPath, err := makeRelativeFn(p, val)
+				if err != nil {
+					return dyn.InvalidValue, err
+				}
+				str, ok := relPath.AsString()
+				if !ok {
+					return dyn.InvalidValue, fmt.Errorf("expected string value but got %s", relPath.Kind())
+				}
+				return dyn.NewValue(filepath.ToSlash(str), relPath.Locations()), nil
+			}))
 			if err != nil {
 				return dyn.InvalidValue, err
 			}
-			v, err = dyn.Map(v, "exclude", dyn.Foreach(m.makeRelativeTo(b.BundleRootPath)))
+
+			v, err = dyn.Map(v, "exclude", dyn.Foreach(func(p dyn.Path, val dyn.Value) (dyn.Value, error) {
+				relPath, err := makeRelativeFn(p, val)
+				if err != nil {
+					return dyn.InvalidValue, err
+				}
+				str, ok := relPath.AsString()
+				if !ok {
+					return dyn.InvalidValue, fmt.Errorf("expected string value but got %s", relPath.Kind())
+				}
+				return dyn.NewValue(filepath.ToSlash(str), relPath.Locations()), nil
+			}))
 			if err != nil {
 				return dyn.InvalidValue, err
 			}
diff --git a/bundle/config/mutator/rewrite_sync_paths_test.go b/bundle/config/mutator/rewrite_sync_paths_test.go
@@ -46,10 +46,10 @@ func TestRewriteSyncPathsRelative(t *testing.T) {
 
 	assert.Equal(t, filepath.Clean("."), b.Config.Sync.Paths[0])
 	assert.Equal(t, filepath.Clean("../common"), b.Config.Sync.Paths[1])
-	assert.Equal(t, filepath.Clean("foo"), b.Config.Sync.Include[0])
-	assert.Equal(t, filepath.Clean("a/bar"), b.Config.Sync.Include[1])
-	assert.Equal(t, filepath.Clean("a/b/baz"), b.Config.Sync.Exclude[0])
-	assert.Equal(t, filepath.Clean("a/b/c/qux"), b.Config.Sync.Exclude[1])
+	assert.Equal(t, "foo", b.Config.Sync.Include[0])
+	assert.Equal(t, "a/bar", b.Config.Sync.Include[1])
+	assert.Equal(t, "a/b/baz", b.Config.Sync.Exclude[0])
+	assert.Equal(t, "a/b/c/qux", b.Config.Sync.Exclude[1])
 }
 
 func TestRewriteSyncPathsAbsolute(t *testing.T) {
@@ -85,10 +85,10 @@ func TestRewriteSyncPathsAbsolute(t *testing.T) {
 
 	assert.Equal(t, filepath.Clean("."), b.Config.Sync.Paths[0])
 	assert.Equal(t, filepath.Clean("../common"), b.Config.Sync.Paths[1])
-	assert.Equal(t, filepath.Clean("foo"), b.Config.Sync.Include[0])
-	assert.Equal(t, filepath.Clean("a/bar"), b.Config.Sync.Include[1])
-	assert.Equal(t, filepath.Clean("a/b/baz"), b.Config.Sync.Exclude[0])
-	assert.Equal(t, filepath.Clean("a/b/c/qux"), b.Config.Sync.Exclude[1])
+	assert.Equal(t, "foo", b.Config.Sync.Include[0])
+	assert.Equal(t, "a/bar", b.Config.Sync.Include[1])
+	assert.Equal(t, "a/b/baz", b.Config.Sync.Exclude[0])
+	assert.Equal(t, "a/b/c/qux", b.Config.Sync.Exclude[1])
 }
 
 func TestRewriteSyncPathsErrorPaths(t *testing.T) {
diff --git a/bundle/config/mutator/sync_infer_root.go b/bundle/config/mutator/sync_infer_root.go
@@ -106,11 +106,14 @@ func (m *syncInferRoot) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagno
 	for i, p := range b.Config.Sync.Paths {
 		b.Config.Sync.Paths[i] = filepath.Join(rel, p)
 	}
+
+	// Convert include and exclude in the sync block to use Unix-style slashes.
+	// This is required for the ignore.GitIgnore we use in libs/fileset to work correctly.
 	for i, p := range b.Config.Sync.Include {
-		b.Config.Sync.Include[i] = filepath.Join(rel, p)
+		b.Config.Sync.Include[i] = filepath.ToSlash(filepath.Join(rel, p))
 	}
 	for i, p := range b.Config.Sync.Exclude {
-		b.Config.Sync.Exclude[i] = filepath.Join(rel, p)
+		b.Config.Sync.Exclude[i] = filepath.ToSlash(filepath.Join(rel, p))
 	}
 
 	// Configure the sync root path.
diff --git a/bundle/config/mutator/sync_infer_root_test.go b/bundle/config/mutator/sync_infer_root_test.go
@@ -103,8 +103,8 @@ func TestSyncInferRoot_ParentDirectory(t *testing.T) {
 
 	// Check that the paths are updated.
 	assert.Equal(t, []string{"common"}, b.Config.Sync.Paths)
-	assert.Equal(t, []string{filepath.FromSlash("dir/foo"), filepath.FromSlash("dir/bar")}, b.Config.Sync.Include)
-	assert.Equal(t, []string{filepath.FromSlash("dir/baz"), filepath.FromSlash("dir/qux")}, b.Config.Sync.Exclude)
+	assert.Equal(t, []string{"dir/foo", "dir/bar"}, b.Config.Sync.Include)
+	assert.Equal(t, []string{"dir/baz", "dir/qux"}, b.Config.Sync.Exclude)
 }
 
 func TestSyncInferRoot_ManyParentDirectories(t *testing.T) {
@@ -135,12 +135,12 @@ func TestSyncInferRoot_ManyParentDirectories(t *testing.T) {
 	// Check that the paths are updated.
 	assert.Equal(t, []string{"common"}, b.Config.Sync.Paths)
 	assert.Equal(t, []string{
-		filepath.FromSlash("dir/that/is/very/deeply/nested/foo"),
-		filepath.FromSlash("dir/that/is/very/deeply/nested/bar"),
+		"dir/that/is/very/deeply/nested/foo",
+		"dir/that/is/very/deeply/nested/bar",
 	}, b.Config.Sync.Include)
 	assert.Equal(t, []string{
-		filepath.FromSlash("dir/that/is/very/deeply/nested/baz"),
-		filepath.FromSlash("dir/that/is/very/deeply/nested/qux"),
+		"dir/that/is/very/deeply/nested/baz",
+		"dir/that/is/very/deeply/nested/qux",
 	}, b.Config.Sync.Exclude)
 }
 
diff --git a/bundle/tests/relative_path_with_includes_test.go b/bundle/tests/relative_path_with_includes_test.go
@@ -2,7 +2,7 @@ package config_tests
 
 import (
 	"context"
-	"path/filepath"
+	"path"
 	"testing"
 
 	"github.com/databricks/cli/bundle"
@@ -23,16 +23,16 @@ func TestRelativePathsWithIncludes(t *testing.T) {
 	assert.ElementsMatch(
 		t,
 		[]string{
-			filepath.Join("folder_a", "*.*"),
-			filepath.Join("subfolder", "folder_c", "*.*"),
+			path.Join("folder_a", "*.*"),
+			path.Join("subfolder", "folder_c", "*.*"),
 		},
 		b.Config.Sync.Include,
 	)
 	assert.ElementsMatch(
 		t,
 		[]string{
-			filepath.Join("folder_b", "*.*"),
-			filepath.Join("subfolder", "folder_d", "*.*"),
+			path.Join("folder_b", "*.*"),
+			path.Join("subfolder", "folder_d", "*.*"),
 		},
 		b.Config.Sync.Exclude,
 	)
diff --git a/bundle/tests/sync_include_exclude_no_matches_test.go b/bundle/tests/sync_include_exclude_no_matches_test.go
@@ -3,6 +3,7 @@ package config_tests
 import (
 	"context"
 	"fmt"
+	"path"
 	"path/filepath"
 	"testing"
 
@@ -32,8 +33,8 @@ func TestSyncIncludeExcludeNoMatchesTest(t *testing.T) {
 	require.Equal(t, 11, diags[0].Locations[0].Column)
 
 	summaries := []string{
-		fmt.Sprintf("Pattern %s does not match any files", filepath.Join("src", "*")),
-		fmt.Sprintf("Pattern %s does not match any files", filepath.Join("tests", "*")),
+		fmt.Sprintf("Pattern %s does not match any files", path.Join("src", "*")),
+		fmt.Sprintf("Pattern %s does not match any files", path.Join("tests", "*")),
 	}
 
 	require.Equal(t, diag.Warning, diags[1].Severity)
diff --git a/bundle/tests/sync_test.go b/bundle/tests/sync_test.go
@@ -14,19 +14,19 @@ func TestSyncOverride(t *testing.T) {
 	b = loadTarget(t, "./sync/override", "development")
 	assert.Equal(t, filepath.FromSlash("sync/override"), b.SyncRootPath)
 	assert.Equal(t, []string{"."}, b.Config.Sync.Paths)
-	assert.ElementsMatch(t, []string{filepath.FromSlash("src/*"), filepath.FromSlash("tests/*")}, b.Config.Sync.Include)
-	assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude)
+	assert.ElementsMatch(t, []string{"src/*", "tests/*"}, b.Config.Sync.Include)
+	assert.ElementsMatch(t, []string{"dist"}, b.Config.Sync.Exclude)
 
 	b = loadTarget(t, "./sync/override", "staging")
 	assert.Equal(t, filepath.FromSlash("sync/override"), b.SyncRootPath)
 	assert.Equal(t, []string{"."}, b.Config.Sync.Paths)
-	assert.ElementsMatch(t, []string{filepath.FromSlash("src/*"), filepath.FromSlash("fixtures/*")}, b.Config.Sync.Include)
+	assert.ElementsMatch(t, []string{"src/*", "fixtures/*"}, b.Config.Sync.Include)
 	assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude)
 
 	b = loadTarget(t, "./sync/override", "prod")
 	assert.Equal(t, filepath.FromSlash("sync/override"), b.SyncRootPath)
 	assert.Equal(t, []string{"."}, b.Config.Sync.Paths)
-	assert.ElementsMatch(t, []string{filepath.FromSlash("src/*")}, b.Config.Sync.Include)
+	assert.ElementsMatch(t, []string{"src/*"}, b.Config.Sync.Include)
 	assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude)
 }
 
@@ -36,13 +36,13 @@ func TestSyncOverrideNoRootSync(t *testing.T) {
 	b = loadTarget(t, "./sync/override_no_root", "development")
 	assert.Equal(t, filepath.FromSlash("sync/override_no_root"), b.SyncRootPath)
 	assert.Equal(t, []string{"."}, b.Config.Sync.Paths)
-	assert.ElementsMatch(t, []string{filepath.FromSlash("tests/*")}, b.Config.Sync.Include)
-	assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude)
+	assert.ElementsMatch(t, []string{"tests/*"}, b.Config.Sync.Include)
+	assert.ElementsMatch(t, []string{"dist"}, b.Config.Sync.Exclude)
 
 	b = loadTarget(t, "./sync/override_no_root", "staging")
 	assert.Equal(t, filepath.FromSlash("sync/override_no_root"), b.SyncRootPath)
 	assert.Equal(t, []string{"."}, b.Config.Sync.Paths)
-	assert.ElementsMatch(t, []string{filepath.FromSlash("fixtures/*")}, b.Config.Sync.Include)
+	assert.ElementsMatch(t, []string{"fixtures/*"}, b.Config.Sync.Include)
 	assert.ElementsMatch(t, []string{}, b.Config.Sync.Exclude)
 
 	b = loadTarget(t, "./sync/override_no_root", "prod")
@@ -64,8 +64,8 @@ func TestSyncNil(t *testing.T) {
 	b = loadTarget(t, "./sync/nil", "staging")
 	assert.Equal(t, filepath.FromSlash("sync/nil"), b.SyncRootPath)
 	assert.Equal(t, []string{"."}, b.Config.Sync.Paths)
-	assert.ElementsMatch(t, []string{filepath.FromSlash("tests/*")}, b.Config.Sync.Include)
-	assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude)
+	assert.ElementsMatch(t, []string{"tests/*"}, b.Config.Sync.Include)
+	assert.ElementsMatch(t, []string{"dist"}, b.Config.Sync.Exclude)
 }
 
 func TestSyncNilRoot(t *testing.T) {
@@ -80,8 +80,8 @@ func TestSyncNilRoot(t *testing.T) {
 	b = loadTarget(t, "./sync/nil_root", "staging")
 	assert.Equal(t, filepath.FromSlash("sync/nil_root"), b.SyncRootPath)
 	assert.Equal(t, []string{"."}, b.Config.Sync.Paths)
-	assert.ElementsMatch(t, []string{filepath.FromSlash("tests/*")}, b.Config.Sync.Include)
-	assert.ElementsMatch(t, []string{filepath.FromSlash("dist")}, b.Config.Sync.Exclude)
+	assert.ElementsMatch(t, []string{"tests/*"}, b.Config.Sync.Include)
+	assert.ElementsMatch(t, []string{"dist"}, b.Config.Sync.Exclude)
 }
 
 func TestSyncPaths(t *testing.T) {

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +{
 +  "method": "POST",
 +  "path": "/api/2.0/workspace-files/import-file/Workspace/Users/[USERNAME]/.bundle/sync_patterns/default/files/conf/dir/test.yml"
 +}
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+Cloud = false`
	`2`	`+RecordRequests = true`