Re: svn commit: r1424469 - in /subversion/trunk/subversion: libsvn_client/repos_diff.c tests/cmdline/merge_reintegrate_tests.py tests/cmdline/merge_tests.py tests/cmdline/merge_tree_conflict_tests.py

On Fri, Dec 21, 2012 at 11:34 AM, Bert Huijben <bert_at_qqmail.nl> wrote:
> The problem is that I didn’t break real changes, but only changed the
> reporting of entry-prop changes (think: last_*) as real changes.
>
> That leaves a probable real bug somewhere else where we skip the processing
> of the real changes (somewhere below the entry prop changes) without even
> looking at them.
>
> This resolved many cases where there were no real changes (most testcases
> didn't document changes on the nodes that were reported as changes before),
> but as you confirmed also showed that we somehow relied on seeing these non
> changes.
>
> Paul, can you reinstate the the cases where there are true changes as XFail
> tests so I can start looking where we skip the real (property or text)
> changes.

Hi Bert,

Done in r1425065 for the merge and merge_reintegrate tests.

I still need to dig into merge_tree_conflict_tests.py 20 'tree
conflicts: tree missing, leaf edit' to see what exactly it's doing,
but I need to step out for a moment. I'll look at that when I return.

-- 
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
> Thanks,
>     Bert
> Sent from Windows Mail
>
> From: Julian Foad <julianfoad_at_btopenworld.com>
> Sent: December 21, 2012 5:20 PM
> To: Paul Burba <ptburba_at_gmail.com>
> CC: dev_at_subversion.apache.org
> Subject: Re: svn commit: r1424469 - in /subversion/trunk/subversion:
> libsvn_client/repos_diff.c tests/cmdline/merge_reintegrate_tests.py
> tests/cmdline/merge_tests.py tests/cmdline/merge_tree_conflict_tests.py
>
> (Dropping 'commits@'.)
>
> Paul Burba wrote:
>> On Thu, Dec 20, 2012 at 9:01 AM,  <rhuijben_at_apache.org> wrote:
>>> URL: http://svn.apache.org/viewvc?rev=1424469&view=rev
>>> Log:
>>> In the repository-repository diff handler: Don't retrieve pristine
>>> properties  when we already know that there are no differences to
>>> report on them.  By checking whether we really need the properties
>>> we avoid about 1000 ra calls over running our test suite (ra local).
>>> This also resolves many spurious merge changes that just change
>>> entry props.
>>>
>>> On top of that stop reporting entry prop only changes as a file/directory
>>> change to avoid doing unneeded work in the merge and diff handling.
>>>
>>> This fixes some issues identified when the merge code was updated to
>>> do a better obstruction detection, as originally we just skipped these
>>> non-changes there in the merge code.
>>>
>>> It is possible that this obscures some tree conflicts which were
>>> identified by entry prop changes that should have been detected by the
>>> real change down the tree (which might have been skipped).
>
> [...]
>
>> This change breaks mergetracking.  Let's walk-through just the test
>> change above (please bear with me, this is a somewhat lengthy
>> explanation, but stsp and julianf seem fine with this, so I want to be
>> completely clear).
>
> I am happy with the concept of the change as stated in the log message.  I
> haven't reviewed this change and I am not fine with the breakage.  Thanks
> for reviewing and catching this.
>
>
> - Julian
>