From 51c6608935baf0168431ce7e421975e97a01048c Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 21 Jan 2025 15:46:38 +0100 Subject: [PATCH 1/5] Add test for sync root outside of git --- .../dotdot-git}/databricks.yml | 0 acceptance/bundle/syncroot/dotdot-git/output.txt | 11 +++++++++++ acceptance/bundle/syncroot/dotdot-git/script | 5 +++++ .../bundle/syncroot/dotdot-nogit/databricks.yml | 5 +++++ .../dotdot-nogit}/output.txt | 0 .../dotdot-nogit}/script | 0 6 files changed, 21 insertions(+) rename acceptance/bundle/{sync-paths-dotdot => syncroot/dotdot-git}/databricks.yml (100%) create mode 100644 acceptance/bundle/syncroot/dotdot-git/output.txt create mode 100644 acceptance/bundle/syncroot/dotdot-git/script create mode 100644 acceptance/bundle/syncroot/dotdot-nogit/databricks.yml rename acceptance/bundle/{sync-paths-dotdot => syncroot/dotdot-nogit}/output.txt (100%) rename acceptance/bundle/{sync-paths-dotdot => syncroot/dotdot-nogit}/script (100%) diff --git a/acceptance/bundle/sync-paths-dotdot/databricks.yml b/acceptance/bundle/syncroot/dotdot-git/databricks.yml similarity index 100% rename from acceptance/bundle/sync-paths-dotdot/databricks.yml rename to acceptance/bundle/syncroot/dotdot-git/databricks.yml diff --git a/acceptance/bundle/syncroot/dotdot-git/output.txt b/acceptance/bundle/syncroot/dotdot-git/output.txt new file mode 100644 index 0000000000..f1dc5fb014 --- /dev/null +++ b/acceptance/bundle/syncroot/dotdot-git/output.txt @@ -0,0 +1,11 @@ +Error: path "$TMPDIR" is not within repository root "$TMPDIR/myrepo" + +Name: test-bundle +Target: default +Workspace: + User: $USERNAME + Path: /Workspace/Users/$USERNAME/.bundle/test-bundle/default + +Found 1 error + +Exit code: 1 diff --git a/acceptance/bundle/syncroot/dotdot-git/script b/acceptance/bundle/syncroot/dotdot-git/script new file mode 100644 index 0000000000..532c67442b --- /dev/null +++ b/acceptance/bundle/syncroot/dotdot-git/script @@ -0,0 +1,5 @@ +mkdir myrepo +cd myrepo +cp ../databricks.yml . +git-repo-init +$CLI bundle validate diff --git a/acceptance/bundle/syncroot/dotdot-nogit/databricks.yml b/acceptance/bundle/syncroot/dotdot-nogit/databricks.yml new file mode 100644 index 0000000000..7215ffea29 --- /dev/null +++ b/acceptance/bundle/syncroot/dotdot-nogit/databricks.yml @@ -0,0 +1,5 @@ +bundle: + name: test-bundle +sync: + paths: + - .. diff --git a/acceptance/bundle/sync-paths-dotdot/output.txt b/acceptance/bundle/syncroot/dotdot-nogit/output.txt similarity index 100% rename from acceptance/bundle/sync-paths-dotdot/output.txt rename to acceptance/bundle/syncroot/dotdot-nogit/output.txt diff --git a/acceptance/bundle/sync-paths-dotdot/script b/acceptance/bundle/syncroot/dotdot-nogit/script similarity index 100% rename from acceptance/bundle/sync-paths-dotdot/script rename to acceptance/bundle/syncroot/dotdot-nogit/script From 509d79f757c5fb2ee5bd6a944c90570cc77d7a92 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 21 Jan 2025 15:48:14 +0100 Subject: [PATCH 2/5] add comments --- acceptance/bundle/syncroot/dotdot-git/script | 1 + acceptance/bundle/syncroot/dotdot-nogit/script | 1 + 2 files changed, 2 insertions(+) diff --git a/acceptance/bundle/syncroot/dotdot-git/script b/acceptance/bundle/syncroot/dotdot-git/script index 532c67442b..28ee46a34b 100644 --- a/acceptance/bundle/syncroot/dotdot-git/script +++ b/acceptance/bundle/syncroot/dotdot-git/script @@ -1,3 +1,4 @@ +# This should error, we do not allow syncroot outside of git repo mkdir myrepo cd myrepo cp ../databricks.yml . diff --git a/acceptance/bundle/syncroot/dotdot-nogit/script b/acceptance/bundle/syncroot/dotdot-nogit/script index 72555b332a..13ee046a29 100644 --- a/acceptance/bundle/syncroot/dotdot-nogit/script +++ b/acceptance/bundle/syncroot/dotdot-nogit/script @@ -1 +1,2 @@ +# This not should error, syncroot can be outside bundle root $CLI bundle validate From c15d378c98ce02229a4aa726adf68926f6d1c69e Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 21 Jan 2025 17:20:02 +0100 Subject: [PATCH 3/5] add replacement with backslash escaped (for windows) --- libs/testdiff/replacement.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/libs/testdiff/replacement.go b/libs/testdiff/replacement.go index 207f425aae..7e1ec0af5b 100644 --- a/libs/testdiff/replacement.go +++ b/libs/testdiff/replacement.go @@ -52,13 +52,26 @@ func (r *ReplacementsContext) append(pattern *regexp.Regexp, replacement string) } func (r *ReplacementsContext) appendLiteral(old, new string) { + // Transform the replacement string such that `$` is interpreted as a literal dollar sign. + // For more information about how the replacement string is used, see [regexp.Regexp.Expand]. + new = strings.ReplaceAll(new, `$`, `$$`) + r.append( // Transform the input strings such that they can be used as literal strings in regular expressions. regexp.MustCompile(regexp.QuoteMeta(old)), - // Transform the replacement string such that `$` is interpreted as a literal dollar sign. - // For more information about how the replacement string is used, see [regexp.Regexp.Expand]. - strings.ReplaceAll(new, `$`, `$$`), + new, ) + + // Make sure we capture paths in messages like this one: + // Error: path "C:\\Users\\runneradmin\\AppData\\Local\\Temp\\TestAcceptbundlesyncrootdotdot-git1631369685\\001" is not within repository root + old1 := strings.ReplaceAll(old, "\\", "\\\\") + if old1 != old { + r.append( + // Transform the input strings such that they can be used as literal strings in regular expressions. + regexp.MustCompile(regexp.QuoteMeta(old1)), + new, + ) + } } func (r *ReplacementsContext) Set(old, new string) { From 8dd305132d246553124eb544803cb61dfaeb72a8 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 22 Jan 2025 10:40:57 +0100 Subject: [PATCH 4/5] fixes to make windows work --- acceptance/acceptance_test.go | 12 ++-- acceptance/bundle/syncroot/dotdot-git/script | 2 +- .../bundle/syncroot/dotdot-nogit/output.txt | 2 +- libs/testdiff/replacement.go | 58 +++++++++++++------ 4 files changed, 49 insertions(+), 25 deletions(-) diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index cfcb0d29fa..9a4564ffae 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -188,12 +188,12 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont tmpDir = t.TempDir() } - repls.Set("/private"+tmpDir, "$TMPDIR") - repls.Set("/private"+filepath.Dir(tmpDir), "$TMPPARENT") - repls.Set("/private"+filepath.Dir(filepath.Dir(tmpDir)), "$TMPGPARENT") - repls.Set(tmpDir, "$TMPDIR") - repls.Set(filepath.Dir(tmpDir), "$TMPPARENT") - repls.Set(filepath.Dir(filepath.Dir(tmpDir)), "$TMPGPARENT") + // Converts C:\Users\DENIS~1.BIL -> C:\Users\denis.bilenko + tmpDirEvalled, err1 := filepath.EvalSymlinks(tmpDir) + if err1 == nil && tmpDirEvalled != tmpDir { + repls.SetPathWithParents(tmpDirEvalled, "$TMPDIR") + } + repls.SetPathWithParents(tmpDir, "$TMPDIR") scriptContents := readMergedScriptContents(t, dir) testutil.WriteFile(t, filepath.Join(tmpDir, EntryPointScript), scriptContents) diff --git a/acceptance/bundle/syncroot/dotdot-git/script b/acceptance/bundle/syncroot/dotdot-git/script index 28ee46a34b..65a0da61f7 100644 --- a/acceptance/bundle/syncroot/dotdot-git/script +++ b/acceptance/bundle/syncroot/dotdot-git/script @@ -3,4 +3,4 @@ mkdir myrepo cd myrepo cp ../databricks.yml . git-repo-init -$CLI bundle validate +$CLI bundle validate | sed 's/\\\\/\//g' diff --git a/acceptance/bundle/syncroot/dotdot-nogit/output.txt b/acceptance/bundle/syncroot/dotdot-nogit/output.txt index 11db3e9ee0..34059e2769 100644 --- a/acceptance/bundle/syncroot/dotdot-nogit/output.txt +++ b/acceptance/bundle/syncroot/dotdot-nogit/output.txt @@ -1,4 +1,4 @@ -Error: path "$TMPPARENT" is not within repository root "$TMPDIR" +Error: path "$TMPDIR_PARENT" is not within repository root "$TMPDIR" Name: test-bundle Target: default diff --git a/libs/testdiff/replacement.go b/libs/testdiff/replacement.go index 7e1ec0af5b..ca76b159cf 100644 --- a/libs/testdiff/replacement.go +++ b/libs/testdiff/replacement.go @@ -3,7 +3,9 @@ package testdiff import ( "encoding/json" "fmt" + "path/filepath" "regexp" + "runtime" "slices" "strings" @@ -52,26 +54,13 @@ func (r *ReplacementsContext) append(pattern *regexp.Regexp, replacement string) } func (r *ReplacementsContext) appendLiteral(old, new string) { - // Transform the replacement string such that `$` is interpreted as a literal dollar sign. - // For more information about how the replacement string is used, see [regexp.Regexp.Expand]. - new = strings.ReplaceAll(new, `$`, `$$`) - r.append( // Transform the input strings such that they can be used as literal strings in regular expressions. regexp.MustCompile(regexp.QuoteMeta(old)), - new, + // Transform the replacement string such that `$` is interpreted as a literal dollar sign. + // For more information about how the replacement string is used, see [regexp.Regexp.Expand]. + strings.ReplaceAll(new, `$`, `$$`), ) - - // Make sure we capture paths in messages like this one: - // Error: path "C:\\Users\\runneradmin\\AppData\\Local\\Temp\\TestAcceptbundlesyncrootdotdot-git1631369685\\001" is not within repository root - old1 := strings.ReplaceAll(old, "\\", "\\\\") - if old1 != old { - r.append( - // Transform the input strings such that they can be used as literal strings in regular expressions. - regexp.MustCompile(regexp.QuoteMeta(old1)), - new, - ) - } } func (r *ReplacementsContext) Set(old, new string) { @@ -87,13 +76,48 @@ func (r *ReplacementsContext) Set(old, new string) { if err == nil { encodedOld, err := json.Marshal(old) if err == nil { - r.appendLiteral(string(encodedOld), string(encodedNew)) + r.appendLiteral(trimQuotes(string(encodedOld)), trimQuotes(string(encodedNew))) } } r.appendLiteral(old, new) } +func trimQuotes(s string) string { + if len(s) > 0 && s[0] == '"' { + s = s[1:] + } + if len(s) > 0 && s[len(s)-1] == '"' { + s = s[:len(s)-1] + } + return s +} + +func (r *ReplacementsContext) SetPath(old, new string) { + r.Set(old, new) + + if runtime.GOOS != "windows" { + return + } + + // Support both forward and backward slashes + m1 := strings.ReplaceAll(old, "\\", "/") + if m1 != old { + r.Set(m1, new) + } + + m2 := strings.ReplaceAll(old, "/", "\\") + if m2 != old && m2 != m1 { + r.Set(m2, new) + } +} + +func (r *ReplacementsContext) SetPathWithParents(old, new string) { + r.SetPath(old, new) + r.SetPath(filepath.Dir(old), new+"_PARENT") + r.SetPath(filepath.Dir(filepath.Dir(old)), new+"_GPARENT") +} + func PrepareReplacementsWorkspaceClient(t testutil.TestingT, r *ReplacementsContext, w *databricks.WorkspaceClient) { t.Helper() // in some clouds (gcp) w.Config.Host includes "https://" prefix in others it's really just a host (azure) From f91be8aef98d212c7bda6d4901e8387f045d8686 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 22 Jan 2025 10:42:43 +0100 Subject: [PATCH 5/5] comments --- acceptance/bundle/syncroot/dotdot-git/script | 2 +- acceptance/bundle/syncroot/dotdot-nogit/script | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/acceptance/bundle/syncroot/dotdot-git/script b/acceptance/bundle/syncroot/dotdot-git/script index 65a0da61f7..0706a1d5e5 100644 --- a/acceptance/bundle/syncroot/dotdot-git/script +++ b/acceptance/bundle/syncroot/dotdot-git/script @@ -1,4 +1,4 @@ -# This should error, we do not allow syncroot outside of git repo +# This should error, we do not allow syncroot outside of git repo. mkdir myrepo cd myrepo cp ../databricks.yml . diff --git a/acceptance/bundle/syncroot/dotdot-nogit/script b/acceptance/bundle/syncroot/dotdot-nogit/script index 13ee046a29..d3388903e1 100644 --- a/acceptance/bundle/syncroot/dotdot-nogit/script +++ b/acceptance/bundle/syncroot/dotdot-nogit/script @@ -1,2 +1,2 @@ -# This not should error, syncroot can be outside bundle root +# This should not error, syncroot can be outside bundle root. $CLI bundle validate