[patch] add Transport.rename

Robey Pointer robey at lag.net
Tue Feb 21 02:58:03 GMT 2006


On 20 Feb 2006, at 18:01, Robert Collins wrote:

> On Tue, 2006-02-21 at 12:57 +1100, Martin Pool wrote:
>> On Tue, 2006-02-14 at 09:08 -0600, John A Meinel wrote:
>
>> Here's a patch which adds a new Transport.rename() method, which is
>> required to fail if the destination is an existing nonempty  
>> directory.
>> This should be possible to atomically detect on all transports.  (I
>> don't think we can avoid overwriting existing files on on  
>> transports or
>> filesystems without a race.)  It also adds tests that all existing
>> transports actually do this.
>
> +1

+1 also.  The default implementation in base Transport will be buggy,  
though (since the 2 different operations are implemented identically).


>> This leaves Transport.move() with the existing meaning, which is  
>> to try
>> to overwrite an existing target using fancy_rename.  Keeping this  
>> there
>> seemed like the least intrusive change.
>>
>>> I *don't* think we want to make fancy_rename a higher level than
>>> transport. Because then all the clients would need to know if  
>>> they are
>>> using a Transport (or platform) which doesn't support atomic  
>>> rename, and
>>> then they need to activate the fancy rename.
>>
>> I think it's better to make it a utility which can be used by  
>> transport
>> implementation that need it to meet the right contract for move().
>> Perhaps it should be a protected Transport method.
>
> +1 for this direction.

John and I were discussing this in another thread, so I'm already +1  
on it.  Moving fancy_rename into Transport would probably also fix  
the bug in the default implementation.

robey





More information about the bazaar mailing list