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.

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.

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
>

Reply via email to