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 1:10 PM, Paul Burba <ptburba_at_gmail.com> wrote:
> 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.

Hey Bert - I switched the these two tests back to their old
expectations too in r1425157. Even though no merge tracking is going
on, we are now only reporting a path as skipped if it is the root of
the missing subtree (missing due to OS-level deletion). The paths you
removed from the skip notifications *do* have operative changes
incoming.

From your private email it sounds like you'll be tackling this in the
short term. If not we should file an issue and associate these
various tests with it so this regression doesn't slip through the
cracks.

Thanks,

-- 
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba
> --
> 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
>>