Opened 13 years ago

Closed 13 years ago

#203 closed defect (fixed)

Solaris 9/10: smartd.conf generation (sed syntax not supported)

Reported by: koitsu2009 Owned by: Christian Franke
Priority: major Milestone: Release 5.43
Component: all Version:
Keywords: solaris Cc:

Description

The below sed problem was introduced in smartmontools 5.42. Errors seen during "make install" phase on both Solaris 9 and Solaris 10:

cat ./smartd.conf.5.in | sed "s|CURRENT_SVN_VERSION|smartmontools-5.42|g; s|CURRENT_SVN_DATE|`sed -n 's,^.*DATE[^"]*"\([^"]*\)".*$,\1,p' svnversion.h`|g; s|CURRENT_SVN_REV|`sed -n 's,^.*REV[^"]*"\([^"]*\)".*$,r\1,p' svnversion.h`|g; s|/usr/local/share/man/|/usr/local/share/man/|g; s|/usr/local/sbin/|/usr/local/sbin/|g; s|/usr/local/etc/rc\\.d/init.d/|/usr/local/etc/init.d/|g; s|/usr/local/share/doc/smartmontools/examplescripts/|!exampledir!|g; s|/usr/local/share/doc/smartmontools/|/usr/local/share/doc/smartmontools/|g; s|!exampledir!|/usr/local/share/doc/smartmontools/examplescripts/|g; s|/usr/local/etc/smartd\\.conf|/usr/local/etc/smartd.conf|g; s|/usr/local/etc/smart_drivedb\\.h|/usr/local/etc/smart_drivedb\\.h|g" | sed '/^\.\\" %IF ENABLE_ATTRIBUTELOG/,/^\.\\" %ENDIF ENABLE_ATTRIBUTELOG/ s,^,.\\"# ,' | sed '/^\.\\" %IF ENABLE_CAPABILITIES/,/^\.\\" %ENDIF ENABLE_CAPABILITIES/ s,^,.\\"# ,' | sed "s|/usr/local/share/smartmontools/drivedb\\.h|/usr/local/share/smartmontools/drivedb.h|g" | sed '/^\.\\" %IF ENABLE_SAVESTATES/,/^\.\\" %ENDIF ENABLE_SAVESTATES/ s,^,.\\"# ,' | if test -n 'Solaris'; then sed -e 's,OS_MAN_FILTER,Solaris,g' -e '/^\.\\" %IF NOT OS .*Solaris/,/^.\\" %ENDIF NOT OS .*Solaris/ s,^,.\\"# ,' -e '/^\.\\" %IF OS .*Solaris/,/^\.\\" %ENDIF OS .*Solaris/ s,^,!!,' -e '/^\.\\" %IF OS ./,/^\.\\" %ENDIF OS ./ s,^,.\\"# ,' -e '/^!*\.\\" %IF NOT OS ./,/^!*\.\\" %ENDIF NOT OS ./ s,^,!!,' -e '/^!!/{ s,^!!!*,,; s,^\.\\"! \(.*\)$,\1 \\"#, ;}' ; else cat; fi > smartd.conf.5
sed: command garbled: /^!!/{ s,^!!!*,,; s,^\.\\"! \(.*\)$,\1 \\"#, ;}
gmake[1]: *** [smartd.conf.5] Error 2

The sed syntax someone chose to use in the new Makefile.am is only compatible with GNU sed, which Solaris does not ship with. The mistake is at line 543 in the source, which was committed in rev 3398.

Annotation:
https://sourceforge.net/browser/trunk/smartmontools/Makefile.am?annotate=blame&rev=3453

I cannot review commit message/revision 3398 because clicking on the line number in SourceForge results in it trying to contact a web server called "piwik.dipohl-kunden.de" which does not answer on TCP port 80 and stalls my web browser forever. There is no mention of this commit revision in CHANGELOG.

The problem can be worked around by installing GNU sed on a Solaris machine, but this is very difficult on Solaris 9 (doesn't compile cleanly for reasons which have nothing to do with smartmontools).

I will work out how the sed command can be changed to work reliably regardless of sed version used.

Attachments (1)

Ticket203-Makefile-sed.patch (601 bytes ) - added by Christian Franke 13 years ago.
Patch for Makefile.am removes {...} from sed command

Download all attachments as: .zip

Change History (11)

comment:1 by koitsu2009, 13 years ago

Pulled from smartmontools.svn.sourceforge.net via Subversion directly:

svn co https://smartmontools.svn.sourceforge.net/svnroot/smartmontools/tags/RELEASE_5_42/ smartmontools

Here's the commit:

r3398 | samm2 | 2011-07-07 15:17:09 -0700 (Thu, 07 Jul 2011) | 1 line

fixed problem with FreeBSD sed

Given that I've used FreeBSD for almost 15 years and use Solaris as well, this sounds to me like something that needs to be addressed in a more appropriate manner. Making assumptions about the capabilities of sed is bad, case in point.

comment:2 by koitsu2009, 13 years ago

$ svn diff -r 3390:3398 Makefile.am | less
Index: Makefile.am
===================================================================
--- Makefile.am (revision 3390)
+++ Makefile.am (revision 3398)
@@ -548,7 +548,7 @@
           -e '/^\.\\" %IF OS .*$(os_man_filter)/,/^\.\\" %ENDIF OS .*$(os_man_filter)/ s,^,!!,' \
           -e '/^\.\\" %IF OS ./,/^\.\\" %ENDIF OS ./ s,^,.\\"\# ,' \
           -e '/^!*\.\\" %IF NOT OS ./,/^!*\.\\" %ENDIF NOT OS ./ s,^,!!,' \
-          -e '/^!!/{ s,^!!!*,,; s,^\.\\"! \(.*\)$$,\1 \\"\#, }' ; \
+          -e '/^!!/{ s,^!!!*,,; s,^\.\\"! \(.*\)$$,\1 \\"\#, ;}' ; \
     else \
       cat; \
     fi

So, given this information, it looks like the actual offending code was introduced prior to revision 3398. It looks like revision 3389 (this isn't a typo) may have been responsible:

r3389 | chrfranke | 2011-06-28 14:50:58 -0700 (Tue, 28 Jun 2011) | 1 line

Add experimental support for platform-specific man pages.

comment:3 by Christian Franke, 13 years ago

Milestone: Release 5.43
Owner: changed from somebody to Christian Franke
Status: newaccepted

Please check whether Solaris sed supports ';' to separate multiple commands on a single line using '{...}' ?

This is "permitted, but not required" by POSIX sed.

by Christian Franke, 13 years ago

Patch for Makefile.am removes {...} from sed command

comment:4 by Christian Franke, 13 years ago

Keywords: solaris added

Please test the above patch.

comment:5 by koitsu2009, 13 years ago

Haven't tested said patch, will do momentarily. Solaris sed supports multiple operators within a block (e.g. /compare/ { op; op; }). The problem is with the syntax of this:

$ sed -e '/^!!/{ s,^\.\\"! \(.*\)$$,\1 \\"\#, ;}'
sed: command garbled: /^!!/{ s,^\.\\"! \(.*\)$$,\1 \\"\#, ;}

Now I assume the double-dollar-sign is due to Makefile complexities and should actually be "$", so I tried that -- same result.

{{
$ sed -e '/!!/{ s,\.
"! \(.*\)$,\1
"\#, ;}'
sed: command garbled: /!!/{ s,\.
"! \(.*\)$,\1
"\#, ;}
}}

Will look at patch momentarily.

comment:6 by koitsu2009, 13 years ago

Patch confirmed as working (strangely enough). Maybe Solaris sed's parser gets confused over use of certain characters when doing combined ops in a block. Not sure. For example, this worked fine:

$ cat smartctl.8.in | sed -e '/IF OS/ { s/IF/snakes/; s/ OS/ rocks/; }' | egrep -c 'snakes|rocks'
44

So it might just be a syntax nuance (parser oddity) of Solaris sed. We'll use the patch on our systems for now, hope to see this in 5.43. Thank you! Now off to file an unrelated bug with SCSI CDBs on Toshiba drives... :-)

comment:7 by koitsu2009, 13 years ago

...that was Fujitsu drives. *laugh* Too many brands for me to keep track of in my head these days.

in reply to:  6 comment:8 by Christian Franke, 13 years ago

Replying to koitsu2009:

Patch confirmed as working (strangely enough).

Thanks for testing, patch will be committed soon.

Maybe Solaris sed's parser gets confused over use of certain characters when doing combined ops in a block. Not sure. For example, this worked fine:
... sed -e '/IF OS/ { s/IF/snakes/; s/ OS/ rocks/; }'

Please retry with '.../ rocks/ ;}'. Fix r3398 added ';' without trailing blank for FreeBSD sed. Does Solaris sed possibly need the blank?

comment:9 by Christian Franke, 13 years ago

The blank preceding ';}' may also be the problem. POSIX: "The functions can be preceded by <blank>s, but shall not be followed by <blank>s."

comment:10 by Christian Franke, 13 years ago

Resolution: fixed
Status: acceptedclosed
Note: See TracTickets for help on using tickets.