OSX is not Linux is not windows (was Re: [MERGE] win32-specific fixes for selftest and bzrlib)

[Vincent Ladeuil]

>>>>> "bialix" == Alexander Belchenko <bialix at ukr.net> writes:

<snip/>

    bialix> === modified file 'bzrlib/tests/repository_implementations/test_commit_builder.py'
    bialix> --- bzrlib/tests/repository_implementations/test_commit_builder.py	2007-09-04 03:53:07 +0000
    bialix> +++ bzrlib/tests/repository_implementations/test_commit_builder.py	2007-09-06 13:30:55 +0000
    bialix> @@ -415,11 +416,20 @@
    bialix>          path = 'name'
    bialix>          make_before(path)
    bialix>          def change_kind():
    bialix> -            try:
    bialix> +            # 'Easier to ask for forgiveness than permission' paradigm
    bialix> +            # is monstrous here
    bialix> +            #try:
    bialix> +            #    os.unlink(path)
    bialix> +            #except OSError, e:
    bialix> +            #    if not (e.errno == EISDIR or
    bialix> +            #        (sys.platform == 'win32' and e.errno == EACCES)):
    bialix> +            #        raise
    bialix> +            #    os.rmdir(path)
    bialix> +            # because it's a test we can do it via direct check
    bialix> +            # (bialix 20070906)
    bialix> +            if not os.path.isdir(path):
    bialix>                  os.unlink(path)
    bialix> -            except OSError, e:
    bialix> -                if e.errno != EISDIR:
    bialix> -                    raise
    bialix> +            else:
    bialix>                  os.rmdir(path)
    bialix>              make_after(path)
    bialix>          self._add_commit_change_check_changed(tree, path, change_kind)


    bialix> Error handling looks monstrous for me and I'm leaning
    bialix> towards direct check (isdir).

I encountered the same problem while trying to run the full test
suite under OSX. And after fixing it I remembered your email so
now, change_kind should be written like this:

            try:
                os.unlink(path)
            except OSError, e:
                if not (e.errno == errno.EISDIR
                        or (sys.platform == 'win32'
                            and e.errno == errno.EACCES)
                        or (sys.platform == 'darwin'
                            and e.errno == errno.EPERM)):
                    raise
                os.rmdir(path)
            make_after(path)

The funny thing is that I rewrote it exactly like you (the leading not).

Seriously, at this point I strongly vote for:

           if os.path.isdir(path):
               os.rmdir(path)
           else:
               os.unlink(path)

Just in case some other OS have another brilliant idea about
POSIX compliance.

And that's a good reason to break the 'Easier to ask for
forgiveness than permission' rule (yes pun about EPERM intended).

      Vincent