[PATCH 2 of 6] transaction: use unfiltered changelog length for transaction start

[Martin von Zweigbergk]

On Fri, May 19, 2017 at 1:05 PM, Pierre-Yves David
<pierre-yves.david at ens-lyon.org> wrote:
>
>
> On 05/18/2017 08:23 PM, Durham Goode wrote:
>>
>> # HG changeset patch
>> # User Durham Goode <durham at fb.com>
>> # Date 1495129620 25200
>> #      Thu May 18 10:47:00 2017 -0700
>> # Node ID 35235b1cefb101dad0672a4c4ee9486fba8b935b
>> # Parent  19be454ee16925d01da8f9d329cb48556083da1b
>> transaction: use unfiltered changelog length for transaction start
>>
>> Previously we used the normal changelog length for the transaction start.
>> This
>> triggered the computehidden logic which in a future patch triggers logic
>> in the
>> new hidden store which may start a transaction. This is circular and
>> results in
>> broken transaction mechanics, so let's use the unfiltered repo instead to
>> avoid
>> triggering computehidden during transaction start.
>>
>> This seems like how it should've been in the first place anyway since
>> recording
>> the length of the filtered changelog seems odd for a transaction.
>>
>> This was causing test failures in a future patch, so test coverage will
>> come
>> from the future patch.
>
>
> yes, writing filtered journal seems a bit odd. However I would rather see
> the whole _writejournal run unfiltered.
>
> There are a '@unfilteredmethod' decorator for this purpose.

That seems like a trivial little improvement for V2. There are no
other calls in the method that seem like they would be affected, so it
should be functionally equivalent as far as I can tell.

>
> To push things further. I wonder why we are not running the transaction
> logic on an unfiltered repository, but I could see potential issue with
> hooks running on an unfiltered repository.
>
> I've run the testsuit with a unfiltered transaction and everything pass.
> That might be from the lack of testing of the problematic cases.
>
>> diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
>> --- a/mercurial/localrepo.py
>> +++ b/mercurial/localrepo.py
>> @@ -1123,7 +1123,7 @@ class localrepository(object):
>>          self.vfs.write("journal.branch",
>>                            encoding.fromlocal(self.dirstate.branch()))
>>          self.vfs.write("journal.desc",
>> -                          "%d\n%s\n" % (len(self), desc))
>> +                          "%d\n%s\n" % (len(self.unfiltered()), desc))
>>          self.vfs.write("journal.bookmarks",
>>                            self.vfs.tryread("bookmarks"))
>>          self.svfs.write("journal.phaseroots",
>
>
>
> --
> Pierre-Yves David
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel