Michael Haggerty <mhag...@alum.mit.edu> writes:

> If a file or directory that we are trying to remove disappears (e.g.,
> because another process has pruned it), do not consider it an error.
>
> Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu>
> ---
>  dir.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index 11e1520..716b613 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1476,7 +1476,9 @@ static int remove_dir_recurse(struct strbuf *path, int 
> flag, int *kept_up)
>       flag &= ~REMOVE_DIR_KEEP_TOPLEVEL;
>       dir = opendir(path->buf);
>       if (!dir) {
> -             if (errno == EACCES && !keep_toplevel)
> +             if (errno == ENOENT)
> +                     return keep_toplevel ? -1 : 0;

If we were told that "foo/bar must go, but do not bother removing
foo/", is it an error if foo itself did not exist?  I think this
step does not change the behaviour from the original (we used to say
"oh, we were told to keep_toplevel, and the top-level cannot be read
for whatever reason, so it is an error"), but because this series is
giving us a finer grained error diagnosis, we may want to think
about it further, perhaps as a follow-up.

I also wonder why the keep-toplevel logic is in this "recurse" part
of the callchain. If everything in "foo/bar/" can be removed but
"foo/bar/" is unreadable, it is OK, when opendir("foo/bar") failed
with EACCESS, to attempt to rmdir("foo/bar") whether we were told
not to attempt removing "foo/", no?

> +             else if (errno == EACCES && !keep_toplevel)

That is, I am wondering why this part just checks !keep_toplevel,
not

        (!keep_toplevel || has_dir_separator(path->buf))

or something.

>                       /*
>                        * An empty dir could be removable even if it
>                        * is unreadable:
> @@ -1496,13 +1498,21 @@ static int remove_dir_recurse(struct strbuf *path, 
> int flag, int *kept_up)
>  
>               strbuf_setlen(path, len);
>               strbuf_addstr(path, e->d_name);
> -             if (lstat(path->buf, &st))
> -                     ; /* fall thru */
> -             else if (S_ISDIR(st.st_mode)) {
> +             if (lstat(path->buf, &st)) {
> +                     if (errno == ENOENT)
> +                             /*
> +                              * file disappeared, which is what we
> +                              * wanted anyway
> +                              */
> +                             continue;
> +                     /* fall thru */
> +             } else if (S_ISDIR(st.st_mode)) {
>                       if (!remove_dir_recurse(path, flag, &kept_down))
>                               continue; /* happy */
> -             } else if (!only_empty && !unlink(path->buf))
> +             } else if (!only_empty &&
> +                        (!unlink(path->buf) || errno == ENOENT)) {
>                       continue; /* happy, too */
> +             }
>  
>               /* path too long, stat fails, or non-directory still exists */
>               ret = -1;
> @@ -1512,7 +1522,7 @@ static int remove_dir_recurse(struct strbuf *path, int 
> flag, int *kept_up)
>  
>       strbuf_setlen(path, original_len);
>       if (!ret && !keep_toplevel && !kept_down)
> -             ret = rmdir(path->buf);
> +             ret = (!rmdir(path->buf) || errno == ENOENT) ? 0 : -1;
>       else if (kept_up)
>               /*
>                * report the uplevel that it is not an error that we
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to