diff --git a/master_changes.md b/master_changes.md index bf99e35d587..f6523cff801 100644 --- a/master_changes.md +++ b/master_changes.md @@ -37,6 +37,7 @@ users) * Use a non-underline uppercase character to denotate the default when asking a question [#6289 @hannesm @kit-ty-kate - fix #6288] ## Switch + * [BUG] Fix `opam switch remove ` failure when it is a linked switch [#6276 @btjorge - fix #6275] ## Config @@ -121,6 +122,7 @@ users) ## Reftests ### Tests + * Add switch removal test: failure on removal linked switch [#6276 @btjorge] ### Engine @@ -161,3 +163,4 @@ users) * `OpamStd.Sys.{uname,getconf}`: now accepts only one argument as parameter, as per their documentation [#6230 @kit-ty-kate] * `OpamStubs.get_stdout_ws_col`: new Unix-only function returning the number of columns of the current terminal window [#6244 @kit-ty-kate] * `OpamSystem`: add `is_archive_from_string` that does the same than `is_archive` but without looking at the file, only analysing the string (extension) [#6219 @rjbou] + * `OpamSystem.remove_dir`: do not fail with an exception when directory is a symbolic link [#6276 @btjorge @rjbou - fix #6275] diff --git a/src/core/opamSystem.ml b/src/core/opamSystem.ml index 75dbb588743..7cbb19ad0f4 100644 --- a/src/core/opamSystem.ml +++ b/src/core/opamSystem.ml @@ -146,18 +146,24 @@ let remove_file_t ?(with_log=true) file = with Unix.Unix_error _ as e -> internal_error "Cannot remove %s (%s)." file (Printexc.to_string e) +let is_reg_dir dir = + match Unix.lstat dir with + | {Unix.st_kind = Unix.S_DIR; _} -> true + | {Unix.st_kind = + Unix.(S_REG | S_LNK | S_CHR | S_BLK | S_FIFO | S_SOCK); _} -> + false + | exception Unix.(Unix_error (ENOENT, _, _)) when Sys.win32 -> + (* This is usually caused by invalid symlinks extracted by a + Cygwin/MSYS2 tar. *) + false + let rec remove_dir_t dir = let files = get_files dir in List.iter (fun file -> let file = Filename.concat dir file in - match Unix.lstat file with - | {Unix.st_kind = Unix.S_DIR; _} -> + if is_reg_dir file then remove_dir_t file - | {Unix.st_kind = Unix.(S_REG | S_LNK | S_CHR | S_BLK | S_FIFO | S_SOCK); _} -> - remove_file_t ~with_log:false file - | exception Unix.(Unix_error (ENOENT, _, _)) when Sys.win32 -> - (* This is usually caused by invalid symlinks extracted by a - Cygwin/MSYS2 tar. *) + else remove_file_t ~with_log:false file ) files; Unix.rmdir dir @@ -165,7 +171,7 @@ let rec remove_dir_t dir = let remove_dir dir = log "rmdir %s" dir; if Sys.file_exists dir then begin - if Sys.is_directory dir then + if is_reg_dir dir then remove_dir_t dir else remove_file_t ~with_log:true dir diff --git a/tests/reftests/dune.inc b/tests/reftests/dune.inc index 643bbb93218..479710810d0 100644 --- a/tests/reftests/dune.inc +++ b/tests/reftests/dune.inc @@ -1763,6 +1763,27 @@ %{targets} (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:switch-list.test} %{read-lines:testing-env})))) +(rule + (alias reftest-switch-remove) + (enabled_if (and (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1)))) + (action + (diff switch-remove.test switch-remove.out))) + +(alias + (name reftest) + (enabled_if (and (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1)))) + (deps (alias reftest-switch-remove))) + +(rule + (targets switch-remove.out) + (deps root-N0REP0) + (enabled_if (and (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1)))) + (package opam) + (action + (with-stdout-to + %{targets} + (run ./run.exe %{exe:../../src/client/opamMain.exe.exe} %{dep:switch-remove.test} %{read-lines:testing-env})))) + (rule (alias reftest-switch-set) (enabled_if (and (or (<> %{env:TESTALL=1} 0) (= %{env:TESTN0REP0=0} 1)))) diff --git a/tests/reftests/switch-remove.test b/tests/reftests/switch-remove.test new file mode 100644 index 00000000000..01af184355a --- /dev/null +++ b/tests/reftests/switch-remove.test @@ -0,0 +1,7 @@ +N0REP0 +### :: Remove linked local switch +### opam switch create foo --empty +### opam switch link foo +Directory ${BASEDIR} set to use switch foo. +Just remove ${BASEDIR}/_opam to unlink. +### opam switch remove .