On 01/10/13 15:18, Chris Larson wrote:

On Thu, Jan 10, 2013 at 7:11 AM, Chris Larson <clarson@kergoth.com> wrote:

On Thu, Jan 10, 2013 at 7:10 AM, Chris Larson <clarson@kergoth.com> wrote:
On Thu, Jan 10, 2013 at 1:46 AM, Martin Ertsaas <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 are almost right. On normal folders, this will work with recurse=True, but on /tmp folders you get: OSError: [Errno 1] Operation not permitted: '/tmp/tmpl8qBDW' on the unlink call. This makes the if test inside the except catching false, and nothing happens.

I'm not sure why this happens, as I am allowed to remove it using shutil.rmtree. Making a folder in linux with the exact same permissions does work however, so it seems to be some difference in how the os handles the call.

Other than that, I feel that it is cleaner to have an explicit check like this, and not use the exception framework for control flow. Which means in the except just use raise, or just ignore it and let someone else handle it like we actually do.
To get back the same functionality though, we should change the isdir call to: 'if os.path.isdir(name) and recurse:'

Are you totally opposed to changing this? What I don't want to do is to add exc.errno == errno.EPERM to the line, which will just bloath it.

- Martin