David Anderson wrote:
> * 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.
It would still catch all commits that include conflicts that haven't been
noticed. It would only fail to catch ones that have been partly resolved
(removing the "<<<<<<<" and ">>>>>>>" but leaving the "=======") and I think
that's absolutely fine because if someone incorrectly resolves a conflict there
are unlimited ways for them to get it wrong and failing to detect this
particular way doesn't matter.
> 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.
Yes, I realised that.
>>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.
That would be one way. Other, half-baked ideas include:
* Have the user set a revision property to indicate "ignore conflict markers in
this commit", and have the script obey this and then perhaps remove the
property before allowing the commit. Problem: I don't think we've a user
interface for providing arbitrary rev-props on a commit.
* running something like this as a post-commit warning rather than a pre-commit
blocking mode.
* trying to make it remember the fact that this user tried the commit, and
allow a second attempt by the same user (if within a certain time, and/or if
with the same set of files affected, and/or ...)
>>> 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?).
It seems there's no obvious single solution, so it might be a good idea to post
here first.
- 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 13:50:24 2006