os: fix join-path (#21425)

vlang · May 13, 2024 · 76142b1 · 76142b1
1 parent 35f6523
commit 76142b1
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 18 deletions.
diff --git a/vlib/os/dir_expansions_test.v b/vlib/os/dir_expansions_test.v
@@ -23,7 +23,7 @@ fn test_ensure_folder_is_writable() {
 
 fn test_expand_tilde_to_home() {
  os.setenv('HOME', '/tmp/home/folder', true)
- os.setenv('USERPROFILE', '/tmp/home/folder', true)
+ os.setenv('USERPROFILE', r'\tmp\home\folder', true)
  //
  home_test := os.join_path(os.home_dir(), 'test', 'tilde', 'expansion')
  home_expansion_test := os.expand_tilde_to_home(os.join_path('~', 'test', 'tilde',

diff --git a/vlib/os/os.v b/vlib/os/os.v
@@ -568,6 +568,8 @@ pub fn is_file(path string) bool {
 
 // join_path joins any number of path elements into a single path, separating
 // them with a platform-specific path_separator. Empty elements are ignored.
+// Windows platform output will rewrite forward slashes to backslash.
+// Consider looking at the unit tests in os_test.v for semi-formal API.
 @[manualfree]
 pub fn join_path(base string, dirs ...string) string {
  // TODO: fix freeing of `dirs` when the passed arguments are variadic,
@@ -576,25 +578,36 @@ pub fn join_path(base string, dirs ...string) string {
  defer {
  unsafe { sb.free() }
  }
- sbase := base.trim_right('\\/')
- defer {
- unsafe { sbase.free() }
- }
- sb.write_string(sbase)
- for d in dirs {
- if d != '' {
- sb.write_string(path_separator)
- sb.write_string(d)
+ mut needs_sep := false
+ if base != '' {
+ $if windows {
+ sb.write_string(base.replace('/', '\\'))
+ } $else {
+ sb.write_string(base)
+ }
+ needs_sep = !base.ends_with(path_separator)
+ }
+ for od in dirs {
+ if od != '' && od != '.' {
+ mut md := od
+ $if windows {
+ md = md.replace('/', '\\')
+ }
+ // NOTE(hholst80): split_any not available in js backend,
+ // which could have been more clean way to implement this.
+ nestdirs := md.split(path_separator)
+ for id in nestdirs {
+ if id != '' && id != '.' {
+ if needs_sep {
+ sb.write_string(path_separator)
+ }
+ sb.write_string(id)
+ needs_sep = !id.ends_with(path_separator)
+ }
+ }
  }
  }
- mut res := sb.str()
- if sbase == '' {
- res = res.trim_left(path_separator)
- }
- if res.contains('/./') {
- // Fix `join_path("/foo/bar", "./file.txt")` => `/foo/bar/./file.txt`
- res = res.replace('/./', '/')
- }
+ res := sb.str()
  return res
 }
 

diff --git a/vlib/os/os_test.c.v b/vlib/os/os_test.c.v
@@ -619,11 +619,16 @@ fn test_join() {
  assert os.join_path('v', 'vlib', 'os') == 'v\\vlib\\os'
  assert os.join_path('', 'f1', 'f2') == 'f1\\f2'
  assert os.join_path('v', '', 'dir') == 'v\\dir'
+ assert os.join_path('v', 'foo/bar', 'dir') == 'v\\foo\\bar\\dir'
+ assert os.join_path('v', 'foo/bar\\baz', '/dir') == 'v\\foo\\bar\\baz\\dir'
+ assert os.join_path('C:', 'f1\\..', 'f2') == 'C:\\f1\\..\\f2'
  } $else {
  assert os.join_path('v', 'vlib', 'os') == 'v/vlib/os'
  assert os.join_path('/foo/bar', './file.txt') == '/foo/bar/file.txt'
  assert os.join_path('', 'f1', 'f2') == 'f1/f2'
  assert os.join_path('v', '', 'dir') == 'v/dir'
+ assert os.join_path('/', 'test') == '/test'
+ assert os.join_path('/foo/bar', './.././file.txt') == '/foo/bar/../file.txt'
  }
 }