Hi Julian.
On 11.04.2013 05:10, Julian Foad wrote:
> Hi Matthias. Thanks for your email, and sorry it was so long before I looked back in my mailbox and replied.
>
> Matthias Buecher wrote on 2013-02-02:
>
>> the contrib script "detect-merge-conflicts.sh" [1] uses a grep command
>> which also finds false positive merge conflict markers: it finds single lines of
>> "=======" and the pre-commit will fail.
>>
>> For example I wanted to add a readme file that contains the following two lines:
>> Install
>> =======
>>
>> Of course committing failed.
>
> Yes, failing the commit just because there is a ======= line is clearly too strong. The beginning and end lines are more distinctive and unusual, starting with seven angle brackets and then a space and then a dot, so we could just look for them.
>
>> The correct solution would be to use sed and search for real blocks of merge
>> conflict marker:
>> SUSPICIOUS=$($SVNLOOK diff -t "$TXN" "$REPOS" | sed -n -e
>> '/^+<<<<<<< \..*$/,/^+>>>>>>>
>> \..*$/ { /^+=======$/p ; //q }' | wc -l)
>>
>> This sed command finds blocks enclosed with new "<<<<<<
>> ." and ">>>>>>>." and checks if this block
>> contains a new line with "=======". If found it prints out that line
>> and quits sed.
>
> Well, that is a possible solution, unless the admin wants to detect a beginning marker even when the user has edited out the end marker. I was a bit concerned that the 'sed' syntax might not be very portable (I'm not sure), so I was about to commit this but also leaving the old 'grep' command as a commented-out example as well. Then I realized that the attempt to look for a block starting with the '<<<<<<<' line and ending with the '>>>>>>>' line doesn't actually ensure that the beginning and end markers are within the same file, as it scans the whole output of 'svnlook diff' in one pass which might include changes to hundreds of files.
There had been several follow-up mails due to sed compatibility and the result was:
SUSPICIOUS=$("${SVNLOOK}" diff -t "${TXN}" "${REPOS}" | "${SED}" -n -e '/^+<<<<<<< \..*$/,/^+>>>>>>> \..*$/ { /^+=======$/ { p ; q ;
}
}' | wc -l)
Tested on Debian, Redhat, FreeBSD and NetBSD.
All distributions support semicolons and the closing brackets have to be on separate lines as per sed manual.
I see the issue with multiple files in a single diff.
> In the end, I decided to make the minimum change so as to minimize the risk of being asked to fix fallout such as portability issues or unwanted behaviour changes. So I just removed the "=======" detection from the existing 'grep' command and added a comment explaining why. Committed revision 1466758. I have no objection to us making further changes if you feel the need.
It's ok for me, as everbody is able to adopt the script to his needs.
> Thanks for the suggestion and the patch.
>
> - Julian
>
>
>> Kind regards
>> Matthias "Maddes" Bücher
>>
>> P.S.:
>> I'm not subscribed and would appreciate being explicitly Cc:ed in any
>> responses. Thanks.
>>
>> [1]
>> http://svn.apache.org/repos/asf/subversion/trunk/contrib/hook-scripts/detect-merge-conflicts.sh
>>
>
Received on 2013-04-12 17:43:41 CEST