Merge: core: fix rmdir not reporting some errors

Fix `rmdir` to raise an error on failure to remove a folder, instead of on success. Also fix error management in recursive calls where some errors could be lost.

The name of the attribute `Path::last_error` is misleading with `rmdir` as it stores the first error of the last call, and it is set to `null` if there is no error (overwriting the true last error), so it very rarely contains the actual last error. Note that, `Path::last_error` has a different logic than `Stream::last_error` which does keep the last error and does not reset it. In the future, I would suggest to rename `Path::last_error` to `error` to avoid confusion or to use an array that is cleared at the beginning of each method (as it is currently done with `Path::last_error`).

Pull-Request: #2309
Reviewed-by: Jean Privat <jean@pryen.org>
Reviewed-by: Romain Chanoir <romain.chanoir@viacesi.fr>
Reviewed-by: Lucas Bajolet <r4pass@hotmail.com>

diff --git a/contrib/nitiwiki/tests/res/wiki1_nitiwiki_status.res b/contrib/nitiwiki/tests/res/wiki1_nitiwiki_status.res
index 6857efd..e1ca4f7 100644 (file)
--- a/contrib/nitiwiki/tests/res/wiki1_nitiwiki_status.res
+++ b/contrib/nitiwiki/tests/res/wiki1_nitiwiki_status.res
@@ -3,7 +3,7 @@ name: wiki1
 config: wiki1/config.ini
 
 There is modified files:
- * pages
+ + pages
  + pages/index.md
 
 Use nitiwiki --render to render modified files

diff --git a/contrib/nitiwiki/tests/res/wiki2_nitiwiki_status.res b/contrib/nitiwiki/tests/res/wiki2_nitiwiki_status.res
index e161e3e..cf691c8 100644 (file)
--- a/contrib/nitiwiki/tests/res/wiki2_nitiwiki_status.res
+++ b/contrib/nitiwiki/tests/res/wiki2_nitiwiki_status.res
@@ -3,13 +3,13 @@ name: wiki2
 config: wiki2/config.ini
 
 There is modified files:
- * pages
+ + pages
  + pages/contact.md
  + pages/index.md
  + pages/other_page.md
  + pages/sec1
  + pages/sec1/index.md
- * pages/sec2
+ + pages/sec2
  + pages/sec2/index.md
  + pages/sec2/sub-sec21
  + pages/sec2/sub-sec21/index.md

diff --git a/lib/core/file.nit b/lib/core/file.nit
index dc1ccb7..cd35cdc 100644 (file)
--- a/lib/core/file.nit
+++ b/lib/core/file.nit
@@ -705,26 +705,32 @@ class Path
                return st.is_dir
        end
 
-       # Delete a directory and all of its content
+       # Recursively delete a directory and all of its content
        #
        # Does not go through symbolic links and may get stuck in a cycle if there
        # is a cycle in the file system.
        #
-       # `last_error` is updated to contains the error information on error, and null on success.
-       # The method does not stop on the first error and try to remove most file and directories.
+       # `last_error` is updated with the first encountered error, or null on success.
+       # The method does not stop on the first error and tries to remove the most files and directories.
        #
        # ~~~
        # var path = "/does/not/exists/".to_path
        # path.rmdir
        # assert path.last_error != null
+       #
+       # path = "/tmp/path/to/create".to_path
+       # path.to_s.mkdir
+       # assert path.exists
+       # path.rmdir
+       # assert path.last_error == null
        # ~~~
        fun rmdir
        do
-               last_error = null
+               var first_error = null
                for file in self.files do
                        var stat = file.link_stat
                        if stat == null then
-                               last_error = file.last_error
+                               if first_error == null then first_error = file.last_error
                                continue
                        end
                        if stat.is_dir then
@@ -733,15 +739,16 @@ class Path
                        else
                                file.delete
                        end
-                       if last_error == null then last_error = file.last_error
+                       if first_error == null then first_error = file.last_error
                end
 
                # Delete the directory itself if things are fine
-               if last_error == null then
-                       if path.to_cstring.rmdir then
-                               last_error = new IOError("Cannot remove `{self}`: {sys.errno.strerror}")
+               if first_error == null then
+                       if not path.to_cstring.rmdir then
+                               first_error = new IOError("Cannot remove `{self}`: {sys.errno.strerror}")
                        end
                end
+               self.last_error = first_error
        end
 
        redef fun ==(other) do return other isa Path and simplified.path == other.simplified.path