Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 146352 - sys-power/hibernate-script: errors in /sbin/functions.sh after resume
Summary: sys-power/hibernate-script: errors in /sbin/functions.sh after resume
Status: RESOLVED WONTFIX
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: High normal (vote)
Assignee: Alon Bar-Lev (RETIRED)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-09-04 22:50 UTC by Alexander Skwar
Modified: 2006-12-22 05:46 UTC (History)
1 user (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
/etc/hibernate (hibernate.tar,30.00 KB, application/x-tar)
2006-09-05 11:33 UTC, Alexander Skwar
Details
/etc/modules.autoload.d/kernel-2.6 (kernel-2.6,3.33 KB, text/plain)
2006-09-18 15:02 UTC, Alexander Skwar
Details
Kernel config (config-2.6.17-suspend2-r6.044.no-kernel-alsa,34.14 KB, text/plain)
2006-09-26 10:25 UTC, Alexander Skwar
Details
modules_gentoo.dash-compatible.patch (modules_gentoo.dash-compatible.patch,2.72 KB, patch)
2006-10-01 13:12 UTC, Alexander Skwar
Details | Diff
modules_gentoo.dash-compatible.v2.patch (modules_gentoo.dash-compatible.v2.patch,2.61 KB, patch)
2006-10-01 14:03 UTC, Alexander Skwar
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Skwar 2006-09-04 22:50:26 UTC
I just resumed my system, after I had it hibernated with 

[ebuild   R   ] sys-power/hibernate-script-1.93-r4  USE="-logrotate vim" 69 kB


alexander@blatt /all $ uname -a
Linux blatt 2.6.17-suspend2-r4.044.no-kernel-alsa #1 PREEMPT Sat Sep 2 14:13:39 CEST 2006 i686 Intel(R) Celeron(R) M processor         1.50GHz GenuineIntel GNU/Linux

To hibernate the system, I ran "sudo hibernate -v3" manually and got the following, when the system resumed:

hibernate: [91] Executing GentooModulesAutoload ...
/sbin/functions.sh: 7: [[: not found
/sbin/functions.sh: 10: declare: not found
/sbin/functions.sh: 11: declare: not found
/sbin/functions.sh: 79: [[: not found
/sbin/functions.sh: 79: [[: not found
/sbin/functions.sh: 88: [[: not found
/sbin/functions.sh: 96: [[: not found
/sbin/functions.sh: 298: Syntax error: Bad substitution


alexander@blatt /all $ emerge --info
Portage 2.1.1_rc1-r2 (default-linux/x86/2006.1, gcc-4.1.1, glibc-2.4-r3, 2.6.17-suspend2-r4.044.no-kernel-alsa i686)
=================================================================
System uname: 2.6.17-suspend2-r4.044.no-kernel-alsa i686 Intel(R) Celeron(R) M processor         1.50GHz
Gentoo Base System version 1.12.4
Last Sync: Tue, 05 Sep 2006 05:20:01 +0000
ccache version 2.4 [enabled]
app-admin/eselect-compiler: [Not Present]
dev-lang/python:     2.3.4-r1, 2.4.3-r3
dev-python/pycrypto: 2.0.1-r5
dev-util/ccache:     2.4-r2
dev-util/confcache:  [Not Present]
sys-apps/sandbox:    1.2.18.1
sys-devel/autoconf:  2.13, 2.60
sys-devel/automake:  1.4_p6, 1.5, 1.6.3, 1.7.9-r1, 1.8.5-r3, 1.9.6-r2
sys-devel/binutils:  2.17
sys-devel/gcc-config: 1.3.13-r3
sys-devel/libtool:   1.5.22
virtual/os-headers:  2.6.17
ACCEPT_KEYWORDS="x86 ~x86"
AUTOCLEAN="yes"
CBUILD="i686-pc-linux-gnu"
CFLAGS="-O2 -mtune=pentium-m -pipe -fomit-frame-pointer"
CHOST="i686-pc-linux-gnu"
CONFIG_PROTECT="/etc /usr/kde/3.5/env /usr/kde/3.5/share/config /usr/kde/3.5/shutdown /usr/share/X11/xkb /usr/share/config"
CONFIG_PROTECT_MASK="/etc/env.d /etc/env.d/java/ /etc/gconf /etc/java-config/vms/ /etc/revdep-rebuild /etc/terminfo /etc/texmf/web2c"
CXXFLAGS="-O2 -mtune=pentium-m -pipe -fomit-frame-pointer"
DISTDIR="/Gentoo/Portage/distfiles"
EMERGE_DEFAULT_OPTS="--alphabetical"
FEATURES="autoconfig buildpkg ccache collision-protect distlocks metadata-transfer sandbox sfperms strict"
GENTOO_MIRRORS="        http://mirrors.sec.informatik.tu-darmstadt.de/gentoo/   http://ftp-stud.fht-esslingen.de/pub/Mirrors/gentoo/       ftp://ftp.tu-clausthal.de/pub/linux/gentoo/     http://distro.ibiblio.org/pub/linux/distributions/gentoo/  ftp://distro.ibiblio.org/pub/linux/distributions/gentoo    http://distfiles.gentoo.org/ "
LANG="de_DE.UTF-8"
LDFLAGS="-Wl,-O1"
LINGUAS="de"
PKGDIR="/Gentoo/Portage/packages"
PORTAGE_RSYNC_OPTS="--recursive --links --safe-links --perms --times --compress --force --whole-file --delete --delete-after --stats --timeout=180 --exclude='/distfiles' --exclude='/local' --exclude='/packages'"
PORTAGE_TMPDIR="/Gentoo/Portage/build"
PORTDIR="/Gentoo/Portage/tree"
PORTDIR_OVERLAY="/Gentoo/Portage/local-tree/misc"
SYNC="rsync://rsync.de.gentoo.org/gentoo-portage"
USE="x86 GAPING_SECURITY_HOLE acpi amd artswrappersuid async bash-completion bdf berkdb bitmap-fonts bluetooth bootsplash cairo caps cardbus ccache cdda cddb cdio cdparanoia cdr cdrom cle266 cli crypt css curlwrappers dbus devmap dillo divx4linux dlloader dri dvd dvdread elibc_glibc emoticon esd exif fam fbcon fbdev firefox fping freetype gdbm gnokii hal hpn icc id3 idn imap imlib2 input_devices_evdev input_devices_keyboard input_devices_mouse insecure-drivers insecure-savers isdnlog javascript jikes kdeenablefinal kdehiddenvisibility kernel_linux libedit libnotify linguas_de linuxthreads-tls logrotate lynxkeymap madwifi maildir matroska mbox mmx mmxext mozilla moznoirc mozsvg mpeg2 mpeg4 mplayer multicall musicbrainz ncurses netboot network new-login nfs nis nls no-old-linux no-suexec noantlr nobcel nobeanutils nobsf nobsh nocd nocommonslogging nocommonsnet nodrm nogg nogulm nojsch nojython nolog4j nomac nooro nopri norhino noxalan noxerces nozaptel nptl nsplugin offensive openssh pam_console pam_timestamp passfile password patented pccts pcmcia pcre perl perlsuid pic player pnp ppds pppd rar readline real recode reflection reiserfs sdl sendfile sensord session sftp sms spf spl sse sse2 ssl startup-notification stream subp subtitles suid symlink sysfs syslog tiff transcode truetype-fonts trusted type1-fonts udev underscores unichrome unicode unsafe usb userland_GNU utf8 uudeview video_cards_fbdev video_cards_vesa video_cards_vga video_cards_via vim vim-pager vlm wifi win32codecs wma123 x11vnc xinetd xml xorg xpm xscreensaver xvid xvmc zlib"
Unset:  CTARGET, INSTALL_MASK, LC_ALL, MAKEOPTS, PORTAGE_RSYNC_EXTRA_OPTS
Comment 1 Christian Heim (RETIRED) gentoo-dev 2006-09-05 10:35:11 UTC
Alexander, could you please upload the files in /etc/hibernate for review ? Simply because it works for me.
Comment 2 Alexander Skwar 2006-09-05 11:33:16 UTC
Created attachment 96094 [details]
/etc/hibernate

As requested - files from /etc/hibernate
Comment 3 Christian Heim (RETIRED) gentoo-dev 2006-09-05 12:05:41 UTC
I can't even reproduce it with your (slightly changed) configuration. Just works here. Sorry.
Comment 4 Alexander Skwar 2006-09-16 22:10:47 UTC
(In reply to comment #3)
> I can't even reproduce it with your (slightly changed) configuration. Just
> works here. Sorry.
> 

Interesting. I built a new system, and there I find the exact same error.

Because of this, I'm reopening this bug.
Comment 5 Alexander Skwar 2006-09-18 15:02:36 UTC
Created attachment 97366 [details]
/etc/modules.autoload.d/kernel-2.6

I get this problem, when I set:

GentooModulesAutoload yes

Without GentooModulesAutoload, no errors show.

Attached, you can find /etc/modules.autoload.d/kernel-2.6. Maybe you can find if there's something broken in the file.
Comment 6 Alexander Skwar 2006-09-26 10:25:46 UTC
Created attachment 98150 [details]
Kernel config

Attached is my kernel configuration.

If you use this kernel AND enable "GentooModulesAutoload yes", you should get the errors I'm getting, I suppose.
Comment 7 Alexander Skwar 2006-09-26 10:27:59 UTC
According to bug #146333 comment #2 , this bug here is a blocker of 146333.
Comment 8 Christian Heim (RETIRED) gentoo-dev 2006-09-26 11:12:21 UTC
OK, Alexander. Let's see if that works for you. Could you please try [1] and see if that fixes your problem ? If yes, then I'll ask Bernard, if he'll accept the patch.

1:http://overlays.gentoo.org/dev/phreak/browser/gentoo/sys-power/hibernate-script/hibernate-script-1.93-r7.ebuild
Comment 9 Alexander Skwar 2006-09-26 11:42:35 UTC
(In reply to comment #8)
> OK, Alexander. Let's see if that works for you. Could you please try [1] and

Certainly.

> see if that fixes your problem ?

Yes, it fixes the problem I had.

> If yes, then I'll ask Bernard, if he'll accept
> the patch.

That'd be great!

Thanks for your help!
Comment 10 Christian Heim (RETIRED) gentoo-dev 2006-09-26 11:53:45 UTC
Thanks a lot for your help again Alexander. This is finally fixed in CVS!
Comment 11 Bo Ørsted Andresen (RETIRED) gentoo-dev 2006-09-26 11:56:35 UTC
I really think you should s/einfo/elog/ in the pkg_postinst() of the ebuild. einfo's aren't shown through elog in a default configuration and the information is important enough that it should be shown..
Comment 12 Alexander Skwar 2006-09-26 22:32:36 UTC
Sorry, it is NOT fixed.

When I suspend my system, I'm calling "hibernate" from a script which runs in a xterm. Only if the return code from hibernate is not 0, I let the xterm be open. Because of this, I did not notice that there's still an error.

When I resume, I get the following errors:

hibernate: [91] Executing GentooModulesAutoload ...
/usr/sbin/hibernate: 7: [[: not found
/usr/sbin/hibernate: 7: [[: not found
/usr/sbin/hibernate: 7: [[: not found
/usr/sbin/hibernate: 7: [[: not found
/usr/sbin/hibernate: 7: [[: not found
/usr/sbin/hibernate: 7: [[: not found
/usr/sbin/hibernate: 7: [[: not found
/usr/sbin/hibernate: 7: arith: syntax error: " KV_MAJOR * 65536 + KV_MINOR * 256 + KV_MICRO "
/usr/sbin/hibernate: 7: [[: not found
/usr/sbin/hibernate: 7: [[: not found
/usr/sbin/hibernate: 7: [[: not found
/usr/sbin/hibernate: 7: [[: not found
/usr/sbin/hibernate: 7: arith: syntax error: " KV_MAJOR * 65536 + KV_MINOR * 256 + KV_MICRO "
Loading modules listed /etc/modules.autoload.d/kernel-2.6
Loading unix
Loading via82cxxx
FATAL: Module via82cxxx not found.
Loading uhci-hcd
Loading ehci-hcd
Loading ath_pci
Loading fan
Loading battery
Loading thermal
Loading button
Loading ac
Loading psmouse
Loading evdev
Loading p4-clockmod
Loading cpufreq_performance
FATAL: Module cpufreq_performance not found.
Loading cpufreq_powersave
Loading cpufreq_ondemand
Loading cpufreq_conservative
Loading ide-cd
Loading via-agp
Loading drm
Loading pcspkr
Loading non-fatal
Loading microcode
hibernate: [90] Executing XStatusProgress ...
Comment 13 Alexander Skwar 2006-09-26 23:51:00 UTC
I'd like to add, that hibernate....-r7 (or rather hibernate-script-1.93-patches-0.5, I suppose) works better than -r6/patches-0.4, though.

With -r6, when I resumed, the system stayed in text mode and had to hit <Alt>+<F7> to switch back to X. When I disable GentooModulesAutoload (or with -r7/0.5), the system automatically switches back to X, just as it should. That's probably another reason why I did not notice the bug yesterday evening.
Comment 14 Christian Heim (RETIRED) gentoo-dev 2006-09-28 04:29:55 UTC
(In reply to comment #12)
> Sorry, it is NOT fixed.
> 
> When I suspend my system, I'm calling "hibernate" from a script which runs in a
> xterm. Only if the return code from hibernate is not 0, I let the xterm be
> open. Because of this, I did not notice that there's still an error.
> 
> When I resume, I get the following errors:
> 
> hibernate: [91] Executing GentooModulesAutoload ...
> /usr/sbin/hibernate: 7: [[: not found
> /usr/sbin/hibernate: 7: [[: not found
> /usr/sbin/hibernate: 7: [[: not found
> /usr/sbin/hibernate: 7: [[: not found
> /usr/sbin/hibernate: 7: [[: not found
> /usr/sbin/hibernate: 7: [[: not found
> /usr/sbin/hibernate: 7: [[: not found
> /usr/sbin/hibernate: 7: arith: syntax error: " KV_MAJOR * 65536 + KV_MINOR *
> 256 + KV_MICRO "
> /usr/sbin/hibernate: 7: [[: not found
> /usr/sbin/hibernate: 7: [[: not found
> /usr/sbin/hibernate: 7: [[: not found
> /usr/sbin/hibernate: 7: [[: not found
> /usr/sbin/hibernate: 7: arith: syntax error: " KV_MAJOR * 65536 + KV_MINOR *
> 256 + KV_MICRO "
> Loading modules listed /etc/modules.autoload.d/kernel-2.6
> Loading unix
> Loading via82cxxx
> FATAL: Module via82cxxx not found.
> Loading uhci-hcd
> Loading ehci-hcd
> Loading ath_pci
> Loading fan
> Loading battery
> Loading thermal
> Loading button
> Loading ac
> Loading psmouse
> Loading evdev
> Loading p4-clockmod
> Loading cpufreq_performance
> FATAL: Module cpufreq_performance not found.
> Loading cpufreq_powersave
> Loading cpufreq_ondemand
> Loading cpufreq_conservative
> Loading ide-cd
> Loading via-agp
> Loading drm
> Loading pcspkr
> Loading non-fatal
> Loading microcode
> hibernate: [90] Executing XStatusProgress ...

OK, another try. Same URL as before (my gentoo overlay on o.g.o but revision 7), this time it _should_ fix it. If you won't remerge the package, simply try to add '#!/bin/bash' to /usr/share/hibernate/scriptlets.d/modules_gentoo at the top.
Comment 15 Alexander Skwar 2006-09-28 12:48:53 UTC
I'm now at hibernate-script-1.93-r8 using your patches v0.6 and I still get this error.

alexander@blatt /dev/shm $ head -n3 /usr/share/hibernate/scriptlets.d/modules_gentoo
#!/bin/bash
# -*- sh -*-
# vim:ft=sh:ts=8:sw=4:noet
Comment 16 Christian Heim (RETIRED) gentoo-dev 2006-10-01 09:06:45 UTC
OK, thanks to Bernard Blackham (the author of most of hibernate-script) and Alexander Skwar, this should be fixed now (hopefully).

/usr/sbin/hibernate was basically looking for /bin/dash which doesn't understand '[[' and certain arithmetics used within KV_to_int (fex).
Comment 17 Alexander Skwar 2006-10-01 13:08:04 UTC
I'll attach a patch against the original modules_gentoo, which makes modules_gentoo be dash compatible. This would obsolete the hibernate-script-1.93-remove-dash-support.patch which is new in the 0.7 patch set.

Reopening this bug to get this patch reviewed (and hopefully integrated).
Comment 18 Alexander Skwar 2006-10-01 13:12:54 UTC
Created attachment 98549 [details, diff]
modules_gentoo.dash-compatible.patch

Patch against modules_gentoo from the hibernate-script-1.93.tar.gz file. With this patch applied, modules_gentoo is dash compatible.

IMO it would also be a good to add this to where this logic was originally taken from (baselayout?), so that this script also gets (more) posix compliant.
Comment 19 Alexander Skwar 2006-10-01 14:03:08 UTC
Created attachment 98554 [details, diff]
modules_gentoo.dash-compatible.v2.patch

New version of the modules_gentoo.dash-compatible.patch - this one resembles the original logic from /etc/init.d/functions much more.

In this version, I exchanged

echo "${KV%%[^[:digit:]]*}"
with
echo "${KV%%[!0-9]*}"

In dash, there's no [:digit:] and a negation is done with ! instead of ^. I also removed the [[ and replaced them with test or [.

I tested it, and on my system, with dash, it works just fine.
Comment 20 SpanKY gentoo-dev 2006-10-01 16:17:02 UTC
what a waste of time

change the code to use /bin/bash and the problem is solved
Comment 21 Alexander Skwar 2006-10-01 23:30:19 UTC
(In reply to comment #20)
> what a waste of time
> 
> change the code to use /bin/bash and the problem is solved
> 

Correct, a waste of time. Write portable code in the first place, and problem solved.

But as you've seen, upstream tries to use dash, if it's available, and dash only understands posix, not bash code. Upstream uses dash, as it's way faster.
Comment 22 Alexander Skwar 2006-10-01 23:31:19 UTC
Spanky, why did you remove the block of bug #146333?

*** This bug has been marked as a duplicate of 146333 ***
Comment 23 SpanKY gentoo-dev 2006-10-02 05:00:19 UTC
you were duplicating functionality already provided by baselayout ... considering that is a Gentoo file, simply using bash and the host baselayout code is fine while rewriting all of the code in baselayout is a waste of time
Comment 24 Alexander Skwar 2006-10-02 07:14:43 UTC
(In reply to comment #23)
> you were duplicating functionality already provided by baselayout ...

Correct, but this has to be done, as the functionality provided by baselayout cannot be used (else this bug would not exist). Reason is, that the code isn't portable, ie. it is not Posix compliant. This in itself can be seen as a bug.

So, either the non-working code from baselayout has to be patched (this IMO is advisable) or it has to be made sure, that only bash is used - this is not advisable, as bash is a lot slower than the Posix compliant dash shell.

> considering that is a Gentoo file, simply using bash and the host baselayout
> code is fine while rewriting all of the code in baselayout is a waste of time

I disagree. It would be proper to rewrite the code in baselayout to be Posix compliant, where possible. And the code which I rewrote was easily rewritable to be Posix compliant, as the functionality did not require any non-Posix code. Writing the code in a non portable way makes no sense; it should only be done IMO, if there's an advantage. In the rewritten code, the non portable code provided no advantage. Or, at least I don't see it - eg. in how far provides [[ an advantage over test, in the way [[ was used here? Or how is [^[:digit:]] better than [!0-9]?

Comment 25 SpanKY gentoo-dev 2006-10-02 07:40:58 UTC
and again, it's a waste of time

you're working on Gentoo code ... Gentoo requires baselayout/bash so just go with those technologies
Comment 26 SpanKY gentoo-dev 2006-10-02 07:54:09 UTC
also, i dont know what spec you're reading, but [:digit:] is valid

because dash does not support it does not mean it is not portable; it means dash is broken
Comment 27 Alexander Skwar 2006-10-02 08:09:08 UTC
(In reply to comment #25)
> and again, it's a waste of time

Yes, it were, if the code were already written in a portable way which conforms to open standards. But it isn't. So, no, it's not a waste of time. It's a waste of time denying that it's bad to break standards for no good reason. As I said, I just don't see why it's better to use [[ instead of test (or [). But please inform me about the advantages (in the way the code is actually used)!

> you're working on Gentoo code ... Gentoo requires baselayout/bash so just go
> with those technologies

hibernate explicitly uses dash for good reasons (as already explained). Because of this, the code has to be fixed. Fixing the non working code is what this bug is about.

(In reply to comment #26)
> also, i dont know what spec you're reading, but [:digit:] is valid

http://www.opengroup.org/onlinepubs/009695399/utilities/sh.html

SuSv3 doesn't show this. At least I haven't found, where in sh named classes are allowed. Please show, where the standard allows named classes.

> because dash does not support it does not mean it is not portable; it means
> dash is broken

That's not correct. It means that [:digit:] shouldn't be used. But maybe you're right - please show where in the standard a named class (eg. [:digit:]) can be used in the shell. Not in RE, but in the shell, ie. in pathname expansion.

Comment 28 SpanKY gentoo-dev 2006-10-02 08:12:30 UTC
Gentoo doesnt aim for POSIX compliance on purpose ... bash has some nice non-POSIX features and we take advantage of them

again, dash is broken ... you didnt read far enough into the specs:
http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_13
[:digit:] is perfectly valid
Comment 29 Alexander Skwar 2006-10-02 08:26:11 UTC
(In reply to comment #26)

> because dash does not support it does not mean it is not portable; it means
> dash is broken

I really don't see that dash is broken. Where is defined, that named classes have to be supported?

In man dash, you can find:

     The following four varieties of parameter expansion provide for substring
     processing.  In each case, pattern matching notation (see Shell
     Patterns), rather than regular expression notation, is used to evaluate
     the patterns.  [...]

     ${parameter%%word}    Remove Largest Suffix Pattern.  The word is
                           expanded to produce a pattern.  [...]

So, word should be Shell Pattern. Shell Patterns are defined as:

   Shell Patterns
     A pattern consists of normal characters, which match themselves, and
     meta-characters.  The meta-characters are ``!'', ``*'', ``?'', and ``[''.
     These characters lose their special meanings if they are quoted.  When
     command or variable substitution is performed and the dollar sign or back
     quotes are not double quoted, the value of the variable or the output of
     the command is scanned for these characters and they are turned into
     meta-characters.

     An asterisk (``*'') matches any string of characters.  A question mark
     matches any single character.  A left bracket (``['') introduces a char-
     acter class.  The end of the character class is indicated by a (``]'');
     if the ``]'' is missing then the ``['' matches a ``['' rather than intro-
     ducing a character class.  A character class matches any of the charac-
     ters between the square brackets.  A range of characters may be specified
     using a minus sign.  The character class may be complemented by making an
     exclamation point the first character of the character class.

     To include a ``]'' in a character class, make it the first character
     listed (after the ``!'', if any).  To include a minus sign, make it the
     first or last character listed.



If dash is buggy, than it's at least a documented bug; or put differently, it behaves according to the documentation. The documentation of Shell Patterns doesn't list named classes (eg. [:digit:]).


(In reply to comment #28)
> Gentoo doesnt aim for POSIX compliance on purpose ... bash has some nice
> non-POSIX features and we take advantage of them

What advantage does [[ provide over [/test in the way it's used here? Where's the advantage of [:digit:] vs. [0-9]?

> again, dash is broken ...

No, it's not - at least I don't see how.

> you didnt read far enough into the specs:
> http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_13
> [:digit:] is perfectly valid

That's about pathname expansion. Is somewhere defined, that in ${parameter%%word} word has to use pathname expansion? If so, where is this defined? Actually, is the syntax of %% defined somewhere? I can't find it in http://www.opengroup.org/onlinepubs/009695399/utilities/sh.html

And actually, I don't understand that fuss. I supplied a patch which makes the code Posix compliant. Even if Gentoo doesn't aim for standards conformance, what would be bad about accepting this code? What disadvantage does it have over the existing code? The advantage is, that it conforms to more standard and can be used in more shells. What advantage does the currently exisiting code provide (other than "it exists")?
Comment 30 SpanKY gentoo-dev 2006-10-02 08:52:32 UTC
so you really need to revise your claims from "POSIX portable" to "dash portable"

i pointed out already that the current baselayout code in question is fully POSIX compliant (even if in general we do not aim for it) ... stop doing a word search for "digit" and actually read the spec ... here, i'll read it for you:

http://www.opengroup.org/onlinepubs/009695399/utilities/sh.html
EXTENDED DESCRIPTION
  See Shell Command Language.

http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02
 2.6.2 Parameter Expansion
${parameter%%word}
    Remove Largest Suffix Pattern.

http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02
 2.13.1 Patterns Matching a Single Character
  The description of basic regular expression bracket expressions in the Base Definitions volume of IEEE Std 1003.1-2001, Section 9.3.5, RE Bracket Expression  shall also apply to the pattern bracket expression
http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap09.html#tag_09_03_05
The following rules and definitions apply to bracket expressions:
 6. All character classes specified in the current locale shall be recognized.
    The following character class expressions shall be supported in all locales:
    [:digit:]
Comment 31 Alexander Skwar 2006-10-02 09:42:00 UTC
(In reply to comment #30)
> so you really need to revise your claims from "POSIX portable" to "dash
> portable"

No, I don't. And even if I'd need to, how would this be bad? What's bad about making the code more portable and thus allowing easier code reusability?

Once more - where's the disadvantage of the code I posted in the patch (attachment id #98554)?

> i pointed out already that the current baselayout code in question is fully
> POSIX compliant

Where did you point this out?

> (even if in general we do not aim for it) ... stop doing a word
> search for "digit"

I didn't. If you need to know, I searched for "class" and found nothing.

> and actually read the spec ... here, i'll read it for you:
> 
> http://www.opengroup.org/onlinepubs/009695399/utilities/sh.html
> EXTENDED DESCRIPTION
>   See Shell Command Language.
> 
> http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02
>  2.6.2 Parameter Expansion
> ${parameter%%word}
>     Remove Largest Suffix Pattern.
> http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02

Thanks for reading to me, what I read to you. :) Granted, you quoted the definite source (the standard) while I quoted the dash man page. But so far, the meaning of the paragraphs is the same, while the wording has some subtle differences.


>  2.13.1 Patterns Matching a Single Character
>   The description of basic regular expression bracket expressions in the Base
> Definitions volume of IEEE Std 1003.1-2001, Section 9.3.5, RE Bracket
> Expression  shall also apply to the pattern bracket expression
> http://www.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap09.html#tag_09_03_05

But note http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_06_02:

The following four varieties of parameter expansion provide for substring processing. In each case, pattern matching notation (see Pattern Matching Notation), rather than regular expression notation, shall be used to evaluate the patterns.

So, regular expressions should NOT be used. You should also read the spec ;)

Some other quote from the specs. http://www.opengroup.org/onlinepubs/009695399/utilities/xcu_chap02.html#tag_02_06_02

The description of basic regular expression bracket expressions in the Base Definitions volume of IEEE Std 1003.1-2001, Section 9.3.5, RE Bracket Expression shall also apply to the pattern bracket expression, except that the exclamation mark character ( '!' ) shall replace the circumflex character ( '^' ) in its role in a "non-matching list" in the regular expression notation.

Would you accept a patch, which replaces the ^ with a !? The spec clearly states, that ^ shouldn't be used.

Also, the spec says:

The description of basic regular expression bracket expressions in the Base Definitions volume of IEEE Std 1003.1-2001, Section 9.3.5, RE Bracket Expression shall also apply to the pattern bracket expression

It says: it *SHALL* apply. So, that's an OPTION. It's clearly not a must. If dash now doesn't implement RE here, than it's still following the standard, I'd say.

Once more - what disadvantage would there be, if the patch modules_gentoo.dash-compatible.v2.patch would also be applied against /etc/init.d/functions? I'm not saying that all of the Gentoo scripts should be rewritten to be Posix (or "dash", if you wish) compliant (although this would be a good idea, IMO). That's not what I'm talking about.

I'm merely talking about accepting the patch and having the KV_* functions be more portable and thus allowing to have this code used in more places (which currently is not the situation; if it were, this bug wouldn't exist).
Comment 32 SpanKY gentoo-dev 2006-10-02 22:19:19 UTC
and where do you draw the line for "portable" code ?  wherever it's convenient for you ?  thanks, but no ... specs exists for a reason; fix the *tool* instead of wasting time "fixing" the code.  do i really need to explain to you why duplicating code is wrong ?

> But note
> <snip>
> So, regular expressions should NOT be used. You should also read the spec ;)

that part is irrelevant to the discussion at hand which is why i did not mention it ... also, who said we were doing regular expressions ?  pattern matching and regular expressions both support bracket expressions for matching sets of elements ... if it didnt, care to explain to me how [0-9] works but [[:digit:]] doesnt ?

in fact, you turn around and correct yourself:

> The description of basic regular expression bracket expressions in the Base
> Definitions volume of IEEE Std 1003.1-2001, Section 9.3.5, RE Bracket
> Expression shall also apply to the pattern bracket expression

in other words, the section i pointed out is certainly valid ... the opengroup guys simply dont duplicate the sections that are the same as doing so is a waste of resources (hmm, sounds familiar ...)

> Would you accept a patch, which replaces the ^ with a !? The spec clearly
> states, that ^ shouldn't be used.

as i said earlier, we support a POSIX base with a bash superset where it provides functionality that is useful ... if you really care, you can file a new bug to change ^ to ! in bracket expansion and i would merge it

> It says: it *SHALL* apply. So, that's an OPTION.

you clearly need to:
 (1) get a dictionary
 (2) look up the word "shall"
 (3) realize you dont know the meaning of "shall"
 (4) read the opengroup specs more often than trying to snip out sections that you *think* supports your argument and then you'd realize that when something is optional, they use the word "optional"

> Once more - what disadvantage would there be, if the patch
> modules_gentoo.dash-compatible.v2.patch would also be applied against
> /etc/init.d/functions?

not a chance in hell ... we're not going to support broken shells for the sake of a few people who insist on not fixing their tools

character classes exist for a reason
Comment 33 Alexander Skwar 2006-10-03 02:09:16 UTC
(In reply to comment #32)
> and where do you draw the line for "portable" code ? 

In the standard.

> wherever it's convenient
> for you ?

This as well. For example, if something works on more programs, than I'd say it's more portable. Eg. [!0-9] is more portable than [^[:digit:]]. Up to now you haven't shown the advantage in the latter code.

>  thanks, but no ... specs exists for a reason;

Exactly.

> fix the *tool* instead
> of wasting time "fixing" the code.

The code is broken (eg. it uses non-standard [[), so why should this *not* be fixed?

>  do i really need to explain to you why
> duplicating code is wrong ?

Yes, you do. Especially, if the fixed code works at least as well if not better. Please explain, why it's good to keep the non working code in /etc/init.d/functions. I just don't understand it.

> > But note
> > <snip>
> > So, regular expressions should NOT be used. You should also read the spec ;)
> 
> that part is irrelevant to the discussion

No, it's not. It is important to the discussion. Is it irrelevant, because it supports my view? Also, not you dictate what's important (neither do I). Please be less aggressive.

> > Would you accept a patch, which replaces the ^ with a !? The spec clearly
> > states, that ^ shouldn't be used.
> 
> as i said earlier, we support a POSIX base with a bash superset where it
> provides functionality that is useful ...

And otherwise you break the standard for no good reason? Is that what you say?

> if you really care, you can file a
> new bug to change ^ to ! in bracket expansion and i would merge it

Fine. Would you also accept a patch which would do away with [[ and replace it with test? Like so:

Old:
[[ -z $1 ]] && return 1 
New:
test x"$1" = "x" && return 1

If not, I'd really be interested in knowing the the advantage of [[ in *THIS* case.

> > It says: it *SHALL* apply. So, that's an OPTION.
> 
> you clearly need to:
>  (1) get a dictionary
>  (2) look up the word "shall"
>  (3) realize you dont know the meaning of "shall"

I don't get you. "shall" means, that something should be done. It doesn't mean, that something has to be done. How about you get a dictionary as well?

>  (4) read the opengroup specs more often than trying to snip out sections that
> you *think* supports your argument and then you'd realize that when something
> is optional, they use the word "optional"
> 
> > Once more - what disadvantage would there be, if the patch
> > modules_gentoo.dash-compatible.v2.patch would also be applied against
> > /etc/init.d/functions?
> 
> not a chance in hell ... we're not going to support broken shells for the sake
> of a few people who insist on not fixing their tools

And where's the disadvantage? For example in using test instead of the clearly non-standard [[? Especially in the way it's used here? I know the theoretical advantage of [[, but it's just not used that way here.

> character classes exist for a reason

In 0-9? No. [0-9] is as good as [[:digit:]]. But maybe I just don't see the disadvantage of using [0-9] instead of [[:digit:]]. Especially not in the use case at hand, ie. in version number of the kernel. Character classes make sense when you've got to deal with unknown locales and don't know what alpha characters are (eg. is 
Comment 34 Alexander Skwar 2006-10-03 02:09:16 UTC
(In reply to comment #32)
> and where do you draw the line for "portable" code ? 

In the standard.

> wherever it's convenient
> for you ?

This as well. For example, if something works on more programs, than I'd say it's more portable. Eg. [!0-9] is more portable than [^[:digit:]]. Up to now you haven't shown the advantage in the latter code.

>  thanks, but no ... specs exists for a reason;

Exactly.

> fix the *tool* instead
> of wasting time "fixing" the code.

The code is broken (eg. it uses non-standard [[), so why should this *not* be fixed?

>  do i really need to explain to you why
> duplicating code is wrong ?

Yes, you do. Especially, if the fixed code works at least as well if not better. Please explain, why it's good to keep the non working code in /etc/init.d/functions. I just don't understand it.

> > But note
> > <snip>
> > So, regular expressions should NOT be used. You should also read the spec ;)
> 
> that part is irrelevant to the discussion

No, it's not. It is important to the discussion. Is it irrelevant, because it supports my view? Also, not you dictate what's important (neither do I). Please be less aggressive.

> > Would you accept a patch, which replaces the ^ with a !? The spec clearly
> > states, that ^ shouldn't be used.
> 
> as i said earlier, we support a POSIX base with a bash superset where it
> provides functionality that is useful ...

And otherwise you break the standard for no good reason? Is that what you say?

> if you really care, you can file a
> new bug to change ^ to ! in bracket expansion and i would merge it

Fine. Would you also accept a patch which would do away with [[ and replace it with test? Like so:

Old:
[[ -z $1 ]] && return 1 
New:
test x"$1" = "x" && return 1

If not, I'd really be interested in knowing the the advantage of [[ in *THIS* case.

> > It says: it *SHALL* apply. So, that's an OPTION.
> 
> you clearly need to:
>  (1) get a dictionary
>  (2) look up the word "shall"
>  (3) realize you dont know the meaning of "shall"

I don't get you. "shall" means, that something should be done. It doesn't mean, that something has to be done. How about you get a dictionary as well?

>  (4) read the opengroup specs more often than trying to snip out sections that
> you *think* supports your argument and then you'd realize that when something
> is optional, they use the word "optional"
> 
> > Once more - what disadvantage would there be, if the patch
> > modules_gentoo.dash-compatible.v2.patch would also be applied against
> > /etc/init.d/functions?
> 
> not a chance in hell ... we're not going to support broken shells for the sake
> of a few people who insist on not fixing their tools

And where's the disadvantage? For example in using test instead of the clearly non-standard [[? Especially in the way it's used here? I know the theoretical advantage of [[, but it's just not used that way here.

> character classes exist for a reason

In 0-9? No. [0-9] is as good as [[:digit:]]. But maybe I just don't see the disadvantage of using [0-9] instead of [[:digit:]]. Especially not in the use case at hand, ie. in version number of the kernel. Character classes make sense when you've got to deal with unknown locales and don't know what alpha characters are (eg. is ß a alpha?). There, something like [[:alpha:]] would make a lot of sense.

So, please go ahead and explain the advantage of [[:digit:]] over [0-9], thanks.

Again - why do you make such a fuss? In how far is it an advantage to use code that exist in /etc/init.d/functions.sh, which cannot be re-used as easily? Do I really need to explain to you, why code re-usability is good?
Comment 35 SpanKY gentoo-dev 2006-10-03 06:05:33 UTC
> The code is broken (eg. it uses non-standard [[), so why should this *not* be
> fixed?

wrong, it is standard ... it is a bracket expansion [] that uses a character class [:digit:]

just because it is written [[:digit:]] does not matter, another valid expression would be [a-b[:digit:]o]

> No, it's not. It is important to the discussion. Is it irrelevant, because it
> supports my view?

then you read it wrong ... it does not support your view.  it simply states that the pattern matching notation is similar to regular expressions, but is not regular expressions.  then, in the sub-section specifically covering bracket expansion, it clearly states:
The description of basic regular expression bracket expressions in the Base Definitions volume of IEEE Std 1003.1-2001, Section 9.3.5, RE Bracket Expression shall also apply to the pattern bracket expression

which means using [:digit:] in [] is perfectly valid POSIX code

> And otherwise you break the standard for no good reason? Is that what you say?

read it again.  i said where bash provides features that do not exist in POSIX (like =~)

> Fine. Would you also accept a patch which would do away with [[ and replace it
> with test? Like so:
> 
> Old:
> [[ -z $1 ]] && return 1 
> New:
> test x"$1" = "x" && return 1

not a chance ... [[ ]] provides sane quoting handling, the =~ operator, and allows for nesting of C like operators rather than -a/-o/etc... which makes for easier reading

> I don't get you. "shall" means, that something should be done.

wrong ... i'll copy and paste the dictionary definition since you wont look it up:
1. To owe; to be under obligation for.
2. To be obliged; must.
3. As an auxiliary, shall indicates a duty or necessity whose obligation is derived from the person speaking; as, you shall go; he shall go; that is, I order or promise your going.


> And where's the disadvantage? For example in using test instead of the clearly
> non-standard [[? Especially in the way it's used here? I know the theoretical
> advantage of [[, but it's just not used that way here.

i dont know why you keep talking about the [[ operator; that isnt what this discussion is about.  we're talking about pattern matching here with brackets, not about the test operator.

> So, please go ahead and explain the advantage of [[:digit:]] over [0-9],
> thanks.

you use the same methodology everywhere ... get out of the style of using "safe" character ranges and into the style of using locale classes.  it generally makes for more readable code and keeps people from having to think "hmm, did i really catch every case"

> Again - why do you make such a fuss?

because you're wasting time pointlessly rewriting code
Comment 36 Alexander Skwar 2006-10-03 09:34:48 UTC
(In reply to comment #34)

> > And otherwise you break the standard for no good reason? Is that what you say?
> 
> read it again.  i said where bash provides features that do not exist in POSIX
> (like =~)

Fine. And why use non standard code, when it's not required (eg. [[ and ^)?

> > Fine. Would you also accept a patch which would do away with [[ and replace it
> > with test? Like so:
> > 
> > Old:
> > [[ -z $1 ]] && return 1 
> > New:
> > test x"$1" = "x" && return 1
> 
> not a chance ... [[ ]] provides sane quoting handling, the =~ operator, and
> allows for nesting of C like operators rather than -a/-o/etc...

In the case of [[ -z $1 ]]? Certainly not. Why break the standard in this case?

So, where's the advantage of [[ -z $1 ]] vs. test x"$1" = "x"?

> which makes for
> easier reading

Depends.

> > I don't get you. "shall" means, that something should be done.
> 
> wrong ... i'll copy and paste the dictionary definition since you wont look it
> up:

You're wrong. Are you always such a dickhead?

> 2. To be obliged; must.

Thanks.

> > And where's the disadvantage? For example in using test instead of the clearly
> > non-standard [[? Especially in the way it's used here? I know the theoretical
> > advantage of [[, but it's just not used that way here.
> 
> i dont know why you keep talking about the [[ operator; that isnt what this
> discussion is about.

It is - see the history of this bug.

Since you obviously are too thick to even read the first comment, let me read something to you:

/sbin/functions.sh: 7: [[: not found

This discussion is about writing code which conforms to the standard. [[ doesn't and never will.

>  we're talking about pattern matching here with brackets,
> not about the test operator.

We're talking about both, as the patch fixes both problems. I don't get, why you 

> > So, please go ahead and explain the advantage of [[:digit:]] over [0-9],
> > thanks.
> 
> you use the same methodology everywhere ... get out of the style of using
> "safe" character ranges and into the style of using locale classes.  it
> generally makes for more readable code and keeps people from having to think
> "hmm, did i really catch every case"

Generally, yes. But not in the case of 0-9 vs. :digit:.

> > Again - why do you make such a fuss?
> 
> because you're wasting time pointlessly rewriting code

If the code would work, then you were right. It doesn't work. One of the reasons is, that the code doesn't follow the SuS standard.

But as the code doesn't work, you're wrong.

You also still need to explain, why it's bad to allow for greater code reusability. You *DO* know what this means, don't you?
Comment 37 SpanKY gentoo-dev 2006-10-04 07:11:11 UTC
i'm tired of this crap so i'll just summarize

 - we draw the line for portability at the POSIX standard ... if it's in POSIX but not in your shell, fix your shell and stop wasting our time
 - [:digit:] is perfectly valid; you of course know this now as you've filed a bug that dsh is indeed broken
 - ^ vs ! is something we didnt notice and dont really care either way; file a bug if you do
 - the baselayout team (you know, the guys actually doing the coding) met some time ago to discuss the bash vs posix issues and we decided then to go fully bash.  there's no point in trying to write POSIX in some places while utilizing bash-only features in others.  that means we standardize on one format only and we do not intermix []/test/[[]] or [0-9]/[:digit:] or any such thing ... go read the STYLE guide in baselayout if you want to know the details

i'm not the maintainer of hibernate-script, phreak is ... that means i'm only telling you your changes wont be accepted in baselayout.  while i think it's stupid to duplicate code in hibernate-script for obvious reasons, the final call is of course not mine.
Comment 38 Alexander Skwar 2006-10-04 07:57:57 UTC
Yes, you're right, this is tiresome - but you know perfectly well, that flame threads tend to be tiresome, I assume. But I don't get, why you started to flame in the first place, but that's your choice.

You are aware, that you're contradicting yourself, aren't you? You say, that you draw the line for portability at the POSIX standard. Fine. But at the same time you say, that you use [[ instead of the portable test/[, although you are aware, that [[ isn't in the POSIX standard and probably never will. 

Please at least try to not contradict yourself - if you say, that you don't care about the standard: Fine. It's a decision. But in this case, don't try to argument using the standard to prove that you're right.

I do agree that it is stupid to duplicate code, however I don't see, why you're so much against code reusability. Really, what's so bad about this, that you try to avoid it so freaking hard?

Well, you've got the last word.
Comment 39 SpanKY gentoo-dev 2006-10-04 09:02:48 UTC
i was talking about the lower line ... bash is a super set of POSIX

you were proposing we lower the bar less than POSIX by not using [:digit:] and i told you no

since we use the superset of bash, and [[ gives us a bunch of nice features that test/[ do not, we use that

stop adding base-system back to CC, i told you what is and isnt going to change
Comment 40 Alon Bar-Lev (RETIRED) gentoo-dev 2006-12-18 12:59:30 UTC
I am catching up...
I really don't understand why it does not work for you...
By default, on Gentoo system /bin/sh is a link to /bin/bash.
/usr/sbin/hibernate uses /bin/sh so it really uses bash.
Everything is working for me...
Do you have a different configuration?
Comment 41 Alon Bar-Lev (RETIRED) gentoo-dev 2006-12-21 12:31:39 UTC
Please reopen if you have some more thought...
Comment 42 Alexander Skwar 2006-12-21 23:53:55 UTC
(In reply to comment #40)
> Please reopen if you have some more thought...
> 

I do have more thoughts, but Gentoo doesn't want to fix this, as you can easily see by the comments of Spanky. Basically, the script should be made portable, but portability isn't wanted. 

So, no, this bug is neither resolved nor invalid.
Comment 43 Alon Bar-Lev (RETIRED) gentoo-dev 2006-12-21 23:57:10 UTC
But it is working!!!!
Please check out again.
Since the default shell is bash, and there is no switch to dash anymore.
Please recheck the last version.
Comment 44 Alexander Skwar 2006-12-22 00:27:53 UTC
Yes, I know that there's no switch to dash anymore and that in itself is wrong. The problem simply is, that the script isn't portable and it can be made portable quite easily. 
Comment 45 Alon Bar-Lev (RETIRED) gentoo-dev 2006-12-22 05:46:39 UTC
OK.
So as I understand it won't be fixed further than it is now.