GNU bug report logs - #28056
Substitution doing global when it should not.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 28056 in the body.
You can then email your comments to 28056 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Techwolf Lupindo <techwolf.lupindo <at> gmail.com>
To: bug-sed <at> gnu.org
Subject: Substitution doing global when it should not.
Date: Fri, 11 Aug 2017 16:04:03 -0400

[Message part 1 (text/plain, inline)]

techwolf <at> laptop ~/test $ sed --version
sed (GNU sed) 4.4
Copyright (C) 2017 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html
>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Jay Fenlason, Tom Lord, Ken Pizzini,
and Paolo Bonzini.
GNU sed home page: <http://www.gnu.org/software/sed/>.
General help using GNU software: <http://www.gnu.org/gethelp/>.
E-mail bug reports to: <bug-sed <at> gnu.org>.

techwolf <at> laptop ~/test $ wget "
https://hg.phoenixviewer.com/phoenix-firestorm-lgpl/raw-file/9d58c58cca90/indra/cmake/BuildVersion.cmake
"

<snipped>

2017-08-11 15:39:45 (38.2 MB/s) - ‘BuildVersion.cmake’ saved [3605/3605]

techwolf <at> laptop ~/test $ sed -e 's:COMMAND ${MERCURIAL}:COMMAND
${MERCURIAL} --cwd ${CMAKE_SOURCE_DIR}:' BuildVersion.cmake >
BuildVersion1.cmake
techwolf <at> laptop ~/test $ sed -e 's:COMMAND ${MERCURIAL}:COMMAND
${MERCURIAL} --cwd ${CMAKE_SOURCE_DIR}:g' BuildVersion.cmake >
BuildVersion2.cmake
techwolf <at> laptop ~/test $ diff -u BuildVersion1.cmake BuildVersion2.cmake
techwolf <at> laptop ~/test $ diff -u BuildVersion.cmake BuildVersion1.cmake
--- BuildVersion.cmake  2017-08-11 15:39:45.639970357 -0400
+++ BuildVersion1.cmake 2017-08-11 15:40:47.646763237 -0400
@@ -25,10 +25,10 @@
                  #         building an earlier revision. Instead, we use
                  #         "hg identify -n" to get the local revision
number
                  #         of the actual state of the repository.
-                 #COMMAND ${MERCURIAL} log -r tip:0 --template '\\n'
+                 #COMMAND ${MERCURIAL} --cwd ${CMAKE_SOURCE_DIR} log -r
tip:0 --template '\\n'
                  #COMMAND ${WORDCOUNT} -l
                  #COMMAND ${SED} "s/ //g"
-                 COMMAND ${MERCURIAL} identify -n
+                 COMMAND ${MERCURIAL} --cwd ${CMAKE_SOURCE_DIR} identify -n
                  COMMAND ${SED} "s/+//"        # [CR] Strip off any + from
the revision number
                  OUTPUT_VARIABLE VIEWER_VERSION_REVISION
                  OUTPUT_STRIP_TRAILING_WHITESPACE

The only difference in the sed commands was s/// and s///g. The diff
between BuildVersion1.cmake and BuildVersion2.cmake should had one change.
But sed is matching two lines without the s///g. All docs I have read says
that without 'g', only the first match is changed. However, this is not in
my case and was not notice for over two years due to my build script not
breaking. I only notice this bug when I was doing some code cleanup and
scratching my head as to why my build script was not breaking when in
theory, it should had an error printed when hg could not see the repo.

[Message part 2 (text/html, inline)]

Message #12 received at 28056-done <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Techwolf Lupindo <techwolf.lupindo <at> gmail.com>, 28056-done <at> debbugs.gnu.org
Subject: Re: bug#28056: Substitution doing global when it should not.
Date: Fri, 11 Aug 2017 17:39:27 -0500

[Message part 1 (text/plain, inline)]

tag 28056 notabug
thanks

On 08/11/2017 03:04 PM, Techwolf Lupindo wrote:

> techwolf <at> laptop ~/test $ sed -e 's:COMMAND ${MERCURIAL}:COMMAND
> ${MERCURIAL} --cwd ${CMAKE_SOURCE_DIR}:' BuildVersion.cmake >
> BuildVersion1.cmake
> techwolf <at> laptop ~/test $ sed -e 's:COMMAND ${MERCURIAL}:COMMAND
> ${MERCURIAL} --cwd ${CMAKE_SOURCE_DIR}:g' BuildVersion.cmake >
> BuildVersion2.cmake


> 
> The only difference in the sed commands was s/// and s///g.

Which tells sed whether to make one substitution on every affected line,
or as many substitutions as possible on every affected line.  But the
number of affected lines remains unchanged for the two variants.

> The diff
> between BuildVersion1.cmake and BuildVersion2.cmake should had one change.

Rather, one change per affected line.

> But sed is matching two lines without the s///g. All docs I have read says
> that without 'g', only the first match is changed.

Only the first match per line - but when both lines affected only had
one match per line, there's no difference in using the 'g' flag.

If you want sed to stop processing as soon as it has changed one line,
rather than going on to look for additional affected lines, you can do so:

sed -e '/COMMAND ${MERCURIAL}/ { s::COMMAND ${MERCURIAL} -- cwd
${CMAKE_SOURCE_DIR}: ; q }'

which says find the first line that matches the pattern, and on that
line make the substitution and immediately quit.

Since sed is behaving as documented, I'm closing this as not a bug;
however, feel free to add further comments to this thread if you need
followup clarification.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Message #15 received at 28056-done <at> debbugs.gnu.org (full text, mbox):

From: Techwolf Lupindo <techwolf.lupindo <at> gmail.com>
To: Eric Blake <eblake <at> redhat.com>
Cc: 28056-done <at> debbugs.gnu.org
Subject: Re: bug#28056: Substitution doing global when it should not.
Date: Fri, 11 Aug 2017 22:19:46 -0400

[Message part 1 (text/plain, inline)]

The docs say:
g
Apply the replacement to all matches to the regexp, not just the first.

Nothing in the docs say it will matches all lines in the input, wither
piped or file inplace. Doc bug?

On Fri, Aug 11, 2017 at 6:39 PM, Eric Blake <eblake <at> redhat.com> wrote:

> tag 28056 notabug
> thanks
>
> On 08/11/2017 03:04 PM, Techwolf Lupindo wrote:
>
> > techwolf <at> laptop ~/test $ sed -e 's:COMMAND ${MERCURIAL}:COMMAND
> > ${MERCURIAL} --cwd ${CMAKE_SOURCE_DIR}:' BuildVersion.cmake >
> > BuildVersion1.cmake
> > techwolf <at> laptop ~/test $ sed -e 's:COMMAND ${MERCURIAL}:COMMAND
> > ${MERCURIAL} --cwd ${CMAKE_SOURCE_DIR}:g' BuildVersion.cmake >
> > BuildVersion2.cmake
>
>
> >
> > The only difference in the sed commands was s/// and s///g.
>
> Which tells sed whether to make one substitution on every affected line,
> or as many substitutions as possible on every affected line.  But the
> number of affected lines remains unchanged for the two variants.
>
> > The diff
> > between BuildVersion1.cmake and BuildVersion2.cmake should had one
> change.
>
> Rather, one change per affected line.
>
> > But sed is matching two lines without the s///g. All docs I have read
> says
> > that without 'g', only the first match is changed.
>
> Only the first match per line - but when both lines affected only had
> one match per line, there's no difference in using the 'g' flag.
>
> If you want sed to stop processing as soon as it has changed one line,
> rather than going on to look for additional affected lines, you can do so:
>
> sed -e '/COMMAND ${MERCURIAL}/ { s::COMMAND ${MERCURIAL} -- cwd
> ${CMAKE_SOURCE_DIR}: ; q }'
>
> which says find the first line that matches the pattern, and on that
> line make the substitution and immediately quit.
>
> Since sed is behaving as documented, I'm closing this as not a bug;
> however, feel free to add further comments to this thread if you need
> followup clarification.
>
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
>
>

[Message part 2 (text/html, inline)]

Message #18 received at 28056-done <at> debbugs.gnu.org (full text, mbox):

From: Techwolf Lupindo <techwolf.lupindo <at> gmail.com>
To: Eric Blake <eblake <at> redhat.com>
Cc: 28056-done <at> debbugs.gnu.org
Subject: Re: bug#28056: Substitution doing global when it should not.
Date: Sat, 12 Aug 2017 14:02:38 -0400

[Message part 1 (text/plain, inline)]

A followup. I tried the command you hinted at. The result was the first
match and substitution was done, but the rest of the file was deleted. This
is using sed -i -e.

On Fri, Aug 11, 2017 at 10:19 PM, Techwolf Lupindo <
techwolf.lupindo <at> gmail.com> wrote:

> The docs say:
> g
> Apply the replacement to all matches to the regexp, not just the first.
>
> Nothing in the docs say it will matches all lines in the input, wither
> piped or file inplace. Doc bug?
>
> On Fri, Aug 11, 2017 at 6:39 PM, Eric Blake <eblake <at> redhat.com> wrote:
>
>> tag 28056 notabug
>> thanks
>>
>> On 08/11/2017 03:04 PM, Techwolf Lupindo wrote:
>>
>> > techwolf <at> laptop ~/test $ sed -e 's:COMMAND ${MERCURIAL}:COMMAND
>> > ${MERCURIAL} --cwd ${CMAKE_SOURCE_DIR}:' BuildVersion.cmake >
>> > BuildVersion1.cmake
>> > techwolf <at> laptop ~/test $ sed -e 's:COMMAND ${MERCURIAL}:COMMAND
>> > ${MERCURIAL} --cwd ${CMAKE_SOURCE_DIR}:g' BuildVersion.cmake >
>> > BuildVersion2.cmake
>>
>>
>> >
>> > The only difference in the sed commands was s/// and s///g.
>>
>> Which tells sed whether to make one substitution on every affected line,
>> or as many substitutions as possible on every affected line.  But the
>> number of affected lines remains unchanged for the two variants.
>>
>> > The diff
>> > between BuildVersion1.cmake and BuildVersion2.cmake should had one
>> change.
>>
>> Rather, one change per affected line.
>>
>> > But sed is matching two lines without the s///g. All docs I have read
>> says
>> > that without 'g', only the first match is changed.
>>
>> Only the first match per line - but when both lines affected only had
>> one match per line, there's no difference in using the 'g' flag.
>>
>> If you want sed to stop processing as soon as it has changed one line,
>> rather than going on to look for additional affected lines, you can do so:
>>
>> sed -e '/COMMAND ${MERCURIAL}/ { s::COMMAND ${MERCURIAL} -- cwd
>> ${CMAKE_SOURCE_DIR}: ; q }'
>>
>> which says find the first line that matches the pattern, and on that
>> line make the substitution and immediately quit.
>>
>> Since sed is behaving as documented, I'm closing this as not a bug;
>> however, feel free to add further comments to this thread if you need
>> followup clarification.
>>
>> --
>> Eric Blake, Principal Software Engineer
>> Red Hat, Inc.           +1-919-301-3266
>> Virtualization:  qemu.org | libvirt.org
>>
>>
>

[Message part 2 (text/html, inline)]

Message #21 received at 28056-done <at> debbugs.gnu.org (full text, mbox):

From: Assaf Gordon <assafgordon <at> gmail.com>
To: Techwolf Lupindo <techwolf.lupindo <at> gmail.com>,
 Eric Blake <eblake <at> redhat.com>
Cc: 28056-done <at> debbugs.gnu.org
Subject: Re: bug#28056: Substitution doing global when it should not.
Date: Sat, 12 Aug 2017 19:51:57 -0600

Hello,

On 12/08/17 12:02 PM, Techwolf Lupindo wrote:
> A followup. I tried the command you hinted at. The result was the first
> match and substitution was done, but the rest of the file was deleted. This
> is using sed -i -e.

If you want to operate on just the first matching line, you can use
the following sed method:

  printf "%s\n" a b c d a b c d | sed '/b/ { s/b/X/ ; :Y ; n ; bY }'

The sed program does the following:

 /b/  - match *any* line that contains 'b'

 { ... } - sed commands grouped together, will be executed
          when the current line matches 'b'

 s/b/X/ - replace the first occurrence of 'b' with 'X' .

 :Y     - a label called 'Y' (arbitrary name)

 n      - read the next line and print it

 bY     - jump (=branch) to label Y


Effectively, the first time a line contains 'b'
matches the regex, sed will perform one replacement,
and then enter an endless loop of reading/printing the lines,
without executing any other sed commands (and thus,
not matching other 'b' lines and not performing replacements).

Expanding Eric's suggestion, your sed command might look like:

    sed -e '/COMMAND ${MERCURIAL}/ { s::COMMAND ${MERCURIAL} -- cwd
${CMAKE_SOURCE_DIR}: ; :Y ; n; bY }'


Also,
When experimenting with sed commands, I'd recommend not using "-i"
(in-place replacements) - this could lead to data loss if your sed
command isn't working as expected, and you don't have a backup.


Lastly,
For quick-and-dirty manual sed replacements of single lines, you can
find the exact line number and replace just it.
Example:

  $ wget https://hg....file/9d58c58cca90/indra/cmake/BuildVersion.cmake

  $ grep -n 'COMMAND ${MERCURIAL}' BuildVersion.cmake
  28:         #COMMAND ${MERCURIAL} log -r tip:0 --template '\\n'
  31:         COMMAND ${MERCURIAL} identify -n

And then assuming you want to change only line 28, use a specific
and even shorter sed command:

  sed '28s/}/} --cwd ${CMAKE_SOURCE_DIR} /' BuildVersion.cmake

The '28s/.../.../' tells sed to perform the 's' command only on line 28.

Hope this helps,

regards,
 - assaf

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.