* Julian Foad <julianfoad@btopenworld.com> [2006-03-29 05:11:41]:
> 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.
Maybe, but does that catch enough cases? The point was to catch all
suspicious commits, not a subset of them.
Also, note that the script only catches adds of suspicious lines, so
working inside a README, or inside a test script that has explicit
conflict markers embedded is no problem. Once a commit has been
accepted, content is ignored by the script. However, touching section
headers, or the part of the script that contains the markers, is
indeed a problem.
> Both of these points would be less important if there were a way to
> over-ride the hook's decision.
I was thinking of including such a mechanism, using a marker given in
the commit log message. I didn't include any because I'm not sure
what format this marker could have to be as unintrusive as possible,
but adding such a check is rather trivial. If a commit log marker
seems satisfactory, I'll commit an update to the script to do that.
> > 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.
Right. The first iteration of this script actually took the output of
svnlook changed, and then scanned each file's diff individually, so
that it could say "Something wrong in foo.c:493". The downside is that
it makes the script a little more complex (though not much when it
comes down to it).
If you have no further objections wrt. my reply, I'll commit an update
to the script, to include bypassing capability as well as a clarified
and verbose error (or submit it as a patch here for review, if you
prefer?).
- Dave.
---------------------------------------------------------------------
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:50:19 2006