Re: Log caching: Crash in `Get merge logs' without access to the repository root

On 11 March 2016 at 23:54, Stefan Küng <[email protected]> wrote:
> On Fri, Mar 11, 2016 at 6:40 PM, Stefan Kueng <[email protected]> wrote:
>
>>
>> Patch looks good!
>> Please commit.
>>
>> But while I was going through the code and already made some similar
>> changes as in your patch I got an idea:
>>
>> maybe we could remove the separate handling of logs with/without merge
>> info completely and just use one code path for all situations?
>>
>> This would mean getting rid of the call to InternalLogWithMerge() and call
>> InternalLog() instead. Then we could also remove the CMergeLogger() class.
>>
>> I'm doing some testing with that change (not finished yet) soon.
>> But in the mean time: what do you think?
>>
>>
>
> Actually, there's more needed to make it work properly. Your patch fixes the
> crash, that's good.
> But if there are actually merged revisions shown/fetched, then the skip
> range gets messed up.
My bad, I missed that.

> We have to 'ignore' all the merged revs that come in and not use them for
> book keeping. We only have to update the cache and report those revisions
> back to the caller, but *not* use them for e.g. updating the skip ranges.
> If we do, then instead of one log call we get at least one more, maybe even
> two.
>
> I have something working so far, but before I commit I'd like to do some
> more testing.
> I'll commit my change tomorrow if I don't find any other problems...
>
There are cases when that wouldn't work, since the merge-aware log is
very different. For example 'svn log -g -c 27193
https://svn.code.sf.net/p/tortoisesvn/code/branches/1.9.x' reports the
following revisions:
[[[
r27193 | steveking | 2016-02-22 21:25:45
------------------------------------------------------------------------
r27179 | ivan-z | 2016-02-22 13:09:20
Merged via: r27193
------------------------------------------------------------------------
r27177 | ivan-z | 2016-02-19 18:30:56
Merged via: r27193
------------------------------------------------------------------------
r27176 | ivan-z | 2016-02-19 18:28:26
Merged via: r27193
------------------------------------------------------------------------
r27175 | ivan-z | 2016-02-19 18:27:41
Merged via: r27193
------------------------------------------------------------------------
r27169 | steveking | 2016-02-10 22:22:11
Merged via: r27193
------------------------------------------------------------------------
r27168 | steveking | 2016-02-10 22:20:50
Merged via: r27193
------------------------------------------------------------------------
r27167 | steveking | 2016-02-08 23:27:58
Merged via: r27193
]]]

But log caching doesn't have information about merges and would report
just r27193 if it's cached. To reproduce with TortoiseSVN trunk@27240:
1. Enable Log Caching
2. Start TortoiseSVN log for
https://svn.code.sf.net/p/tortoisesvn/code/branches/1.9.x:
    TortoiseProc.exe /command:log
/path:https://svn.code.sf.net/p/tortoisesvn/code/branches/1.9.x
/startrev:27193 /endrev:27193
3. Click "Include merged revisions"
   Merged revisions will not be displayed, while they should. They may
appear after pressing F5, but they will not be reported as merged.

I think the best solution would be just to skip log caching completely
for merge-aware logs. I'm going to prepare patch for this.

-- 
Ivan Zhakov

------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=3165791

To unsubscribe from this discussion, e-mail: [[email protected]].