Portable use of "{", ";", etc. within sed scripts

[Paul Eggert]

Ralf Wildenhues <address@hidden> writes:

> +verbs.  On the other hand, commands after @address@hidden should be preceded
> +by @samp{;} unless preceded by a newline.

That's not strict enough, I'm afraid.  The POSIX rule is that you
cannot use semicolons inside { } at all, in a sed script.  Nor can you
use addresses like 1 or $ or /x/ inside { }; nor can you use !; and
there are some other limits.

Automake uses '{' within sed commands, so it should be swept for these
problems, too.  I'll follow up on bug-automake.

I have added documentation for this in the Autoconf manual, as
described below.  I also swept Autoconf for instances of this problem
and fixed all the ones that I found.

Here's the patch I installed.

2006-01-05  Paul Eggert  <address@hidden>

        Fix Posix-conformance bugs re use of { command in sed scripts,
        and improve the sed-related documentation a bit.
        * doc/autoconf.texi (Installation Directory Variables): Use
        our own style advice re 's,a,b,' versus 's|a|b|'.  Use "Sed"
        rather than "sed" when talking about Sed in general.
        (Particular Programs): Likewise.
        (Coding Style): y is like s with respect to / and ,.
        (Limitations of Usual Tools): Document the weird restrictions
        that Posix has about { }.  Use better quoting.
        * lib/autoconf/status.m4 (_AC_OUTPUT_FILES_PREPARE, _AC_OUTPUT_HEADER):
        Rewrite to conform to Posix rules about { } in sed scripts.
        * lib/m4sugar/m4sh.m4 (AS_DIRNAME_SED, AS_BASENAME_SED): Likewise.
        * tests/foreign.at (Libtool): Likewise.
        * tests/semantics.at (AC_CHECK_PROG & AC_CHECK_PROGS):
        Use our own style advice re 's,a,b,' versus 's|a|b|'.

Index: doc/autoconf.texi
===================================================================
RCS file: /cvsroot/autoconf/autoconf/doc/autoconf.texi,v
retrieving revision 1.939
diff -p -u -r1.939 autoconf.texi
--- doc/autoconf.texi   4 Jan 2006 22:28:37 -0000       1.939
+++ doc/autoconf.texi   5 Jan 2006 21:42:53 -0000
@@ -2458,8 +2458,8 @@ Makefile snippet similar to:
 @example
 @group
 edit = sed \
-        -e 's|@@datadir[@@]|$(pkgdatadir)|g' \
-        -e 's|@@prefix[@@]|$(prefix)|g'
+        -e 's,@@datadir[@@],$(pkgdatadir),g' \
+        -e 's,@@prefix[@@],$(prefix),g'
 @end group
 
 @group
@@ -2484,7 +2484,7 @@ Some details are noteworthy:
 @table @samp
 @item @@datadir[@@]
 The brackets prevent @command{configure} from replacing
address@hidden@@datadir@@} in the sed expression itself.
address@hidden@@datadir@@} in the Sed expression itself.
 Brackets are preferable to a backslash here, since
 Posix says @samp{\@@} is not portable.
 
@@ -2493,7 +2493,7 @@ Don't use @samp{@@pkgdatadir@@}!  Use th
 instead.
 
 @item ,
-Don't use @samp{/} in the sed expression(s) since most likely the
+Don't use @samp{/} in the Sed expression(s) since most likely the
 variables you use, such as @samp{$(pkgdatadir)}, will contain
 some.
 
@@ -3509,7 +3509,7 @@ is found, and otherwise to @samp{:} (do 
 @defmac AC_PROG_SED
 @acindex{PROG_SED}
 @ovindex SED
-Set output variable @code{SED} to a @code{sed} on @env{PATH} that
+Set output variable @code{SED} to a Sed implementation on @env{PATH} that
 truncates as few characters as possible.  If @sc{gnu} Sed is found,
 use that instead.
 @end defmac
@@ -9619,9 +9619,9 @@ etc.  If you add portability issues to t
 better than hacking Autoconf @code{:-)}.
 
 When using @command{sed}, don't use @option{-e} except for indenting
-purpose.  With the @code{s} command, the preferred separator is @samp{/}
-unless @samp{/} itself is used in the command, in which case you should
-use @samp{,}.
+purposes.  With the @code{s} and @code{y} commands, the preferred
+separator is @samp{/} unless @samp{/} itself might appear in the pattern
+or replacement, in which case you should use @samp{,}.
 
 @xref{Macro Definitions}, for details on how to define a macro.  If a
 macro doesn't use @code{AC_REQUIRE} and it is expected to never be the
@@ -11397,7 +11397,7 @@ shift, but in addition it is not portabl
 RISC/OS} 4.52 refuses to do it.
 
 Don't use @samp{shift 2} etc.; it was not in the 7th Edition Bourne shell,
-and it is also absent in many pre-POSIX shells.
+and it is also absent in many pre-Posix shells.
 
 
 @item @command{source}
@@ -12373,6 +12373,12 @@ but Posix says that this use of a semico
 should use semicolon only with simple scripts that do not use these
 verbs.
 
+Commands inside @{ @} brackets are further restricted.  Posix says that
+they cannot be preceded by addresses, @samp{!}, or @samp{;}, and that
+each command must be followed immediately by a newline, without any
+intervening blanks or semicolons.  The closing bracket must be alone on
+a line, other than white space preceding or following it.
+
 Contrary to yet another urban legend, you may portably use @samp{&} in
 the replacement part of the @code{s} command to mean ``what was
 matched''.  All descendants of Unix version 7 @command{sed}
@@ -12393,6 +12399,10 @@ $ @kbd{echo "foo" | sed -n '/bar/ !p'}
 foo
 @end example
 
+Posix also says that you should not combine @samp{!} and @samp{;}.  If
+you use @samp{!}, it is best to put it on a command that is delimited by
+newlines rather than @samp{;}.
+
 Also note that Posix requires that the @samp{b}, @samp{t}, @samp{r}, and
 @samp{w} commands be followed by exactly one space before their argument.
 On the other hand, no white space is allowed between @samp{:} and the
@@ -12443,20 +12453,21 @@ kept
 deleted
 @end example
 
-Why?  When processing 1, a matches, therefore sets the t flag, b jumps to
-d, and the output is produced.  When processing line 2, the t flag is
-still set (this is the bug).  Line a fails to match, but @command{sed}
-is not supposed to clear the t flag when a substitution fails.  Line b
-sees that the flag is set, therefore it clears it, and jumps to d, hence
-you get @samp{delete me} instead of @samp{deleted}.  When processing 3, t
-is clear, a matches, so the flag is set, hence b clears the flags and
-jumps.  Finally, since the flag is clear, 4 is processed properly.
+Why?  When processing line 1, (a) matches, therefore sets the @samp{t}
+flag, (b)b jumps to (d), and the output is produced.  When processing
+line 2, the @samp{t} flag is still set (this is the bug).  Command (a)
+fails to match, but @command{sed} is not supposed to clear the @samp{t}
+flag when a substitution fails.  Command (b) sees that the flag is set,
+therefore it clears it, and jumps to (d), hence you get @samp{delete me}
+instead of @samp{deleted}.  When processing line (3), @samp{t} is clear,
+(a) matches, so the flag is set, hence (b) clears the flags and jumps.
+Finally, since the flag is clear, line 4 is processed properly.
 
 There are two things one should remember about @samp{t} in @command{sed}.
 Firstly, always remember that @samp{t} jumps if @emph{some} substitution
 succeeded, not only the immediately preceding substitution.  Therefore,
-always use a fake @samp{t clear; :clear} to reset the t flag where
-needed.
+always use a fake @samp{t clear} followed by a @samp{:clear} on the next
+line, to reset the @samp{t} flag where needed.
 
 Secondly, you cannot rely on @command{sed} to clear the flag at each new
 cycle.
Index: lib/autoconf/status.m4
===================================================================
RCS file: /cvsroot/autoconf/autoconf/lib/autoconf/status.m4,v
retrieving revision 1.81
diff -p -u -r1.81 status.m4
--- lib/autoconf/status.m4      5 Jan 2006 15:17:47 -0000       1.81
+++ lib/autoconf/status.m4      5 Jan 2006 21:42:53 -0000
@@ -1,7 +1,7 @@
 # This file is part of Autoconf.                       -*- Autoconf -*-
 # Parameterizing and creating config.status.
 # Copyright (C) 1992, 1993, 1994, 1995, 1996, 1998, 1999, 2000, 2001,
-# 2002, 2003, 2004, 2005 Free Software Foundation, Inc.
+# 2002, 2003, 2004, 2005, 2006 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -363,7 +363,8 @@ m4_define([_AC_SED_CMD_NUM], m4_eval(_AC
 m4_define([_AC_SED_FRAG],
 m4_defn([_AC_SED_FRAG])dnl
 [/^[   address@hidden@[        ]*$/{ r $]_AC_Var[
-d; }
+d
+}
 ])dnl
 ])dnl
 # Remaining file output variables are in a fragment that also has non-file
@@ -461,12 +462,12 @@ m4_popdef([_AC_SED_FRAG])dnl
 # (actually we leave an empty line to preserve line numbers).
 if test "x$srcdir" = x.; then
   ac_vpsub=['/^[        ]*VPATH[        ]*=/{
-s/:*\$(srcdir):*/:/;
-s/:*\${srcdir}:*/:/;
-s/:address@hidden@:*/:/;
-s/^\([^=]*=[    ]*\):*/\1/;
-s/:*$//;
-s/^[^=]*=[      ]*$//;
+s/:*\$(srcdir):*/:/
+s/:*\${srcdir}:*/:/
+s/:address@hidden@:*/:/
+s/^\([^=]*=[    ]*\):*/\1/
+s/:*$//
+s/^[^=]*=[      ]*$//
 }']
 fi
 
@@ -643,9 +644,10 @@ do
   dnl Quote, for the `[ ]' and `define'.
   echo ['    # First, check the format of the line:
     cat >"$tmp/defines.sed" <<CEOF
-/^[     ]*#[    ]*undef[        ][      ]*$ac_word_regexp[     ]*$/!{
-/^[     ]*#[    ]*define[       ][      ]*$ac_word_regexp[(    ]/!b
-}'] >>$CONFIG_STATUS
+/^[     ]*#[    ]*undef[        ][      ]*$ac_word_regexp[     ]*$/b def
+/^[     ]*#[    ]*define[       ][      ]*$ac_word_regexp[(    ]/b def
+b
+:def'] >>$CONFIG_STATUS
   sed ${ac_max_sed_lines}q conftest.defines >>$CONFIG_STATUS
   echo 'CEOF
     sed -f "$tmp/defines.sed"' "$ac_in >$ac_out" >>$CONFIG_STATUS
Index: lib/m4sugar/m4sh.m4
===================================================================
RCS file: /cvsroot/autoconf/autoconf/lib/m4sugar/m4sh.m4,v
retrieving revision 1.152
diff -p -u -r1.152 m4sh.m4
--- lib/m4sugar/m4sh.m4 2 Dec 2005 19:22:49 -0000       1.152
+++ lib/m4sugar/m4sh.m4 5 Jan 2006 21:42:53 -0000
@@ -2,7 +2,7 @@
 # M4 sugar for common shell constructs.
 # Requires GNU M4 and M4sugar.
 #
-# Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005 Free Software
+# Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2006 Free Software
 # Foundation, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
@@ -639,11 +639,23 @@ $as_expr X[]$1 : 'X\(.*[[^/]]\)//*[[^/][
 
 m4_defun([AS_DIRNAME_SED],
 [echo X[]$1 |
-    sed ['/^X\(.*[^/]\)\/\/*[^/][^/]*\/*$/{ s//\1/; q; }
-         /^X\(\/\/\)[^/].*/{ s//\1/; q; }
-         /^X\(\/\/\)$/{ s//\1/; q; }
-         /^X\(\/\).*/{ s//\1/; q; }
-         s/.*/./; q']])
+    sed ['/^X\(.*[^/]\)\/\/*[^/][^/]*\/*$/{
+           s//\1/
+           q
+         }
+         /^X\(\/\/\)[^/].*/{
+           s//\1/
+           q
+         }
+         /^X\(\/\/\)$/{
+           s//\1/
+           q
+         }
+         /^X\(\/\).*/{
+           s//\1/
+           q
+         }
+         s/.*/./; q']])
 
 m4_defun([AS_DIRNAME],
 [(dirname $1) 2>/dev/null ||
@@ -665,10 +677,19 @@ $as_expr X/[]$1 : '.*/\([[^/][^/]*]\)/*$
 
 m4_defun([AS_BASENAME_SED],
 [echo X/[]$1 |
-    sed ['/^.*\/\([^/][^/]*\)\/*$/{ s//\1/; q; }
-         /^X\/\(\/\/\)$/{ s//\1/; q; }
-         /^X\/\(\/\).*/{ s//\1/; q; }
-         s/.*/./; q']])
+    sed ['/^.*\/\([^/][^/]*\)\/*$/{
+           s//\1/
+           q
+         }
+         /^X\/\(\/\/\)$/{
+           s//\1/
+           q
+         }
+         /^X\/\(\/\).*/{
+           s//\1/
+           q
+         }
+         s/.*/./; q']])
 
 m4_defun([AS_BASENAME],
 [AS_REQUIRE([_$0_PREPARE])dnl
Index: tests/foreign.at
===================================================================
RCS file: /cvsroot/autoconf/autoconf/tests/foreign.at,v
retrieving revision 1.23
diff -p -u -r1.23 foreign.at
--- tests/foreign.at    14 May 2005 07:00:40 -0000      1.23
+++ tests/foreign.at    5 Jan 2006 21:42:53 -0000
@@ -2,7 +2,7 @@
 
 AT_BANNER([Compatibility with other tools.])
 
-# Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005 Free Software
+# Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2006 Free Software
 # Foundation, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
@@ -33,7 +33,11 @@ AT_CHECK([libtoolize --version || exit 7
 # CVS Autoconf probably does not work with too old Libtools.
 AT_CHECK([[
 case `libtoolize --version |
-      sed -n '/^.* \([0-9][0-9.a-z]*\)$/{s//\1/;p;q;}'` in
+      sed -n '/^.* \([0-9][0-9.a-z]*\)$/{
+               s//\1/
+               p
+               q
+       }'` in
   0.*          ) exit 77;;
   1.[0123]*    ) exit 77;;
 esac]],
Index: tests/semantics.at
===================================================================
RCS file: /cvsroot/autoconf/autoconf/tests/semantics.at,v
retrieving revision 1.52
diff -p -u -r1.52 semantics.at
--- tests/semantics.at  27 Sep 2005 16:08:12 -0000      1.52
+++ tests/semantics.at  5 Jan 2006 21:42:53 -0000
@@ -2,7 +2,8 @@
 
 AT_BANNER([Semantics.])
 
-# Copyright (C) 2000, 2001, 2002, 2004, 2005 Free Software Foundation, Inc.
+# Copyright (C) 2000, 2001, 2002, 2004, 2005, 2006 Free Software
+# Foundation, Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -420,7 +421,7 @@ AT_DATA([configure.ac],
 [[AC_INIT
 pwd=`pwd`
 
p="1${PATH_SEPARATOR}2${PATH_SEPARATOR}3${PATH_SEPARATOR}4${PATH_SEPARATOR}5${PATH_SEPARATOR}6"
-path=`echo $p | sed -e 's|\([[0-9]]\)|'"$pwd"'/path/\1|g'`
+path=`echo $p | sed -e 's,\([[0-9]]\),'"$pwd"'/path/\1,g'`
 fail=false
 
 AC_CHECK_PROG(TOOL1, tool, found, not-found, $path)
@@ -507,7 +508,7 @@ AT_DATA([configure.ac],
 [[AC_INIT
 pwd=`pwd`
 
p="1${PATH_SEPARATOR}2${PATH_SEPARATOR}3${PATH_SEPARATOR}4${PATH_SEPARATOR}5${PATH_SEPARATOR}6"
-path=`echo $p | sed -e 's|\([[0-9]]\)|'"$pwd"'/path/\1|g'`
+path=`echo $p | sed -e 's,\([[0-9]]\),'"$pwd"'/path/\1,g'`
 fail=false
 
 AC_PATH_PROG(TOOL1, tool, not-found, $path)