Skip to content

Commit 43539ca

Browse files
committed
Expand relative paths in shell.MakeDirectories
This function needed defined behavior for relative paths. For example, the default log directory for Postgres is just "log" which it interprets relative to the data directory. Issue: PGO-2558
1 parent c7842e7 commit 43539ca

File tree

2 files changed

+46
-3
lines changed

2 files changed

+46
-3
lines changed

internal/shell/paths.go

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ func CleanFileName(path string) string {
3636
// exists. It creates every directory leading to path from (but not including)
3737
// base and sets their permissions for Kubernetes, regardless of umask.
3838
//
39+
// Relative paths are expanded relative to base.
40+
//
3941
// See:
4042
// - https://pubs.opengroup.org/onlinepubs/9799919799/utilities/chmod.html
4143
// - https://pubs.opengroup.org/onlinepubs/9799919799/utilities/mkdir.html
@@ -47,8 +49,17 @@ func MakeDirectories(base string, paths ...string) string {
4749
return `test -d ` + QuoteWord(base)
4850
}
4951

50-
allPaths := slices.Clone(paths)
51-
for _, p := range paths {
52+
// Expand each path relative to the base path.
53+
expandedPaths := slices.Clone(paths)
54+
for i, p := range expandedPaths {
55+
if !filepath.IsAbs(p) {
56+
expandedPaths[i] = filepath.Join(base, p)
57+
}
58+
}
59+
60+
// Gather parent directories of each path.
61+
allPaths := slices.Clone(expandedPaths)
62+
for _, p := range expandedPaths {
5263
if r, err := filepath.Rel(base, p); err == nil && filepath.IsLocal(r) {
5364
// The result of [filepath.Rel] is a shorter representation of the full path; skip it.
5465
r = filepath.Dir(r)
@@ -72,7 +83,7 @@ func MakeDirectories(base string, paths ...string) string {
7283

7384
return `` +
7485
// Create all the paths and any missing parents.
75-
`mkdir -p ` + strings.Join(QuoteWords(paths...), " ") +
86+
`mkdir -p ` + strings.Join(QuoteWords(expandedPaths...), " ") +
7687

7788
// Try to set the permissions of every path and each parent.
7889
// This swallows the exit status of `chmod` because not all filesystems

internal/shell/paths_test.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,38 @@ func TestMakeDirectories(t *testing.T) {
9494
})
9595
})
9696

97+
t.Run("Relative", func(t *testing.T) {
98+
assert.Equal(t,
99+
MakeDirectories("/x", "one", "two/three"),
100+
`mkdir -p '/x/one' '/x/two/three' && { chmod 0775 '/x/one' '/x/two/three' '/x/two' || :; }`,
101+
"expected paths relative to base")
102+
103+
assert.Equal(t,
104+
MakeDirectories("/x/y/z", "../one", "./two", "../../../../three"),
105+
`mkdir -p '/x/y/one' '/x/y/z/two' '/three' && { chmod 0775 '/x/y/one' '/x/y/z/two' '/three' || :; }`,
106+
"expected paths relative to base")
107+
108+
script := MakeDirectories("x/y", "../one", "./two", "../../../../three")
109+
assert.Equal(t, script,
110+
`mkdir -p 'x/one' 'x/y/two' '../../three' && { chmod 0775 'x/one' 'x/y/two' '../../three' || :; }`,
111+
"expected paths relative to base")
112+
113+
// Calling `mkdir -p '../..'` works, but run it by ShellCheck as a precaution.
114+
t.Run("ShellCheckPOSIX", func(t *testing.T) {
115+
shellcheck := require.ShellCheck(t)
116+
117+
dir := t.TempDir()
118+
file := filepath.Join(dir, "script.sh")
119+
assert.NilError(t, os.WriteFile(file, []byte(script), 0o600))
120+
121+
// Expect ShellCheck for "sh" to be happy.
122+
// - https://www.shellcheck.net/wiki/SC2148
123+
cmd := exec.CommandContext(t.Context(), shellcheck, "--enable=all", "--shell=sh", file)
124+
output, err := cmd.CombinedOutput()
125+
assert.NilError(t, err, "%q\n%s", cmd.Args, output)
126+
})
127+
})
128+
97129
t.Run("Unrelated", func(t *testing.T) {
98130
assert.Equal(t,
99131
MakeDirectories("/one", "/two/three/four"),

0 commit comments

Comments
 (0)