[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