On 10/13/2016 11:08 AM, Stefan wrote:
> On 10/10/2016 11:39 PM, Stefan wrote:
>> On 10/10/2016 6:12 PM, Ivan Zhakov wrote:
>>> On 10 October 2016 at 17:53, Stefan <luke1410_at_posteo.de> wrote:
>>>> On 8/28/2016 11:32 PM, Bert Huijben wrote:
>>>>>> -----Original Message-----
>>>>>> From: Daniel Shahaf [mailto:d.s_at_daniel.shahaf.name]
>>>>>> Sent: zondag 28 augustus 2016 20:23
>>>>>> To: Stefan <luke1410_at_posteo.de>
>>>>>> Cc: dev_at_subversion.apache.org
>>>>>> Subject: Re: [PATCH] Fix a conflict resolution issue related to binary files (patch
>>>>>> v4)
>>>>>>
>>>>>> Stefan wrote on Sun, Aug 28, 2016 at 13:31:39 +0200:
>>>>>>> The regression test was tested against 1.9.4, 1.9.x and trunk r1743999.
>>>>>>>
>>>>>>> I also tried to run the test against 1.8.16 but there it fails (didn't
>>>>>>> investigate in detail).
>>>>>>> Trunk r1758069 caused some build issues on my machine. Therefore I
>>>>>>> couldn't validate/check the patch against the latest trunk (maybe it's
>>>>>>> just some local issue with my build machine rather than some actual
>>>>>>> problem on trunk - didn't look into that yet).
>>>>>> For future reference, you might have tried building trunk_at_HEAD after
>>>>>> locally reverting r1758069; i.e.:
>>>>>>
>>>>>> svn up
>>>>>> svn merge -c -r1758069
>>>>>> <apply patch>
>>>>>> make check
>>>>>>
>>>>>> Stefan wrote on Sun, Aug 28, 2016 at 18:33:55 +0200:
>>>>>>> Got approved by Bert.
>>>>>>>
>>>>>> (Thanks for stating so on the thread.)
>>>>>>
>>>>>>> Separated the repro test from the actual fix in order to have the
>>>>>>> possibility to selectively only backport the regression test to the 1.8
>>>>>>> branch.
>>>>>> Good call, but the fix and the "remove XFail markers" (r1758129 and
>>>>>> r1758130) should have been done in a single revision: they _are_
>>>>>> a single logical change. That would also avoid breaking 'make check'
>>>>>> (at r1758129 'make check' exits non-zero because of the XPASS).
>>>>> I do this the same way sometimes, when I want to use the separate revision for backporting... But usually I commit things close enough that nobody notices the bot results ;-)
>>>>> (While the initial XFail addition is still running, you can commit the two follow ups, and the buildbots collapses all the changes to a single build)
>>>>>
>>>>> I just committed the followup patch posted in another thread to unbreak the bots for the night...
>>>>>
>>>>> Bert
>>>> Attached is a patch which should resolve the test case you added, Bert.
>>>> Anybody feels like approving it? Or is there something I should
>>>> improve/change?
>>>>
>>>> [[[
>>>>
>>>> Add support for the svn_client_conflict_option_working_text resolution for
>>>> binary file conflicts.
>>>>
>>>> * subversion/libsvn_client/conflicts.c
>>>> (): Add svn_client_conflict_option_working_text to binary_conflict_options
>>>>
>>>> * subversion/tests/cmdline/resolve_tests.py
>>>> (automatic_binary_conflict_resolution): Remove XFail marker
>>>>
>>>> ]]]
>>>>
>>> It seems this patch breaks interactive conflict resolve:
>>> With trunk I get the following to 'svn resolve' on binary file:
>>> [[[
>>> Merge conflict discovered in binary file 'A_COPY\theta'.
>>> Select: (p) postpone,
>>> (r) accept binary file as it appears in the working copy,
>>> (tf) accept incoming version of binary file: h
>>>
>>> (p) - skip this conflict and leave it unresolved [postpone]
>>> (tf) - accept incoming version of binary file [theirs-full]
>>> (r) - accept binary file as it appears in the working copy [working]
>>> (q) - postpone all remaining conflicts
>>> ]]]
>>>
>>> But with patch I get the following:
>>> [[[
>>> Merge conflict discovered in binary file 'A_COPY\theta'.
>>> Select: (p) postpone,
>>> (r) accept binary file as it appears in the working copy,
>>> (tf) accept incoming version of binary file: h
>>>
>>> (p) - skip this conflict and leave it unresolved [postpone]
>>> (tf) - accept incoming version of binary file [theirs-full]
>>> (mf) - accept binary file as it appears in the working copy [mine-full]
>>> (r) - accept binary file as it appears in the working copy [working]
>>> (q) - postpone all remaining conflicts
>>> ]]]
>>>
>>> I think it's confusing and we should not offer the same option twice.
>>>
>> Completely agreed. The display of the option in the UI shouldn't be like
>> that. Certainly an oversight on my side. Will revise the patch and come
>> up with a different/better approach tomorrow.
>>
>> Regards,
>> Stefan
> Trying to put together a revised patch without the issue. The attached
> patch fixes the 4647 test but breaks two other tests:
>
> basic#41
> update#38
>
> Still looking into what I'm doing wrong, but any pointers would be much
> appreciated.
Looks like update#38 is actually fixed. Leaving basic#41 broken:
FAIL: basic_tests.py 41: automatic conflict resolution
Attached is the full test output.
Regards,
Stefan
Received on 2016-10-13 11:38:51 CEST