RFC: version (big) file snapshots with storage outside a Mercurial repo with snap

Klaus Koch kuk42 at gmx.net
Tue Aug 17 18:37:51 UTC 2010


On Aug 16, 2010, at 10:33 PM, Benoit Boissinot wrote:

> [please keep the cc list]
> 
> 
> Indeed. Ideally it should be completely transparent. The hash should
> not depend on where the file is stored.
> That's why it would be nice if the new changegroup protocol would
> support that kind of functionality (very similar to lightweight
> copies).
Yes.  Please keep also in mind that one may want to control whether the data of the (big) snapshot files is transferred as well.  In snap you can avoid to transmit big data by pointing the paths 'snap-store' and 'snap-default' to the same 'store'.

>> 
>>> Finally, I noticed a lot of places in the code that read
>>> 
>>>     finally:
>>>         del(fsrc)
>>>         del(fdst)
>>>         del(frep)
>>> 
>>> just before the end of the function/method.
>>> 
>>> Is that meant to trigger the close method of those file handles? If so,
>>> then calling close explicitly is better since there is not guarantee
>>> exactly when the objects are reclaimed.
>>> 
>>> The normal CPython implementation will clone the files when you delete
>>> the last reference to them since it uses a reference counting scheme,
>>> but this is not true for the other Python implementations that use a
>>> more modern garbage collector.
>> 
>> It follows the advised method in
>> http://mercurial.selenic.com/wiki/DealingWithDestructors
>> 
>> The reasoning is that we do not know whether fsrc, fdst, or frep were
>> created successfully, i.e., if they are file objects with a close
>> method.  For example, fsrc may be opened successfully, but fdst raises
>> an exception.  Then we could close fsrc, but not fdst and frep.  Of
>> course, we could use
> 
> It seems in all the places where you del, you have an assignement to
> the filehandle.
> So fsrc can't be None (an open() call can't return None, it raises an
> exception).
> 
> Which means you should be able to convert to filehandle.close() everywhere.
> 
> cheers,
> 
> Benoit
Yes, the fsrc is no problem, but the open call for fdst could raise an exception.  Anyway, I will just change it to if fsrc: fsrc.close() etc.

Klaus


More information about the Mercurial-devel mailing list