[PATCH 4 of 7 V2] transaction: add support for non-append files

Durham Goode durham at fb.com
Tue Apr 1 01:27:36 UTC 2014


On 3/31/14 5:45 PM, "Pierre-Yves David" <pierre-yves.david at ens-lyon.org>
wrote:

>
>
>On 03/31/2014 04:19 PM, Durham Goode wrote:
>> # HG changeset patch
>> # User Durham Goode <durham at fb.com>
>> # Date 1395699711 25200
>> #      Mon Mar 24 15:21:51 2014 -0700
>> # Node ID 498a1087dd60ec7234c4352e566abe977eb63332
>> # Parent  f85f9ea96d16e180de3e38cfc468e53441f41743
>> transaction: add support for non-append files
>>
>> This adds support for normal, non-append-only files in transactions.
>>For
>> example, .hg/store/fncache and .hg/store/phaseroots should be written
>>as part of
>> the transaction, but are not append only files.
>>
>> This adds a journal.backupfiles along side the normal journal. This
>>tracks which
>> files have been backed up as part of the transaction.
>>transaction.addbackup()
>> creates a backup of the file (using a hardlink), which is later used to
>>recover
>> in the event of the transaction failing.
>>
>> Using a seperate journal allows the repository to still be used by older
>> versions of Mercurial. A future patch will use this functionality and
>>add tests
>> for it.
>
>> +    backupfiles = []
>>       for f, o, ignore in entries:
>>           if o or not unlink:
>> -            try:
>> -                fp = opener(f, 'a')
>> -                fp.truncate(o)
>> -                fp.close()
>> -            except IOError:
>> -                report(_("failed to truncate %s\n") % f)
>> -                raise
>> +            if isinstance(o, int):
>> +                try:
>> +                    fp = opener(f, 'a')
>> +                    fp.truncate(o)
>> +                    fp.close()
>> +                except IOError:
>> +                    report(_("failed to truncate %s\n") % f)
>> +                    raise
>> +            else:
>> +                fpath = opener.join(f)
>> +                opath = opener.join(o)
>> +                util.copyfile(opath, fpath)
>> +                backupfiles.append(o)
>
>Why do you use the same list entries for two different entry.
>
>It looks like the transaction could have a fullfile entry with different
>content.
>
>(Using isinstance is usually considered fragile and inelegant)

I used the same list to minimize the amount of code churn and duplication
in this patch. It's mainly a problem because transactions have a notion of
'groups' that use this single list idea and would require a fair bit of
churn to make work with separate lists.

I'll implement your other suggestions though.



More information about the Mercurial-devel mailing list