Re: [PATCH] utils.py: Use shutil.rmtree if the path we wish to remove is a directory.

[Martin Ertsaas <martiert@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2641 bytes --]

On 01/10/13 15:18, Chris Larson wrote:
>
> On Thu, Jan 10, 2013 at 7:11 AM, Chris Larson <clarson@kergoth.com
> <mailto:clarson@kergoth.com>> wrote:
>
>
>     On Thu, Jan 10, 2013 at 7:10 AM, Chris Larson <clarson@kergoth.com
>     <mailto:clarson@kergoth.com>> wrote:
>
>         On Thu, Jan 10, 2013 at 1:46 AM, Martin Ertsaas
>         <martiert@gmail.com <mailto:martiert@gmail.com>> wrote:
>
>             On mac, os.unlink does not remove directories, and we
>             therefor have
>             to explicitly use shutil.rmtree if the path is a directory.
>             ---
>              lib/bb/utils.py |    5 ++++-
>              1 file changed, 4 insertions(+), 1 deletion(-)
>
>             diff --git a/lib/bb/utils.py b/lib/bb/utils.py
>             index cef0fdd..8b6d3f5 100644
>             --- a/lib/bb/utils.py
>             +++ b/lib/bb/utils.py
>             @@ -561,7 +561,10 @@ def remove(path, recurse=False):
>                  import os, errno, shutil, glob
>                  for name in glob.glob(path):
>                      try:
>             -            os.unlink(name)
>             +            if os.path.isdir(name):
>             +                shutil.rmtree(name)
>             +            else:
>             +                os.unlink(name)
>                      except OSError as exc:
>                          if recurse and exc.errno == errno.EISDIR:
>                              shutil.rmtree(name)
>
>
>
>         Look 2 lines down, where it checks to see if the os.unlink
>         failed due to it being a directory and runs shutil.rmtree if
>         that's the case.
>
>
>     I'm guessing you're trying to use it without passing recurse=True.
>
>
> Having through about it further, I think it might be best to be alter
> the code to also handle the case where recurse==False and the path is
> a directory by calling os.rmdir(). Perhaps this would reduce confusion.
> -- 
> Christopher Larson
You think to recursively call remove on the children of the directory?
Wouldn't something like:

for name in glob.glob(path):
    if os.path.isdir(name) and recurse:
        shutil.rmtree(name)
    elif os.path.isdir(name):
        os.rmdir(name)
    else:
        os.unlink(name)

Then we will possibly get an exception from os.rmdir, which we can
catch. But this also avoids the need to use the exception as a control
flow structure as it is used today.

At least in my mind, that is a lot easier to grasp than the if statement
in the exception handling, which I totally overlooked as it is an exception.

- Martin

[-- Attachment #2: Type: text/html, Size: 5508 bytes --]