[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r19082 - trunk/contrib/hook-scripts

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2006-03-29 06:11:41 CEST

Justin Erenkrantz wrote:
> On 3/28/06, danderson@tigris.org <danderson@tigris.org> wrote:
>
>>Author: danderson
>>Date: Tue Mar 28 18:27:10 2006
>>New Revision: 19082
>>
>>Added:
>> trunk/contrib/hook-scripts/detect-merge-conflicts.sh (contents, props changed)
>>
>>Log:
>>Add a contrib pre-commit hook script that aborts the commit if it
>>finds things that look like merge conflict markers in the commit diff.
>
> My concern with this pre-commit hook is that it'd not allow itself to
> be committed. =P

It would, since it doesn't contain a line starting with seven conflict mark
characters. But...

It won't allow many of our README and similar text files to be committed, and
they don't contain context markers but just arbitrary-length lines of '='
characters. This hook script ought to be more strict in what it considers to
be a conflict marker: after the seven marks should come a space (for '<' and
'>') or EOL (for '='). (I've just checked and that is true for standard
"diff3" output as well as "svn" output.) That should significantly decrease
the false-reject rate, but a line consisting of seven '=' characters will still
occur reasonably often (e.g. in our contrib/client-side/svncopy.README), so
perhaps just looking for the '<' and '>' types of lines would be best.

It won't allow some of our test files (e.g.
subversion/tests/cmdline/trans_tests.py) which intentionally contain real
conflict markers. I don't see any practical way to avoid this. (Obviously,
tests could be re-written to not include real conflict markers directly, but
that's not a general solution.) It shouldn't be a real possibility in most
people's repositories, only for people who are developing tools that deal with
"patch" and "merge" behaviour, and those people (including us) can avoid using
this script.

Both of these points would be less important if there were a way to over-ride
the hook's decision.

> echo "Some parts of your commit look suspiciously like merge" >&2
> echo "conflict markers. Please double-check your diff and try" >&2
> echo "committing again." >&2
> exit 1

I think this message would be better if it made clear that simply checking and
trying again won't get you any further: you need to remove the changes that
look like conflict markers. In order to help the user in awkward situations,
and remembering that the user will usually not have access to the script's
source code to discover exactly what it dislikes, it would be worth this error
message being a bit more specific about what it found and/or where it found it,
e.g. printing the first line that it encountered that it didn't like.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Mar 29 06:12:07 2006

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.