[PATCH 3 of 3] util.makedirs: cleanup of termination of recursion

[Mads Kiilerich]

On 08/22/2011 10:25 AM, Adrian Buehlmann wrote:
> On 2011-08-22 10:07, Adrian Buehlmann wrote:
>> If we want to postpone (and reorder) the parent calculation, we could do:
>>
>> diff --git a/mercurial/util.py b/mercurial/util.py
>> --- a/mercurial/util.py
>> +++ b/mercurial/util.py
>> @@ -780,12 +780,12 @@
>>
>>   def makedirs(name, mode=None):
>>       """recursive directory creation with parent mode inheritance"""
>> -    parent = os.path.abspath(os.path.dirname(name))
>>       try:
>>           os.mkdir(name)
>>       except OSError, err:
>>           if err.errno == errno.EEXIST:
>>               return
>> +        parent = os.path.dirname(os.path.abspath(name))
>>           if not name or parent == name or err.errno != errno.ENOENT:
>>               raise
>>           makedirs(parent, mode)
>>
>
> or perhaps:
>
> # HG changeset patch
> # User Adrian Buehlmann<adrian at cadifra.com>
> # Date 1314001316 -7200
> # Node ID 906f611f9c32ec362d7c4e9d67397ad05f232445
> # Parent  5a50f36ec65f3eb2246407f46fdf9bf53e38aca7
> util: postpone and reorder parent calculation in makedirs
>
> diff --git a/mercurial/util.py b/mercurial/util.py
> --- a/mercurial/util.py
> +++ b/mercurial/util.py
> @@ -780,13 +780,15 @@
>
>   def makedirs(name, mode=None):
>       """recursive directory creation with parent mode inheritance"""
> -    parent = os.path.abspath(os.path.dirname(name))
>       try:
>           os.mkdir(name)
>       except OSError, err:
>           if err.errno == errno.EEXIST:
>               return
> -        if not name or parent == name or err.errno != errno.ENOENT:
> +        if err.errno != errno.ENOENT or not name:
> +            raise
> +        parent = os.path.dirname(os.path.abspath(name))
> +        if parent == name:
>               raise
>           makedirs(parent, mode)
>           os.mkdir(name)

Agreed. I realized this morning while waking up that the assumption that 
dirname 'reduces' the problem was wrong.

The path operations are however relatively cheap and most of the time we 
won't get that far, so now I prefer:

         parent = os.path.dirname(os.path.abspath(name))
         if err.errno != errno.ENOENT or not name or parent == name:
             raise

I will ignore that we perhaps in principle should add some normpath and 
realpath to this ...

/Mads