You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2013/02/20 15:47:06 UTC
Re: [bug] detect-merge-conflicts.sh reports false positive merge
conflict markers
Thanks Gavin. Matthias: how portable are the sed constructs you used?
They seem to be invalid on BSD sed:
% printf 'foo\nbar\nbaz\n' | sed -ne '/foo/,/baz/ { /bar/p }'
sed: 1: "/foo/,/baz/ { /bar/p }
": extra characters at the end of p command
I don't mind improving the detection but I'd like to keep the script
portable (and the check cheap, if possible, since 'diff' output can be
large).
Gavin Baumanis wrote on Wed, Feb 20, 2013 at 09:10:48 -0500:
> Ping.
> This Patch submission has received no comments.
>
>
>
> > -----Original Message-----
> > From: Matthias Buecher / Germany [mailto:mail@maddes.net]
> > Sent: Saturday, 2 February 2013 15:20
> > To: dev@subversion.apache.org
> > Subject: [bug] detect-merge-conflicts.sh reports false positive merge conflict
> > markers
> >
> > Hello,
> >
> > 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.
> >
> > 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.
> >
> > 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
>
Re: Re: [bug] detect-merge-conflicts.sh reports false positive merge
conflict markers
Posted by Matthias Buecher / Germany <ma...@maddes.net>.
Alan, thanks for the info.
I have re-tested on NetBSD 5.0, Debian 6.0, Redhat on Sourceforge, FreeBSD 9 and also on OpenWrt/Busybox 1.19.4.
All sed versions support semicolon.
NetBSD needs newlines after closing brackets.
Busybox needs nested brackets for "p" and "q" to work correctly (also makes the logic more visible).
Result is the following test script:
#!/bin/sh
printf 'abc\nfoo\nbar1\nbaz\nklj\nfoo\nbar2\nbaz\nxyz\n' | sed -ne '/foo/,/baz/ { /bar/ { p ; q ;
}
}'
Therefore the corrected line for detect-merge-conflicts.sh is:
SUSPICIOUS=$($SVNLOOK diff -t "$TXN" "$REPOS" | sed -n -e '/^+<<<<<<< \..*$/,/^+>>>>>>> \..*$/ { /^+=======$/ { p ; q ;
}
}' | wc -l)
I hope that is the final portable answer :)
Maddes
P.S.:
I'm not subscribed and would appreciate being explicitly Cc:ed in any responses. Thanks.
On 21.02.2013 08:34, Alan Barrett wrote:
> On Wed, 20 Feb 2013, Matthias Buecher / Germany wrote:
>> I tested on Debian 6.0 Squeeze and on Sourceforge Redhat server. Just
>> installed FreeBSD 9.0 in a VM and sed on FreeBSD needs a semicolon to
>> separate last command and closing bracket.
>
> Semicolon might often work, but the portable syntax requires a newline.
>
> See
> <http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html>,
> which includes:
>
> The <right-brace> shall be preceded by a <newline> and can be
> preceded or followed by <blank> characters.
>
> and:
>
> Historically, the sed ! and } editing commands did not permit
> multiple commands on a single line using a <semicolon> as a
> command delimiter. Implementations are permitted, but not
> required, to support this extension.
>
> --apb (Alan Barrett)
Re: [bug] detect-merge-conflicts.sh reports false positive merge
conflict markers
Posted by Alan Barrett <ap...@cequrux.com>.
On Wed, 20 Feb 2013, Matthias Buecher / Germany wrote:
> I tested on Debian 6.0 Squeeze and on Sourceforge Redhat server.
> Just installed FreeBSD 9.0 in a VM and sed on FreeBSD needs a
> semicolon to separate last command and closing bracket.
Semicolon might often work, but the portable syntax requires a newline.
See
<http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html>,
which includes:
The <right-brace> shall be preceded by a <newline> and can be
preceded or followed by <blank> characters.
and:
Historically, the sed ! and } editing commands did not permit
multiple commands on a single line using a <semicolon> as a
command delimiter. Implementations are permitted, but not
required, to support this extension.
--apb (Alan Barrett)
Re: [bug] detect-merge-conflicts.sh reports false positive merge
conflict markers
Posted by Matthias Buecher / Germany <ma...@maddes.net>.
I tested on Debian 6.0 Squeeze and on Sourceforge Redhat server.
Just installed FreeBSD 9.0 in a VM and sed on FreeBSD needs a semicolon to separate last command and closing bracket.
The used sed functions are just the most basic sed and regexe functions.
Corrected line:
SUSPICIOUS=$($SVNLOOK diff -t "$TXN" "$REPOS" | sed -n -e '/^+<<<<<<< \..*$/,/^+>>>>>>> \..*$/ { /^+=======$/p ; //q ; }' | wc -l)
Also re-tested successfully on Debian 6.0 Squeeze and Sourceforge Redhat server.
Sorry for the inconvenience
Maddes
I'm not subscribed and would appreciate being explicitly Cc:ed in any responses. Thanks.
On 20.02.2013 15:47, Daniel Shahaf wrote:
> Thanks Gavin. Matthias: how portable are the sed constructs you used?
>
> They seem to be invalid on BSD sed:
> % printf 'foo\nbar\nbaz\n' | sed -ne '/foo/,/baz/ { /bar/p }'
> sed: 1: "/foo/,/baz/ { /bar/p }
> ": extra characters at the end of p command
>
> I don't mind improving the detection but I'd like to keep the script
> portable (and the check cheap, if possible, since 'diff' output can be
> large).
>
> Gavin Baumanis wrote on Wed, Feb 20, 2013 at 09:10:48 -0500:
>> Ping.
>> This Patch submission has received no comments.
>>
>>
>>
>> > -----Original Message-----
>> > From: Matthias Buecher / Germany [mailto:mail@maddes.net]
>> > Sent: Saturday, 2 February 2013 15:20
>> > To: dev@subversion.apache.org
>> > Subject: [bug] detect-merge-conflicts.sh reports false positive merge conflict
>> > markers
>> >
>> > Hello,
>> >
>> > 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.
>> >
>> > 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.
>> >
>> > 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
>>
>
Matthias "Maddes" Bücher
--
http://www.maddes.net/
Home: Earth / Germany / Ruhr-Area
Re: [bug] detect-merge-conflicts.sh reports false positive merge
conflict markers
Posted by Alan Barrett <ap...@cequrux.com>.
On Wed, 20 Feb 2013, Daniel Shahaf wrote:
>They seem to be invalid on BSD sed:
>% printf 'foo\nbar\nbaz\n' | sed -ne '/foo/,/baz/ { /bar/p }'
>sed: 1: "/foo/,/baz/ { /bar/p }
>": extra characters at the end of p command
The close brace must be on a new line. This works on NetBSD:
$ printf 'foo\nbar\nbaz\n' | sed -ne '/foo/,/baz/ { /bar/p
}'
--apb (Alan Barrett)