Discussion:
[PATCH] make it possible to keep Config/Kbuild snippets in *.c files
(too old to reply)
Denys Vlasenko
2010-05-09 02:04:35 UTC
Permalink
Please review.
--
vda


diff -ad -urpN busybox.0/findutils/Config.in busybox.1/findutils/Config.in
--- busybox.0/findutils/Config.in 2010-04-09 20:01:59.000000000 +0200
+++ busybox.1/findutils/Config.in 1970-01-01 01:00:00.000000000 +0100
@@ -1,253 +0,0 @@
-#
-# For a description of the syntax of this configuration file,
-# see scripts/kbuild/config-language.txt.
-#
-
-menu "Finding Utilities"
-
-config FIND
- bool "find"
- default n
- help
- find is used to search your system to find specified files.
-
-config FEATURE_FIND_PRINT0
- bool "Enable -print0: NUL-terminated output"
- default y
- depends on FIND
- help
- Causes output names to be separated by a NUL character
- rather than a newline. This allows names that contain
- newlines and other whitespace to be more easily
- interpreted by other programs.
-
-config FEATURE_FIND_MTIME
- bool "Enable -mtime: modified time matching"
- default y
- depends on FIND
- help
- Allow searching based on the modification time of
- files, in days.
-
-config FEATURE_FIND_MMIN
- bool "Enable -mmin: modified time matching by minutes"
- default y
- depends on FIND
- help
- Allow searching based on the modification time of
- files, in minutes.
-
-config FEATURE_FIND_PERM
- bool "Enable -perm: permissions matching"
- default y
- depends on FIND
- help
- Enable searching based on file permissions.
-
-config FEATURE_FIND_TYPE
- bool "Enable -type: file type matching (file/dir/link/...)"
- default y
- depends on FIND
- help
- Enable searching based on file type (file,
- directory, socket, device, etc.).
-
-config FEATURE_FIND_XDEV
- bool "Enable -xdev: 'stay in filesystem'"
- default y
- depends on FIND
- help
- This option allows find to restrict searches to a single filesystem.
-
-config FEATURE_FIND_MAXDEPTH
- bool "Enable -maxdepth N"
- default y
- depends on FIND
- help
- This option enables -maxdepth N option.
-
-config FEATURE_FIND_NEWER
- bool "Enable -newer: compare file modification times"
- default y
- depends on FIND
- help
- Support the 'find -newer' option for finding any files which have
- a modified time that is more recent than the specified FILE.
-
-config FEATURE_FIND_INUM
- bool "Enable -inum: inode number matching"
- default y
- depends on FIND
- help
- Support the 'find -inum' option for searching by inode number.
-
-config FEATURE_FIND_EXEC
- bool "Enable -exec: execute commands"
- default y
- depends on FIND
- help
- Support the 'find -exec' option for executing commands based upon
- the files matched.
-
-config FEATURE_FIND_USER
- bool "Enable -user: username/uid matching"
- default y
- depends on FIND
- help
- Support the 'find -user' option for searching by username or uid.
-
-config FEATURE_FIND_GROUP
- bool "Enable -group: group/gid matching"
- default y
- depends on FIND
- help
- Support the 'find -group' option for searching by group name or gid.
-
-config FEATURE_FIND_NOT
- bool "Enable the 'not' (!) operator"
- default y
- depends on FIND
- help
- Support the '!' operator to invert the test results.
- If 'Enable full-blown desktop' is enabled, then will also support
- the non-POSIX notation '-not'.
-
-config FEATURE_FIND_DEPTH
- bool "Enable -depth"
- default y
- depends on FIND
- help
- Process each directory's contents before the directory itself.
-
-config FEATURE_FIND_PAREN
- bool "Enable parens in options"
- default y
- depends on FIND
- help
- Enable usage of parens '(' to specify logical order of arguments.
-
-config FEATURE_FIND_SIZE
- bool "Enable -size: file size matching"
- default y
- depends on FIND
- help
- Support the 'find -size' option for searching by file size.
-
-config FEATURE_FIND_PRUNE
- bool "Enable -prune: exclude subdirectories"
- default y
- depends on FIND
- help
- If the file is a directory, dont descend into it. Useful for
- exclusion .svn and CVS directories.
-
-config FEATURE_FIND_DELETE
- bool "Enable -delete: delete files/dirs"
- default n
- depends on FIND && FEATURE_FIND_DEPTH
- help
- Support the 'find -delete' option for deleting files and directories.
- WARNING: This option can do much harm if used wrong. Busybox will not
- try to protect the user from doing stupid things. Use with care.
-
-config FEATURE_FIND_PATH
- bool "Enable -path: match pathname with shell pattern"
- default y
- depends on FIND
- help
- The -path option matches whole pathname instead of just filename.
-
-config FEATURE_FIND_REGEX
- bool "Enable -regex: match pathname with regex"
- default y
- depends on FIND
- help
- The -regex option matches whole pathname against regular expression.
-
-config FEATURE_FIND_CONTEXT
- bool "Enable -context: security context matching"
- default n
- depends on FIND && SELINUX
- help
- Support the 'find -context' option for matching security context.
-
-config FEATURE_FIND_LINKS
- bool "Enable -links: link count matching"
- default n
- depends on FIND
- help
- Support the 'find -links' option for matching number of links.
-
-config GREP
- bool "grep"
- default n
- help
- grep is used to search files for a specified pattern.
-
-config FEATURE_GREP_EGREP_ALIAS
- bool "Enable extended regular expressions (egrep & grep -E)"
- default y
- depends on GREP
- help
- Enabled support for extended regular expressions. Extended
- regular expressions allow for alternation (foo|bar), grouping,
- and various repetition operators.
-
-config FEATURE_GREP_FGREP_ALIAS
- bool "Alias fgrep to grep -F"
- default y
- depends on GREP
- help
- fgrep sees the search pattern as a normal string rather than
- regular expressions.
- grep -F always works, this just creates the fgrep alias.
-
-config FEATURE_GREP_CONTEXT
- bool "Enable before and after context flags (-A, -B and -C)"
- default y
- depends on GREP
- help
- Print the specified number of leading (-B) and/or trailing (-A)
- context surrounding our matching lines.
- Print the specified number of context lines (-C).
-
-config XARGS
- bool "xargs"
- default n
- help
- xargs is used to execute a specified command for
- every item from standard input.
-
-config FEATURE_XARGS_SUPPORT_CONFIRMATION
- bool "Enable -p: prompt and confirmation"
- default n
- depends on XARGS
- help
- Support -p: prompt the user whether to run each command
- line and read a line from the terminal.
-
-config FEATURE_XARGS_SUPPORT_QUOTES
- bool "Enable single and double quotes and backslash"
- default n
- depends on XARGS
- help
- Support quoting in the input.
-
-config FEATURE_XARGS_SUPPORT_TERMOPT
- bool "Enable -x: exit if -s or -n is exceeded"
- default n
- depends on XARGS
- help
- Support -x: exit if the command size (see the -s or -n option)
- is exceeded.
-
-config FEATURE_XARGS_SUPPORT_ZERO_TERM
- bool "Enable -0: NUL-terminated input"
- default n
- depends on XARGS
- help
- Support -0: input items are terminated by a NUL character
- instead of whitespace, and the quotes and backslash
- are not special.
-
-endmenu
diff -ad -urpN busybox.0/findutils/Config.src busybox.1/findutils/Config.src
--- busybox.0/findutils/Config.src 1970-01-01 01:00:00.000000000 +0100
+++ busybox.1/findutils/Config.src 2010-05-09 01:43:41.000000000 +0200
@@ -0,0 +1,10 @@
+#
+# For a description of the syntax of this configuration file,
+# see scripts/kbuild/config-language.txt.
+#
+
+menu "Finding Utilities"
+
+INSERT
+
+endmenu
diff -ad -urpN busybox.0/findutils/find.c busybox.1/findutils/find.c
--- busybox.0/findutils/find.c 2010-04-27 07:20:34.000000000 +0200
+++ busybox.1/findutils/find.c 2010-05-09 01:52:43.000000000 +0200
@@ -53,6 +53,181 @@
* diff -u /tmp/std_find /tmp/bb_find && echo Identical
*/

+// kbuild:lib-$(CONFIG_FIND) += find.o
+// config:
+// config:config FIND
+// config: bool "find"
+// config: default n
+// config: help
+// config: find is used to search your system to find specified files.
+// config:
+// config:config FEATURE_FIND_PRINT0
+// config: bool "Enable -print0: NUL-terminated output"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Causes output names to be separated by a NUL character
+// config: rather than a newline. This allows names that contain
+// config: newlines and other whitespace to be more easily
+// config: interpreted by other programs.
+// config:
+// config:config FEATURE_FIND_MTIME
+// config: bool "Enable -mtime: modified time matching"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Allow searching based on the modification time of
+// config: files, in days.
+// config:
+// config:config FEATURE_FIND_MMIN
+// config: bool "Enable -mmin: modified time matching by minutes"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Allow searching based on the modification time of
+// config: files, in minutes.
+// config:
+// config:config FEATURE_FIND_PERM
+// config: bool "Enable -perm: permissions matching"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Enable searching based on file permissions.
+// config:
+// config:config FEATURE_FIND_TYPE
+// config: bool "Enable -type: file type matching (file/dir/link/...)"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Enable searching based on file type (file,
+// config: directory, socket, device, etc.).
+// config:
+// config:config FEATURE_FIND_XDEV
+// config: bool "Enable -xdev: 'stay in filesystem'"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: This option allows find to restrict searches to a single filesystem.
+// config:
+// config:config FEATURE_FIND_MAXDEPTH
+// config: bool "Enable -maxdepth N"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: This option enables -maxdepth N option.
+// config:
+// config:config FEATURE_FIND_NEWER
+// config: bool "Enable -newer: compare file modification times"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Support the 'find -newer' option for finding any files which have
+// config: a modified time that is more recent than the specified FILE.
+// config:
+// config:config FEATURE_FIND_INUM
+// config: bool "Enable -inum: inode number matching"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Support the 'find -inum' option for searching by inode number.
+// config:
+// config:config FEATURE_FIND_EXEC
+// config: bool "Enable -exec: execute commands"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Support the 'find -exec' option for executing commands based upon
+// config: the files matched.
+// config:
+// config:config FEATURE_FIND_USER
+// config: bool "Enable -user: username/uid matching"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Support the 'find -user' option for searching by username or uid.
+// config:
+// config:config FEATURE_FIND_GROUP
+// config: bool "Enable -group: group/gid matching"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Support the 'find -group' option for searching by group name or gid.
+// config:
+// config:config FEATURE_FIND_NOT
+// config: bool "Enable the 'not' (!) operator"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Support the '!' operator to invert the test results.
+// config: If 'Enable full-blown desktop' is enabled, then will also support
+// config: the non-POSIX notation '-not'.
+// config:
+// config:config FEATURE_FIND_DEPTH
+// config: bool "Enable -depth"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Process each directory's contents before the directory itself.
+// config:
+// config:config FEATURE_FIND_PAREN
+// config: bool "Enable parens in options"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Enable usage of parens '(' to specify logical order of arguments.
+// config:
+// config:config FEATURE_FIND_SIZE
+// config: bool "Enable -size: file size matching"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: Support the 'find -size' option for searching by file size.
+// config:
+// config:config FEATURE_FIND_PRUNE
+// config: bool "Enable -prune: exclude subdirectories"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: If the file is a directory, dont descend into it. Useful for
+// config: exclusion .svn and CVS directories.
+// config:
+// config:config FEATURE_FIND_DELETE
+// config: bool "Enable -delete: delete files/dirs"
+// config: default n
+// config: depends on FIND && FEATURE_FIND_DEPTH
+// config: help
+// config: Support the 'find -delete' option for deleting files and directories.
+// config: WARNING: This option can do much harm if used wrong. Busybox will not
+// config: try to protect the user from doing stupid things. Use with care.
+// config:
+// config:config FEATURE_FIND_PATH
+// config: bool "Enable -path: match pathname with shell pattern"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: The -path option matches whole pathname instead of just filename.
+// config:
+// config:config FEATURE_FIND_REGEX
+// config: bool "Enable -regex: match pathname with regex"
+// config: default y
+// config: depends on FIND
+// config: help
+// config: The -regex option matches whole pathname against regular expression.
+// config:
+// config:config FEATURE_FIND_CONTEXT
+// config: bool "Enable -context: security context matching"
+// config: default n
+// config: depends on FIND && SELINUX
+// config: help
+// config: Support the 'find -context' option for matching security context.
+// config:
+// config:config FEATURE_FIND_LINKS
+// config: bool "Enable -links: link count matching"
+// config: default n
+// config: depends on FIND
+// config: help
+// config: Support the 'find -links' option for matching number of links.
+
#include <fnmatch.h>
#include "libbb.h"
#if ENABLE_FEATURE_FIND_REGEX
diff -ad -urpN busybox.0/findutils/grep.c busybox.1/findutils/grep.c
--- busybox.0/findutils/grep.c 2010-04-30 09:30:10.000000000 +0200
+++ busybox.1/findutils/grep.c 2010-05-09 01:49:28.000000000 +0200
@@ -19,6 +19,41 @@
* (C) 2006 Jac Goudsmit added -o option
*/

+// kbuild:lib-$(CONFIG_GREP) += grep.o
+// config:
+// config:config GREP
+// config: bool "grep"
+// config: default n
+// config: help
+// config: grep is used to search files for a specified pattern.
+// config:
+// config:config FEATURE_GREP_EGREP_ALIAS
+// config: bool "Enable extended regular expressions (egrep & grep -E)"
+// config: default y
+// config: depends on GREP
+// config: help
+// config: Enabled support for extended regular expressions. Extended
+// config: regular expressions allow for alternation (foo|bar), grouping,
+// config: and various repetition operators.
+// config:
+// config:config FEATURE_GREP_FGREP_ALIAS
+// config: bool "Alias fgrep to grep -F"
+// config: default y
+// config: depends on GREP
+// config: help
+// config: fgrep sees the search pattern as a normal string rather than
+// config: regular expressions.
+// config: grep -F always works, this just creates the fgrep alias.
+// config:
+// config:config FEATURE_GREP_CONTEXT
+// config: bool "Enable before and after context flags (-A, -B and -C)"
+// config: default y
+// config: depends on GREP
+// config: help
+// config: Print the specified number of leading (-B) and/or trailing (-A)
+// config: context surrounding our matching lines.
+// config: Print the specified number of context lines (-C).
+
#include "libbb.h"
#include "xregex.h"

diff -ad -urpN busybox.0/findutils/Kbuild busybox.1/findutils/Kbuild
--- busybox.0/findutils/Kbuild 2010-04-09 20:01:59.000000000 +0200
+++ busybox.1/findutils/Kbuild 1970-01-01 01:00:00.000000000 +0100
@@ -1,10 +0,0 @@
-# Makefile for busybox
-#
-# Copyright (C) 1999-2005 by Erik Andersen <andersen at codepoet.org>
-#
-# Licensed under the GPL v2, see the file LICENSE in this tarball.
-
-lib-y:=
-lib-$(CONFIG_FIND) += find.o
-lib-$(CONFIG_GREP) += grep.o
-lib-$(CONFIG_XARGS) += xargs.o
diff -ad -urpN busybox.0/findutils/Kbuild.src busybox.1/findutils/Kbuild.src
--- busybox.0/findutils/Kbuild.src 1970-01-01 01:00:00.000000000 +0100
+++ busybox.1/findutils/Kbuild.src 2010-05-09 03:54:53.000000000 +0200
@@ -0,0 +1,9 @@
+# Makefile for busybox
+#
+# Copyright (C) 1999-2005 by Erik Andersen <andersen at codepoet.org>
+#
+# Licensed under the GPL v2, see the file LICENSE in this tarball.
+
+lib-y:=
+
+INSERT
diff -ad -urpN busybox.0/findutils/xargs.c busybox.1/findutils/xargs.c
--- busybox.0/findutils/xargs.c 2010-04-09 20:01:59.000000000 +0200
+++ busybox.1/findutils/xargs.c 2010-05-09 01:49:25.000000000 +0200
@@ -17,6 +17,47 @@
*
*/

+// kbuild:lib-$(CONFIG_XARGS) += xargs.o
+// config:
+// config:config XARGS
+// config: bool "xargs"
+// config: default n
+// config: help
+// config: xargs is used to execute a specified command for
+// config: every item from standard input.
+// config:
+// config:config FEATURE_XARGS_SUPPORT_CONFIRMATION
+// config: bool "Enable -p: prompt and confirmation"
+// config: default n
+// config: depends on XARGS
+// config: help
+// config: Support -p: prompt the user whether to run each command
+// config: line and read a line from the terminal.
+// config:
+// config:config FEATURE_XARGS_SUPPORT_QUOTES
+// config: bool "Enable single and double quotes and backslash"
+// config: default n
+// config: depends on XARGS
+// config: help
+// config: Support quoting in the input.
+// config:
+// config:config FEATURE_XARGS_SUPPORT_TERMOPT
+// config: bool "Enable -x: exit if -s or -n is exceeded"
+// config: default n
+// config: depends on XARGS
+// config: help
+// config: Support -x: exit if the command size (see the -s or -n option)
+// config: is exceeded.
+// config:
+// config:config FEATURE_XARGS_SUPPORT_ZERO_TERM
+// config: bool "Enable -0: NUL-terminated input"
+// config: default n
+// config: depends on XARGS
+// config: help
+// config: Support -0: input items are terminated by a NUL character
+// config: instead of whitespace, and the quotes and backslash
+// config: are not special.
+
#include "libbb.h"

/* This is a NOEXEC applet. Be very careful! */
diff -ad -urpN busybox.0/Makefile busybox.1/Makefile
--- busybox.0/Makefile 2010-04-09 20:01:58.000000000 +0200
+++ busybox.1/Makefile 2010-05-09 03:53:15.000000000 +0200
@@ -377,6 +377,11 @@ ifneq ($(KBUILD_SRC),)
$(srctree) $(objtree) $(VERSION) $(PATCHLEVEL)
endif

+# This target generates Kbuild's and Config.in's from *.c files
+PHONY += gen_build_files
+gen_build_files:
+ $(Q)$(srctree)/scripts/gen_build_files.sh $(srctree) $(objtree)
+
# To make sure we do not include .config for any of the *config targets
# catch them early, and hand them over to scripts/kconfig/Makefile
# It is allowed to specify more targets when calling make, including
@@ -428,7 +433,7 @@ ifeq ($(config-targets),1)
-include $(srctree)/arch/$(ARCH)/Makefile
export KBUILD_DEFCONFIG

-config %config: scripts_basic outputmakefile FORCE
+config %config: scripts_basic outputmakefile gen_build_files FORCE
$(Q)mkdir -p include
$(Q)$(MAKE) $(build)=scripts/kconfig $@
$(Q)$(MAKE) -C $(srctree) KBUILD_SRC= .kernelrelease
@@ -443,7 +448,7 @@ ifeq ($(KBUILD_EXTMOD),)
# Carefully list dependencies so we do not try to build scripts twice
# in parrallel
PHONY += scripts
-scripts: scripts_basic include/config/MARKER
+scripts: gen_build_files scripts_basic include/config/MARKER
$(Q)$(MAKE) $(build)=$(@)

scripts_basic: include/autoconf.h
diff -ad -urpN busybox.0/scripts/gen_build_files.sh busybox.1/scripts/gen_build_files.sh
--- busybox.0/scripts/gen_build_files.sh 1970-01-01 01:00:00.000000000 +0100
+++ busybox.1/scripts/gen_build_files.sh 2010-05-09 03:58:09.000000000 +0200
@@ -0,0 +1,51 @@
+#!/bin/sh
+
+test $# -ge 2 || exit 1
+
+# cd to objtree
+cd "$2" || exit 1
+
+srctree="$1"
+
+find -type d \
+| while read; do
+ d="$REPLY"
+
+ test -f "$srctree/$d"/Kbuild.src && {
+ echo " CHK $d/Kbuild"
+ s=`grep -h '^// kbuild:' "$d"/*.c | sed 's^// kbuild:^^'`
+ {
+ while read; do
+ test x"$REPLY" = x"INSERT" && REPLY="$s"
+ printf "%s\n" "$REPLY"
+ done <"$srctree/$d"/Kbuild.src
+ } >"$d"/Kbuild.$$.tmp
+ if cmp -s "$d"/Kbuild.$$.tmp "$d"/Kbuild; then
+ rm "$d"/Kbuild.$$.tmp
+ else
+ echo " GEN $d/Kbuild"
+ mv "$d"/Kbuild.$$.tmp "$d"/Kbuild
+ fi
+ }
+
+ test -f "$srctree/$d"/Config.src && {
+ echo " CHK $d/Config.in"
+ s=`grep -h '^// config:' "$d"/*.c | sed 's^// config:^^'`
+ {
+ while read; do
+ test x"$REPLY" = x"INSERT" && REPLY="$s"
+ printf "%s\n" "$REPLY"
+ done <"$srctree/$d"/Config.src
+ } >"$d"/Config.in.$$.tmp
+ if cmp -s "$d"/Config.in.$$.tmp "$d"/Config.in; then
+ rm "$d"/Config.in.$$.tmp
+ else
+ echo " GEN $d/Config.in"
+ mv "$d"/Config.in.$$.tmp "$d"/Config.in
+ fi
+ }
+
+done
+
+# Last read failed. This is normal. Don't exit with its error code:
+exit 0
Rob Landley
2010-05-09 05:31:48 UTC
Permalink
Post by Denys Vlasenko
Please review.
--
vda
Well, obviously I'm in favor of the concept, since toybox does something
similar. :)
Post by Denys Vlasenko
diff -ad -urpN busybox.0/findutils/Config.in busybox.1/findutils/Config.in
--- busybox.0/findutils/Config.in 2010-04-09 20:01:59.000000000 +0200
+++ busybox.1/findutils/Config.in 1970-01-01 01:00:00.000000000 +0100
@@ -1,253 +0,0 @@
-#
-# For a description of the syntax of this configuration file,
-# see scripts/kbuild/config-language.txt.
-#
-
-menu "Finding Utilities"
...
Post by Denys Vlasenko
-endmenu
Zapped the whole file, ok...
Post by Denys Vlasenko
diff -ad -urpN busybox.0/findutils/Config.src
busybox.1/findutils/Config.src ---
busybox.0/findutils/Config.src 1970-01-01 01:00:00.000000000 +0100 +++
+#
+# For a description of the syntax of this configuration file,
+# see scripts/kbuild/config-language.txt.
+#
+
+menu "Finding Utilities"
+
+INSERT
+
+endmenu
If you're going to have some commentary in this file already, you might mention
that Config.in is generated from Config.src file by gen_build_files.sh.

Structural comment: I separated the generated code from the source code,
putting it all in a "generated" directory.

I believe you're changing "Config.in" from a file that's tracked in the
repository to a file that doesn't exist in the repository but is generated in
situ as a temp file? (And presumably goes away with "make clean"?) People may
not notice that, and may waste time editing a temp file. If the file moves,
people will be forced to notice. (I find it also makes the dependencies and
cleaning logic easier to follow, but that could just be me...)
Post by Denys Vlasenko
diff -ad -urpN busybox.0/findutils/find.c busybox.1/findutils/find.c
--- busybox.0/findutils/find.c 2010-04-27 07:20:34.000000000 +0200
+++ busybox.1/findutils/find.c 2010-05-09 01:52:43.000000000 +0200
@@ -53,6 +53,181 @@
* diff -u /tmp/std_find /tmp/bb_find && echo Identical
*/
+// kbuild:lib-$(CONFIG_FIND) += find.o
+// config:config FIND
+// config: bool "find"
+// config: default n
+// config: help
+// config: find is used to search your system to find specified
...
Post by Denys Vlasenko
+// config: Support the 'find -links' option for matching number of
links. +
#include <fnmatch.h>
#include "libbb.h"
#if ENABLE_FEATURE_FIND_REGEX
A little on the verbose side as syntaxes go, but fairly straightforward.

What do you do if somebody typos "comfig" or similar? With the syntax I chose,
if there was a screw-up the build would generally break. In this case, a
single line of could silently drop out in a way that wouldn't necessarily be
immediately obvious...

I note that CONFIG_APPNAME += appname.o is the common case, which could
presumably be generated automatically for 90% of the commands. (You only
really need to specify something like this when there's more than one .o file,
or if the .o file is a different name...)
Post by Denys Vlasenko
diff -ad -urpN busybox.0/findutils/grep.c busybox.1/findutils/grep.c
--- busybox.0/findutils/grep.c 2010-04-30 09:30:10.000000000 +0200
+++ busybox.1/findutils/grep.c 2010-05-09 01:49:28.000000000 +0200
@@ -19,6 +19,41 @@
* (C) 2006 Jac Goudsmit added -o option
*/
+// kbuild:lib-$(CONFIG_GREP) += grep.o
+// config:config GREP
+// config: bool "grep"
+// config: default n
...
Post by Denys Vlasenko
+// config: depends on GREP
+// config: help
+// config: Print the specified number of leading (-B) and/or
trailing (-A) +// config: context surrounding our matching lines.
+// config: Print the specified number of context lines (-C).
+
#include "libbb.h"
#include "xregex.h"
More of the same, ok.
Post by Denys Vlasenko
diff -ad -urpN busybox.0/findutils/Kbuild busybox.1/findutils/Kbuild
--- busybox.0/findutils/Kbuild 2010-04-09 20:01:59.000000000 +0200
+++ busybox.1/findutils/Kbuild 1970-01-01 01:00:00.000000000 +0100
@@ -1,10 +0,0 @@
-# Makefile for busybox
-#
-# Copyright (C) 1999-2005 by Erik Andersen <andersen at codepoet.org>
-#
-# Licensed under the GPL v2, see the file LICENSE in this tarball.
-
-lib-y:=
-lib-$(CONFIG_FIND) += find.o
-lib-$(CONFIG_GREP) += grep.o
-lib-$(CONFIG_XARGS) += xargs.o
Deleting whole file, ok.
Post by Denys Vlasenko
diff -ad -urpN busybox.0/findutils/Kbuild.src
busybox.1/findutils/Kbuild.src ---
busybox.0/findutils/Kbuild.src 1970-01-01 01:00:00.000000000 +0100 +++
+# Makefile for busybox
+#
+# Copyright (C) 1999-2005 by Erik Andersen <andersen at codepoet.org>
+#
+# Licensed under the GPL v2, see the file LICENSE in this tarball.
+
+lib-y:=
+
+INSERT
More boilerplate.

Again, this looks like a default case. If you're going to do this for more
than one directory, possibly this should be emitted by the script rather than
appended to, and only non-default stuff should go in here.

The Config.src file had a reason to exist because it was naming the menu, but
this one isn't actually doing anything.

Also, since these files are generated you don't really need one per directory.
You could just make one big one. (Which brings us back to the generated
directory, mentioned earlier.)

Still, that sort of thing could easily be done in stages. (Cleanup on top of
cleanup, this is fine for now...) The important thing is getting the syntax
that goes into the .c files right, so that conversion doesn't have to be
redone.
Post by Denys Vlasenko
diff -ad -urpN busybox.0/findutils/xargs.c busybox.1/findutils/xargs.c
--- busybox.0/findutils/xargs.c 2010-04-09 20:01:59.000000000 +0200
+++ busybox.1/findutils/xargs.c 2010-05-09 01:49:25.000000000 +0200
@@ -17,6 +17,47 @@
*
*/
+// kbuild:lib-$(CONFIG_XARGS) += xargs.o
+// config:config XARGS
...
Post by Denys Vlasenko
+// config: are not special.
+
#include "libbb.h"
/* This is a NOEXEC applet. Be very careful! */
More of the same, ok.
Post by Denys Vlasenko
diff -ad -urpN busybox.0/Makefile busybox.1/Makefile
--- busybox.0/Makefile 2010-04-09 20:01:58.000000000 +0200
+++ busybox.1/Makefile 2010-05-09 03:53:15.000000000 +0200
@@ -377,6 +377,11 @@ ifneq ($(KBUILD_SRC),)
$(srctree) $(objtree) $(VERSION) $(PATCHLEVEL)
endif
+# This target generates Kbuild's and Config.in's from *.c files
+PHONY += gen_build_files
+ $(Q)$(srctree)/scripts/gen_build_files.sh $(srctree) $(objtree)
+
This rebuilds every time. Wouldn't it be better to do dependencies on the .c
files, ala:

gen_build_files: findutils/*.c
Post by Denys Vlasenko
# To make sure we do not include .config for any of the *config targets
# catch them early, and hand them over to scripts/kconfig/Makefile
# It is allowed to specify more targets when calling make, including
@@ -428,7 +433,7 @@ ifeq ($(config-targets),1)
-include $(srctree)/arch/$(ARCH)/Makefile
export KBUILD_DEFCONFIG
-config %config: scripts_basic outputmakefile FORCE
+config %config: scripts_basic outputmakefile gen_build_files FORCE
$(Q)mkdir -p include
$(Q)$(MAKE) -C $(srctree) KBUILD_SRC= .kernelrelease
@@ -443,7 +448,7 @@ ifeq ($(KBUILD_EXTMOD),)
# Carefully list dependencies so we do not try to build scripts twice
# in parrallel
PHONY += scripts
-scripts: scripts_basic include/config/MARKER
+scripts: gen_build_files scripts_basic include/config/MARKER
scripts_basic: include/autoconf.h
It seems like that first hunk would have been sufficient, but the busybox
makefiles have always been a bit overcomplicated, haven't they?
Post by Denys Vlasenko
diff -ad -urpN busybox.0/scripts/gen_build_files.sh
busybox.1/scripts/gen_build_files.sh ---
busybox.0/scripts/gen_build_files.sh 1970-01-01 01:00:00.000000000
+0100 +++ busybox.1/scripts/gen_build_files.sh 2010-05-09
+#!/bin/sh
+
+test $# -ge 2 || exit 1
+
Might want to emit a usage message there. Just a thought.
Post by Denys Vlasenko
+# cd to objtree
+cd "$2" || exit 1
+
+srctree="$1"
+
+find -type d \
+| while read; do
+ d="$REPLY"
find -type d | while read d
do
Post by Denys Vlasenko
+ test -f "$srctree/$d"/Kbuild.src && {
Possibly just a style thing, but it seems more shell-ish to do:

if [ -f "$srctree/$d/Kbuild.src" ]
then

*shrug*
Post by Denys Vlasenko
+ echo " CHK $d/Kbuild"
+ s=`grep -h '^// kbuild:' "$d"/*.c | sed 's^// kbuild:^^'`
+ {
+ while read; do
+ test x"$REPLY" = x"INSERT" && REPLY="$s"
+ printf "%s\n" "$REPLY"
+ done <"$srctree/$d"/Kbuild.src
+ } >"$d"/Kbuild.$$.tmp
Um, where is this read loop getting input from? I missed a curve. (The outer
read loop had something piped into it, but this one is listening to stdin?)

So we pass along most lines unmodified, but if the line we read was "INSERT"
then we grab the data sed accumlated above. Currently, the INSERT line is at
the end of the file. If that's always going to be the case (and this is part
of the declarative code of the makefile so order shouldn't matter), can't we
Post by Denys Vlasenko
+ if cmp -s "$d"/Kbuild.$$.tmp "$d"/Kbuild; then
+ rm "$d"/Kbuild.$$.tmp
+ else
+ echo " GEN $d/Kbuild"
+ mv "$d"/Kbuild.$$.tmp "$d"/Kbuild
+ fi
+ }
If you get the dependencies right you won't regenerate these when you don't
need to.
Post by Denys Vlasenko
+ test -f "$srctree/$d"/Config.src && {
+ echo " CHK $d/Config.in"
+ s=`grep -h '^// config:' "$d"/*.c | sed 's^// config:^^'`
+ {
+ while read; do
+ test x"$REPLY" = x"INSERT" && REPLY="$s"
+ printf "%s\n" "$REPLY"
+ done <"$srctree/$d"/Config.src
+ } >"$d"/Config.in.$$.tmp
+ if cmp -s "$d"/Config.in.$$.tmp "$d"/Config.in; then
+ rm "$d"/Config.in.$$.tmp
+ else
+ echo " GEN $d/Config.in"
+ mv "$d"/Config.in.$$.tmp "$d"/Config.in
+ fi
+ }
And the same again for Config.in.

I note that bash is a wonderful thing, and in theory you should be able to do
most of the above with something like (off the top of my head, untested):

sed '/^INSERT$//r '<$(sed 's@^// config:@@p' "$d"/*.c) > "$d"/Config.in

*shrug*
Post by Denys Vlasenko
+done
+
+exit 0
Looks like it'll probably work.

Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
Denys Vlasenko
2010-05-09 14:19:58 UTC
Permalink
Post by Rob Landley
I note that CONFIG_APPNAME += appname.o is the common case, which could
presumably be generated automatically for 90% of the commands. (You only
really need to specify something like this when there's more than one .o file,
or if the .o file is a different name...)
Yes, de do have those 10%
Post by Rob Landley
Again, this looks like a default case. If you're going to do this for more
than one directory, possibly this should be emitted by the script rather than
appended to, and only non-default stuff should go in here.
The Config.src file had a reason to exist because it was naming the menu, but
this one isn't actually doing anything.
applets/ and archival/ have non-trivial Kbuilds
Post by Rob Landley
Also, since these files are generated you don't really need one per directory.
You could just make one big one. (Which brings us back to the generated
directory, mentioned earlier.)
Still, that sort of thing could easily be done in stages. (Cleanup on top of
cleanup, this is fine for now...) The important thing is getting the syntax
that goes into the .c files right, so that conversion doesn't have to be
redone.
Right.
Post by Rob Landley
This rebuilds every time. Wouldn't it be better to do dependencies on the .c
gen_build_files: findutils/*.c
Tried that:

gen_build_files: $(wildcard */*.c)

It attempted to rebuild a host tool:

$ make
HOSTCC scripts/basic/fixdep
HOSTCC scripts/basic/split-include
HOSTCC scripts/basic/docproc
HOSTCC applets/usage
applets/usage.c:11:22: error: autoconf.h: No such file or directory
In file included from applets/usage.c:27:
include/applets.h:70: error: expected ',' or ';' before 'IF_TEST'
make[2]: *** [applets/usage] Error 1
make[1]: *** [applets_dir] Error 2
make: *** [include/autoconf.h] Error 2
Post by Rob Landley
Post by Denys Vlasenko
# To make sure we do not include .config for any of the *config targets
# catch them early, and hand them over to scripts/kconfig/Makefile
# It is allowed to specify more targets when calling make, including
@@ -428,7 +433,7 @@ ifeq ($(config-targets),1)
-include $(srctree)/arch/$(ARCH)/Makefile
export KBUILD_DEFCONFIG
-config %config: scripts_basic outputmakefile FORCE
+config %config: scripts_basic outputmakefile gen_build_files FORCE
$(Q)mkdir -p include
$(Q)$(MAKE) -C $(srctree) KBUILD_SRC= .kernelrelease
@@ -443,7 +448,7 @@ ifeq ($(KBUILD_EXTMOD),)
# Carefully list dependencies so we do not try to build scripts twice
# in parrallel
PHONY += scripts
-scripts: scripts_basic include/config/MARKER
+scripts: gen_build_files scripts_basic include/config/MARKER
scripts_basic: include/autoconf.h
It seems like that first hunk would have been sufficient, but the busybox
makefiles have always been a bit overcomplicated, haven't they?
(1) I don't know.
(2) Yes.
Post by Rob Landley
Post by Denys Vlasenko
diff -ad -urpN busybox.0/scripts/gen_build_files.sh
busybox.1/scripts/gen_build_files.sh ---
busybox.0/scripts/gen_build_files.sh 1970-01-01 01:00:00.000000000
+0100 +++ busybox.1/scripts/gen_build_files.sh 2010-05-09
+#!/bin/sh
+
+test $# -ge 2 || exit 1
+
Might want to emit a usage message there. Just a thought.
Ok. New version of the script:


#!/bin/sh

test $# -ge 2 || { echo "Syntax: $0 SRCTREE OBJTREE"; exit 1; }

# cd to objtree
cd "$2" || { echo "Syntax: $0 SRCTREE OBJTREE"; exit 1; }

srctree="$1"

find -type d | while read; do
d="$REPLY"

src="$srctree/$d/Kbuild.src"
dst="$d/Kbuild"
if test -f "$src"; then
echo " CHK $dst"

s=`sed -n 's@^//kbuild:@@p' "$srctree/$d"/*.c`
echo "# DO NOT EDIT. This file is generated from Kbuild.src" >"$dst.$$.tmp"
while read; do
test x"$REPLY" = x"INSERT" && REPLY="$s"
printf "%s\n" "$REPLY"
done <"$src" >>"$dst.$$.tmp"

if test -f "$dst" && cmp -s "$dst.$$.tmp" "$dst"; then
rm "$dst.$$.tmp"
else
echo " GEN $dst"
mv "$dst.$$.tmp" "$dst"
fi
fi

src="$srctree/$d/Config.src"
dst="$d/Config.in"
if test -f "$src"; then
echo " CHK $dst"

s=`sed -n 's@^//config:@@p' "$srctree/$d"/*.c`
echo "# DO NOT EDIT. This file is generated from Config.src" >"$dst.$$.tmp"
while read; do
test x"$REPLY" = x"INSERT" && REPLY="$s"
printf "%s\n" "$REPLY"
done <"$src" >>"$dst.$$.tmp"

if test -f "$dst" && cmp -s "$dst.$$.tmp" "$dst"; then
rm "$dst.$$.tmp"
else
echo " GEN $dst"
mv "$dst.$$.tmp" "$dst"
fi
fi

done

# Last read failed. This is normal. Don't exit with its error code:
exit 0
Rob Landley
2010-05-09 17:06:45 UTC
Permalink
Post by Denys Vlasenko
Post by Rob Landley
I note that CONFIG_APPNAME += appname.o is the common case, which could
presumably be generated automatically for 90% of the commands. (You only
really need to specify something like this when there's more than one .o
file, or if the .o file is a different name...)
Yes, de do have those 10%
Post by Rob Landley
Again, this looks like a default case. If you're going to do this for
more than one directory, possibly this should be emitted by the script
rather than appended to, and only non-default stuff should go in here.
The Config.src file had a reason to exist because it was naming the menu,
but this one isn't actually doing anything.
applets/ and archival/ have non-trivial Kbuilds
Oh sure. There would need to be a way to override the default case. (Perhaps
if there are no kbuild lines provide the default, if there are any don't.)
Post by Denys Vlasenko
Post by Rob Landley
Also, since these files are generated you don't really need one per
directory. You could just make one big one. (Which brings us back to the
generated directory, mentioned earlier.)
Still, that sort of thing could easily be done in stages. (Cleanup on
top of cleanup, this is fine for now...) The important thing is getting
the syntax that goes into the .c files right, so that conversion doesn't
have to be redone.
Right.
Post by Rob Landley
This rebuilds every time. Wouldn't it be better to do dependencies on
gen_build_files: findutils/*.c
gen_build_files: $(wildcard */*.c)
$ make
HOSTCC scripts/basic/fixdep
HOSTCC scripts/basic/split-include
HOSTCC scripts/basic/docproc
HOSTCC applets/usage
applets/usage.c:11:22: error: autoconf.h: No such file or directory
include/applets.h:70: error: expected ',' or ';' before 'IF_TEST'
make[2]: *** [applets/usage] Error 1
make[1]: *** [applets_dir] Error 2
make: *** [include/autoconf.h] Error 2
That's weird.

A) Why is it doing that? The *.c file exists. Presumably some other makefile
is indicating a strange prereqisite here that's getting triggered.

B) The applets directory hasn't actually got any applets in it, so the
dependency should exclude it. Using */*.c is fiddly because libbb and the
testsite and docs and examples and so on don't have applets in them. The
build logic for those other directories is who knows what, and those are most
likely to cause trouble.

Probably there should be some kind of APPLET_DIRS. I note that the top level
Makefile is setting libs-y to more or less the directory list we want, probably
what needs to happen is that list should first be saved in a variable
(containing all those dirs except libbb), and then libs-y should be set to
that variable (plus libbb).

Does that sound reasonable?
Post by Denys Vlasenko
Post by Rob Landley
Post by Denys Vlasenko
# To make sure we do not include .config for any of the *config
targets # catch them early, and hand them over to
scripts/kconfig/Makefile # It is allowed to specify more targets when
-include $(srctree)/arch/$(ARCH)/Makefile
export KBUILD_DEFCONFIG
-config %config: scripts_basic outputmakefile FORCE
+config %config: scripts_basic outputmakefile gen_build_files FORCE
$(Q)mkdir -p include
$(Q)$(MAKE) -C $(srctree) KBUILD_SRC= .kernelrelease
@@ -443,7 +448,7 @@ ifeq ($(KBUILD_EXTMOD),)
# Carefully list dependencies so we do not try to build scripts twice
# in parrallel
PHONY += scripts
-scripts: scripts_basic include/config/MARKER
+scripts: gen_build_files scripts_basic include/config/MARKER
scripts_basic: include/autoconf.h
It seems like that first hunk would have been sufficient, but the busybox
makefiles have always been a bit overcomplicated, haven't they?
(1) I don't know.
(2) Yes.
I hate makefiles. That's why I never got around to doing a serious crapectomy
on the busybox ones. (For my own projects, I tend to just have a shell script
drive the build...)

Still, it's a lot better than it was. The "30 seconds to figure out there's
nothing to do" days are gone, as is Vladimir's amazing segfaulting dependency
generator...
Post by Denys Vlasenko
Post by Rob Landley
Post by Denys Vlasenko
diff -ad -urpN busybox.0/scripts/gen_build_files.sh
busybox.1/scripts/gen_build_files.sh ---
busybox.0/scripts/gen_build_files.sh 1970-01-01
01:00:00.000000000 +0100 +++ busybox.1/scripts/gen_build_files.sh
+#!/bin/sh
+
+test $# -ge 2 || exit 1
+
Might want to emit a usage message there. Just a thought.
#!/bin/sh
test $# -ge 2 || { echo "Syntax: $0 SRCTREE OBJTREE"; exit 1; }
# cd to objtree
cd "$2" || { echo "Syntax: $0 SRCTREE OBJTREE"; exit 1; }
srctree="$1"
find -type d | while read; do
d="$REPLY"
src="$srctree/$d/Kbuild.src"
dst="$d/Kbuild"
if test -f "$src"; then
echo " CHK $dst"
Mixing spaces and tabs in the indent.
Post by Denys Vlasenko
echo "# DO NOT EDIT. This file is generated from Kbuild.src"
Post by Rob Landley
"$dst.$$.tmp" while read; do
test x"$REPLY" = x"INSERT" && REPLY="$s"
printf "%s\n" "$REPLY"
done <"$src" >>"$dst.$$.tmp"
if test -f "$dst" && cmp -s "$dst.$$.tmp" "$dst"; then
rm "$dst.$$.tmp"
else
echo " GEN $dst"
mv "$dst.$$.tmp" "$dst"
fi
fi
src="$srctree/$d/Config.src"
dst="$d/Config.in"
if test -f "$src"; then
echo " CHK $dst"
echo "# DO NOT EDIT. This file is generated from Config.src"
Post by Rob Landley
"$dst.$$.tmp" while read; do
test x"$REPLY" = x"INSERT" && REPLY="$s"
printf "%s\n" "$REPLY"
done <"$src" >>"$dst.$$.tmp"
if test -f "$dst" && cmp -s "$dst.$$.tmp" "$dst"; then
rm "$dst.$$.tmp"
else
echo " GEN $dst"
mv "$dst.$$.tmp" "$dst"
fi
fi
done
exit 0
Looks reasonable. More cleanup passes can always occur later...

Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
Denys Vlasenko
2010-05-10 00:30:25 UTC
Permalink
Post by Rob Landley
Post by Denys Vlasenko
Post by Rob Landley
It seems like that first hunk would have been sufficient, but the busybox
makefiles have always been a bit overcomplicated, haven't they?
(1) I don't know.
(2) Yes.
I hate makefiles.
Who doesn't?
--
vda
Peter Korsgaard
2010-05-10 06:35:17 UTC
Permalink
Post by Rob Landley
I hate makefiles.
Denys> Who doesn't?

Me.
--
Bye, Peter Korsgaard
Rob Landley
2010-05-10 17:51:40 UTC
Permalink
Post by Peter Korsgaard
Post by Rob Landley
I hate makefiles.
Denys> Who doesn't?
Me.
Stockholm syndrome.

Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
walter harms
2010-05-10 07:05:50 UTC
Permalink
Post by Denys Vlasenko
Post by Rob Landley
Post by Denys Vlasenko
Post by Rob Landley
It seems like that first hunk would have been sufficient, but the busybox
makefiles have always been a bit overcomplicated, haven't they?
(1) I don't know.
(2) Yes.
I hate makefiles.
Who doesn't?
me
Rob Landley
2010-05-14 06:22:45 UTC
Permalink
Post by Denys Vlasenko
Post by Rob Landley
I note that CONFIG_APPNAME += appname.o is the common case, which could
presumably be generated automatically for 90% of the commands. (You only
really need to specify something like this when there's more than one .o
file, or if the .o file is a different name...)
Yes, de do have those 10%
Post by Rob Landley
Again, this looks like a default case. If you're going to do this for
more than one directory, possibly this should be emitted by the script
rather than appended to, and only non-default stuff should go in here.
The Config.src file had a reason to exist because it was naming the menu,
but this one isn't actually doing anything.
applets/ and archival/ have non-trivial Kbuilds
Post by Rob Landley
Also, since these files are generated you don't really need one per
directory. You could just make one big one. (Which brings us back to the
generated directory, mentioned earlier.)
Still, that sort of thing could easily be done in stages. (Cleanup on
top of cleanup, this is fine for now...) The important thing is getting
the syntax that goes into the .c files right, so that conversion doesn't
have to be redone.
Right.
Post by Rob Landley
This rebuilds every time. Wouldn't it be better to do dependencies on
gen_build_files: findutils/*.c
gen_build_files: $(wildcard */*.c)
$ make
HOSTCC scripts/basic/fixdep
HOSTCC scripts/basic/split-include
HOSTCC scripts/basic/docproc
HOSTCC applets/usage
applets/usage.c:11:22: error: autoconf.h: No such file or directory
include/applets.h:70: error: expected ',' or ';' before 'IF_TEST'
make[2]: *** [applets/usage] Error 1
make[1]: *** [applets_dir] Error 2
make: *** [include/autoconf.h] Error 2
Post by Rob Landley
Post by Denys Vlasenko
# To make sure we do not include .config for any of the *config
targets # catch them early, and hand them over to
scripts/kconfig/Makefile # It is allowed to specify more targets when
-include $(srctree)/arch/$(ARCH)/Makefile
export KBUILD_DEFCONFIG
-config %config: scripts_basic outputmakefile FORCE
+config %config: scripts_basic outputmakefile gen_build_files FORCE
$(Q)mkdir -p include
$(Q)$(MAKE) -C $(srctree) KBUILD_SRC= .kernelrelease
@@ -443,7 +448,7 @@ ifeq ($(KBUILD_EXTMOD),)
# Carefully list dependencies so we do not try to build scripts twice
# in parrallel
PHONY += scripts
-scripts: scripts_basic include/config/MARKER
+scripts: gen_build_files scripts_basic include/config/MARKER
scripts_basic: include/autoconf.h
It seems like that first hunk would have been sufficient, but the busybox
makefiles have always been a bit overcomplicated, haven't they?
(1) I don't know.
(2) Yes.
Post by Rob Landley
Post by Denys Vlasenko
diff -ad -urpN busybox.0/scripts/gen_build_files.sh
busybox.1/scripts/gen_build_files.sh ---
busybox.0/scripts/gen_build_files.sh 1970-01-01
01:00:00.000000000 +0100 +++ busybox.1/scripts/gen_build_files.sh
+#!/bin/sh
+
+test $# -ge 2 || exit 1
+
Might want to emit a usage message there. Just a thought.
#!/bin/sh
test $# -ge 2 || { echo "Syntax: $0 SRCTREE OBJTREE"; exit 1; }
# cd to objtree
cd "$2" || { echo "Syntax: $0 SRCTREE OBJTREE"; exit 1; }
srctree="$1"
find -type d | while read; do
d="$REPLY"
To get this technique to stop modifying the data going through it, you have to
export IFS="$(echo -e "\n")" to get it to stop trimming leading spaces, and
then you have to feed -r to the read command to get it to stop doing this:

+ PATH="$(pwd)/distcc_links" "$(which distccd)" --no-detach --daemon --
listen 127.0.0.1 -a 127.0.0.1 -p $PORT --jobs $CPUS --log-stderr --verbose
2>distccd.log &
- PATH="$(pwd)/distcc_links" "$(which distccd)" --no-detach --daemon \
- --listen 127.0.0.1 -a 127.0.0.1 -p $PORT --jobs $CPUS \
- --log-stderr --verbose 2>distccd.log &

+DISTCC_PATH="$(which $ARCH-cc 2>/dev/null | sed 's@(.*)/.*@1@')"
-DISTCC_PATH="$(which $ARCH-cc 2>/dev/null | sed 's@\(.*\)/.*@\1@')"

FYI.

Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
Timo Teräs
2010-05-14 07:25:42 UTC
Permalink
Post by Denys Vlasenko
Post by Rob Landley
I note that CONFIG_APPNAME += appname.o is the common case, which could
presumably be generated automatically for 90% of the commands. (You only
really need to specify something like this when there's more than one .o
file, or if the .o file is a different name...)
Yes, de do have those 10%
Post by Rob Landley
Again, this looks like a default case. If you're going to do this for
more than one directory, possibly this should be emitted by the script
rather than appended to, and only non-default stuff should go in here.
The Config.src file had a reason to exist because it was naming the menu,
but this one isn't actually doing anything.
applets/ and archival/ have non-trivial Kbuilds
Post by Rob Landley
Also, since these files are generated you don't really need one per
directory. You could just make one big one. (Which brings us back to the
generated directory, mentioned earlier.)
Still, that sort of thing could easily be done in stages. (Cleanup on
top of cleanup, this is fine for now...) The important thing is getting
the syntax that goes into the .c files right, so that conversion doesn't
have to be redone.
Right.
While at it, we could get rid of include/applets.h too. One very nice
way would be to use linker section. You can use gcc __attribute__
section("sectionname") [1] to put the applet definitions to separate
memory area. It is after that possible to iterate over the structures
in that special section using __start_sectionname and __stop_sectionname
symbols which are autocreated [2]. This is also roughly how Linux kernel
initcalls work [3].

Now it appears that the applets need to be sorted for binary search.
I haven't actually tried this, but I think linker could probably do the
job. Normally, the section symbols get the order the object files
are given to ld, but it is also possible to have the linker sort the
filenames alphabetically [4, 5]. This of course assumes that each applet
is in it's own object file, and that the object file name sorting order
is the same as applet name sorting order.

This obviously depends on GCC specific attribute, and GNU LD specific
linking. I'm not sure how well this is supported by other compilers.

Would this be useful approach for busybox? Or do we want to be more
generic and possibly just create some scripts to autogenerate the
applet.h? Other ideas?

Just a thought.

Cheers,
Timo

[1] http://gcc.gnu.org/onlinedocs/gcc-4.4.3/gcc/Variable-Attributes.html
[2] http://www.airs.com/blog/archives/56
[3] http://kernelnewbies.org/InitcallMechanism/HowItWorks
[4] http://www.delorie.com/gnu/docs/binutils/ld_23.html
[5] http://www.delorie.com/gnu/docs/binutils/ld_24.html
Rob Landley
2010-05-14 09:33:09 UTC
Permalink
Post by Timo Teräs
Post by Denys Vlasenko
Post by Rob Landley
I note that CONFIG_APPNAME += appname.o is the common case, which could
presumably be generated automatically for 90% of the commands. (You
only really need to specify something like this when there's more than
one .o file, or if the .o file is a different name...)
Yes, de do have those 10%
Post by Rob Landley
Again, this looks like a default case. If you're going to do this for
more than one directory, possibly this should be emitted by the script
rather than appended to, and only non-default stuff should go in here.
The Config.src file had a reason to exist because it was naming the
menu, but this one isn't actually doing anything.
applets/ and archival/ have non-trivial Kbuilds
Post by Rob Landley
Also, since these files are generated you don't really need one per
directory. You could just make one big one. (Which brings us back to
the generated directory, mentioned earlier.)
Still, that sort of thing could easily be done in stages. (Cleanup on
top of cleanup, this is fine for now...) The important thing is
getting the syntax that goes into the .c files right, so that
conversion doesn't have to be redone.
Right.
While at it, we could get rid of include/applets.h too. One very nice
way would be to use linker section. You can use gcc __attribute__
section("sectionname") [1] to put the applet definitions to separate
memory area. It is after that possible to iterate over the structures
in that special section using __start_sectionname and __stop_sectionname
symbols which are autocreated [2]. This is also roughly how Linux kernel
initcalls work [3].
Actually we have vague plans to generate that in a similar manner.

Linker magic is a bit non-obvious for my tastes, and also won't be able to
automatically alphebetize them and thus lend itself to a binary search,
reducing launch latencies in scripts...
Post by Timo Teräs
Now it appears that the applets need to be sorted for binary search.
I haven't actually tried this, but I think linker could probably do the
job. Normally, the section symbols get the order the object files
are given to ld, but it is also possible to have the linker sort the
filenames alphabetically [4, 5]. This of course assumes that each applet
is in it's own object file, and that the object file name sorting order
is the same as applet name sorting order.
This obviously depends on GCC specific attribute, and GNU LD specific
linking. I'm not sure how well this is supported by other compilers.
With pcc and llvm getting more interesting, I'd rather _not_ add anything gnu
specific. (One of the things I like about busybox is the ability to wean a
system off GNU insanity.)
Post by Timo Teräs
Would this be useful approach for busybox? Or do we want to be more
generic and possibly just create some scripts to autogenerate the
applet.h? Other ideas?
Script to autogenerate applet.h is what I did for toybox back in 2007.

Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
Timo Teräs
2010-05-14 09:58:53 UTC
Permalink
Post by Rob Landley
Post by Timo Teräs
While at it, we could get rid of include/applets.h too. One very nice
way would be to use linker section. You can use gcc __attribute__
section("sectionname") [1] to put the applet definitions to separate
memory area. It is after that possible to iterate over the structures
in that special section using __start_sectionname and __stop_sectionname
symbols which are autocreated [2]. This is also roughly how Linux kernel
initcalls work [3].
Actually we have vague plans to generate that in a similar manner.
Linker magic is a bit non-obvious for my tastes, and also won't be able to
automatically alphebetize them and thus lend itself to a binary search,
reducing launch latencies in scripts...
Like the immediate text says, it's possible for linker to do the
sorting, but with some limitations.
Post by Rob Landley
Post by Timo Teräs
Now it appears that the applets need to be sorted for binary search.
I haven't actually tried this, but I think linker could probably do the
job. Normally, the section symbols get the order the object files
are given to ld, but it is also possible to have the linker sort the
filenames alphabetically [4, 5]. This of course assumes that each applet
is in it's own object file, and that the object file name sorting order
is the same as applet name sorting order.
This obviously depends on GCC specific attribute, and GNU LD specific
linking. I'm not sure how well this is supported by other compilers.
With pcc and llvm getting more interesting, I'd rather _not_ add anything gnu
specific. (One of the things I like about busybox is the ability to wean a
system off GNU insanity.)
I consider linker sections very convenient. And those are requirement
to build bootloader or kernel on most platforms. I would assume that
llvm and e.g. icc would support this, but have not double checked.
And as said, linux kernel does use this, so any viable Linux compiler
should be supporting these. Then again, it is nice to be able to
run bb on targets which is not supported by gcc.

I do also think that it's cleaner if we have less build scripts, and the
makefile dependencies are simpler. The generator script for applet.h can
be slightly complicated on that front.

Both ways do need documentation, so I'm not sure if either of them is
more easier to maintain than the other.

But this said, yes it's still trading portability with convenience. And
we probably want to stay as portable as possible, so the linker magic
would get ruled out due to that.
Post by Rob Landley
Post by Timo Teräs
Would this be useful approach for busybox? Or do we want to be more
generic and possibly just create some scripts to autogenerate the
applet.h? Other ideas?
Script to autogenerate applet.h is what I did for toybox back in 2007.
As said, I'd be equally happy with applet.h being autogenerated. :)

- Timo
Denys Vlasenko
2010-05-14 18:34:03 UTC
Permalink
Post by Timo Teräs
While at it, we could get rid of include/applets.h too. One very nice
way would be to use linker section. You can use gcc __attribute__
section("sectionname") [1] to put the applet definitions to separate
memory area. It is after that possible to iterate over the structures
in that special section using __start_sectionname and __stop_sectionname
symbols which are autocreated [2]. This is also roughly how Linux kernel
initcalls work [3].
This will break ld --gc-sections.
--
vda
Timo Teräs
2010-05-14 20:54:45 UTC
Permalink
Post by Denys Vlasenko
Post by Timo Teräs
While at it, we could get rid of include/applets.h too. One very nice
way would be to use linker section. You can use gcc __attribute__
section("sectionname") [1] to put the applet definitions to separate
memory area. It is after that possible to iterate over the structures
in that special section using __start_sectionname and __stop_sectionname
symbols which are autocreated [2]. This is also roughly how Linux kernel
initcalls work [3].
This will break ld --gc-sections.
I think ld handles --gc-sections properly if __start_sectionname symbol
is referenced. If not, using __attribute__((used)) should work.

- Timo
Rob Landley
2010-05-15 02:39:39 UTC
Permalink
Post by Timo Teräs
Post by Denys Vlasenko
Post by Timo Teräs
While at it, we could get rid of include/applets.h too. One very nice
way would be to use linker section. You can use gcc __attribute__
section("sectionname") [1] to put the applet definitions to separate
memory area. It is after that possible to iterate over the structures
in that special section using __start_sectionname and __stop_sectionname
symbols which are autocreated [2]. This is also roughly how Linux kernel
initcalls work [3].
This will break ld --gc-sections.
I think ld handles --gc-sections properly if __start_sectionname symbol
is referenced. If not, using __attribute__((used)) should work.
- Timo
Having a script create a human readable source file is something you could
explain to a middle school student with two weeks experience with C. The
tools to do so are standardized by posix, and the versions busybox itself
produces support all necessary functionality.

Having a magic linker function use vendor-specific extensions in a manner only
supported on certain toolchain versions is not just non-obvious, but you
yourself aren't sure off the top of your head how you'd have to go about
implementing it, and the explanation you just tried to give is full of non-
obvious conditional requirements.

Given that the two techniqes produce equivalent results, this doesn't look
like a hard decision...

Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
Timo Teräs
2010-05-15 06:20:09 UTC
Permalink
Post by Rob Landley
Post by Timo Teräs
Post by Denys Vlasenko
Post by Timo Teräs
While at it, we could get rid of include/applets.h too. One very nice
way would be to use linker section. You can use gcc __attribute__
section("sectionname") [1] to put the applet definitions to separate
memory area. It is after that possible to iterate over the structures
in that special section using __start_sectionname and __stop_sectionname
symbols which are autocreated [2]. This is also roughly how Linux kernel
initcalls work [3].
This will break ld --gc-sections.
I think ld handles --gc-sections properly if __start_sectionname symbol
is referenced. If not, using __attribute__((used)) should work.
Having a script create a human readable source file is something you could
explain to a middle school student with two weeks experience with C. The
tools to do so are standardized by posix, and the versions busybox itself
produces support all necessary functionality.
Having a magic linker function use vendor-specific extensions in a manner only
supported on certain toolchain versions is not just non-obvious, but you
yourself aren't sure off the top of your head how you'd have to go about
implementing it, and the explanation you just tried to give is full of non-
obvious conditional requirements.
The above was just to explain that it is supported and well thought
mechanism, even if I don't happen to know all the details. Intention was
to point that --gc-section is not a problem. But as I already previously
agreed, portability is. And so is the sorting issue if we want to do
funny sorting or have multiple applets in one file.
Post by Rob Landley
Given that the two techniqes produce equivalent results, this doesn't look
like a hard decision...
Yes, I've already agreed on this. Instead of flaming the idea, could we
move on and concentrate on getting the script done?
Cristian Ionescu-Idbohrn
2010-05-14 15:46:54 UTC
Permalink
Post by Rob Landley
To get this technique to stop modifying the data going through it, you
have to export IFS="$(echo -e "\n")" to get it to stop trimming leading
spaces,
Please don't do that. It's not portable. With dash being the default
shell on ubuntu, on the way to become the default shell on debian too,
followed soonish by all other debian derivatives and probably other
distributions, you'll get a broken IFS. Like:

00000000 2d 65 20 |-e |

If resetting IFS is important, please do it like this instead:

IFS='
'

You'll get what you want:

00000000 0a |.|

without breaking anything.


Cheers,
--
Cristian
Rob Landley
2010-05-14 20:44:33 UTC
Permalink
Post by Cristian Ionescu-Idbohrn
Post by Rob Landley
To get this technique to stop modifying the data going through it, you
have to export IFS="$(echo -e "\n")" to get it to stop trimming leading
spaces,
Please don't do that. It's not portable. With dash being the default
shell on ubuntu, on the way to become the default shell on debian too,
followed soonish by all other debian derivatives and probably other
00000000 2d 65 20 |-e |
IFS='
'
00000000 0a |.|
without breaking anything.
Cheers,
Or just #!/bin/bash and teach hush to eventually replace bash.

Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
Cristian Ionescu-Idbohrn
2010-05-15 01:13:09 UTC
Permalink
Post by Rob Landley
Post by Cristian Ionescu-Idbohrn
IFS='
'
00000000 0a |.|
without breaking anything.
Cheers,
Or just #!/bin/bash and teach hush to eventually replace bash.
Neither 'or' nor 'and'. The way to set IFS to <NL>, regardless shell
name, was, is and will (most likely) be:

IFS='
'


Cheers,
--
Cristian
walter harms
2010-05-15 08:21:07 UTC
Permalink
Post by Cristian Ionescu-Idbohrn
Post by Rob Landley
To get this technique to stop modifying the data going through it, you
have to export IFS="$(echo -e "\n")" to get it to stop trimming leading
spaces,
Please don't do that. It's not portable. With dash being the default
shell on ubuntu, on the way to become the default shell on debian too,
followed soonish by all other debian derivatives and probably other
00000000 2d 65 20 |-e |
IFS='
'
00000000 0a |.|
without breaking anything.
That will make it unreadable due to invisible chars. what is about:
IFS=$( printf "\n" )

printf is posix and should be supported by dash.

re,
wh
Cristian Ionescu-Idbohrn
2010-05-15 17:59:38 UTC
Permalink
Post by walter harms
Post by Cristian Ionescu-Idbohrn
IFS='
'
00000000 0a |.|
without breaking anything.
That will make it unreadable due to invisible chars.
It's readable alright, IMO. Just look for the closing quote character.
Post by walter harms
IFS=$( printf "\n" )
printf is posix and should be supported by dash.
Yes, printf is posix, supported by bash, dash and whatnot.

But, even if it did work (did you test? what shell?), forking a subshell
just to get an LF character stashed into a variable, is way too expensive.


Cheers,
--
Cristian
walter harms
2010-05-16 14:51:04 UTC
Permalink
Post by Cristian Ionescu-Idbohrn
Post by walter harms
Post by Cristian Ionescu-Idbohrn
IFS='
'
00000000 0a |.|
without breaking anything.
That will make it unreadable due to invisible chars.
It's readable alright, IMO. Just look for the closing quote character.
Post by walter harms
IFS=$( printf "\n" )
printf is posix and should be supported by dash.
Yes, printf is posix, supported by bash, dash and whatnot.
But, even if it did work (did you test? what shell?), forking a subshell
just to get an LF character stashed into a variable, is way too expensive.
I does not matter, it simply addresse the problem:
1. avoid broken shells with "echo -e" missing/broken
2. avoid non-printable characters while setting a variable
3. use existing posix cmds

when you have a time problem that can be fixed using
IFS='
'
its fine, but if you can not spend the time for a printf(1) you may
have a different type of problem.

re,
wh
Rob Landley
2010-05-16 23:40:28 UTC
Permalink
Post by Cristian Ionescu-Idbohrn
But, even if it did work (did you test? what shell?),
Me, on bash. And the point of the quotes around the "$()" is to preserve
whitespace in the output. (Alas, it doesn't preserve embedded NUL bytes, and
yes I did test that.)
Post by Cristian Ionescu-Idbohrn
forking a subshell
just to get an LF character stashed into a variable, is way too expensive.
You're about to launch gcc, and forking a subshell is too expensive?

I think you've officially passed into la-la land with that remark.

Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
Denys Vlasenko
2010-05-14 18:25:25 UTC
Permalink
Post by Rob Landley
Post by Denys Vlasenko
Post by Rob Landley
I note that CONFIG_APPNAME += appname.o is the common case, which could
presumably be generated automatically for 90% of the commands. ?(You only
really need to specify something like this when there's more than one .o
file, or if the .o file is a different name...)
Yes, de do have those 10%
Post by Rob Landley
Again, this looks like a default case. ?If you're going to do this for
more than one directory, possibly this should be emitted by the script
rather than appended to, and only non-default stuff should go in here.
The Config.src file had a reason to exist because it was naming the menu,
but this one isn't actually doing anything.
applets/ and archival/ have non-trivial Kbuilds
Post by Rob Landley
Also, since these files are generated you don't really need one per
directory. You could just make one big one. ?(Which brings us back to the
generated directory, mentioned earlier.)
Still, that sort of thing could easily be done in stages. ?(Cleanup on
top of cleanup, this is fine for now...) ?The important thing is getting
the syntax that goes into the .c files right, so that conversion doesn't
have to be redone.
Right.
Post by Rob Landley
This rebuilds every time. ?Wouldn't it be better to do dependencies on
? gen_build_files: findutils/*.c
gen_build_files: $(wildcard */*.c)
$ make
? HOSTCC ?scripts/basic/fixdep
? HOSTCC ?scripts/basic/split-include
? HOSTCC ?scripts/basic/docproc
? HOSTCC ?applets/usage
applets/usage.c:11:22: error: autoconf.h: No such file or directory
include/applets.h:70: error: expected ',' or ';' before 'IF_TEST'
make[2]: *** [applets/usage] Error 1
make[1]: *** [applets_dir] Error 2
make: *** [include/autoconf.h] Error 2
Post by Rob Landley
?# To make sure we do not include .config for any of the *config
targets # catch them early, and hand them over to
scripts/kconfig/Makefile # It is allowed to specify more targets when
?-include $(srctree)/arch/$(ARCH)/Makefile
?export KBUILD_DEFCONFIG
-config %config: scripts_basic outputmakefile FORCE
+config %config: scripts_basic outputmakefile gen_build_files FORCE
? ? ? ? $(Q)mkdir -p include
? ? ? ? $(Q)$(MAKE) -C $(srctree) KBUILD_SRC= .kernelrelease
@@ -443,7 +448,7 @@ ifeq ($(KBUILD_EXTMOD),)
?# Carefully list dependencies so we do not try to build scripts twice
?# in parrallel
?PHONY += scripts
-scripts: scripts_basic include/config/MARKER
+scripts: gen_build_files scripts_basic include/config/MARKER
?scripts_basic: include/autoconf.h
It seems like that first hunk would have been sufficient, but the busybox
makefiles have always been a bit overcomplicated, haven't they?
(1) I don't know.
(2) Yes.
Post by Rob Landley
diff -ad -urpN busybox.0/scripts/gen_build_files.sh
busybox.1/scripts/gen_build_files.sh ---
busybox.0/scripts/gen_build_files.sh ? ? ? ?1970-01-01
01:00:00.000000000 +0100 +++ busybox.1/scripts/gen_build_files.sh
+#!/bin/sh
+
+test $# -ge 2 || exit 1
+
Might want to emit a usage message there. ?Just a thought.
#!/bin/sh
test $# -ge 2 || { echo "Syntax: $0 SRCTREE OBJTREE"; exit 1; }
# cd to objtree
cd "$2" || { echo "Syntax: $0 SRCTREE OBJTREE"; exit 1; }
srctree="$1"
find -type d | while read; do
? ? d="$REPLY"
To get this technique to stop modifying the data going through it, you have to
export IFS="$(echo -e "\n")" to get it to stop trimming leading spaces, and
Tweaking IFS is not needed.

I did discover a lot of interesting details about shells
while hacking on them. One is that bare "read"
is NOT the same thing as "read REPLY"! Look at this:

$ echo " foo b\ar " | { read -r; echo "[$REPLY]"; }
[ foo b\ar ]
$ echo " foo b\ar " | { read -r REPLY; echo "[$REPLY]"; }
[foo b\ar]

(I will add -r, thanks)
--
vda
Cristian Ionescu-Idbohrn
2010-05-14 19:44:52 UTC
Permalink
Post by Denys Vlasenko
I did discover a lot of interesting details about shells
while hacking on them. One is that bare "read"
$ echo " foo b\ar " | { read -r; echo "[$REPLY]"; }
[ foo b\ar ]
$ dash -c 'echo " foo b\ar " | { read -r; echo "[$REPLY]"; }'
read: 1: arg count
[]
Post by Denys Vlasenko
$ echo " foo b\ar " | { read -r REPLY; echo "[$REPLY]"; }
[foo b\ar]
$ dash -c 'echo " foo b\ar " | { read -r REPLY; echo "[$REPLY]"; }'
[foo br]

(with a bell ringing)
Post by Denys Vlasenko
(I will add -r, thanks)
Be careful out there :)


Cheers,
--
Cristian
Denys Vlasenko
2010-05-15 15:01:51 UTC
Permalink
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
I did discover a lot of interesting details about shells
while hacking on them. One is that bare "read"
$ echo " foo b\ar " | { read -r; echo "[$REPLY]"; }
[ foo b\ar ]
$ dash -c 'echo " foo b\ar " | { read -r; echo "[$REPLY]"; }'
read: 1: arg count
[]
They are not bash compat :)
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
$ echo " foo b\ar " | { read -r REPLY; echo "[$REPLY]"; }
[foo b\ar]
$ dash -c 'echo " foo b\ar " | { read -r REPLY; echo "[$REPLY]"; }'
[foo br]
(with a bell ringing)
Bug per http://www.opengroup.org/onlinepubs/009695399/utilities/read.html
--
vda
Cristian Ionescu-Idbohrn
2010-05-15 17:57:49 UTC
Permalink
Post by Denys Vlasenko
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
I did discover a lot of interesting details about shells
while hacking on them. One is that bare "read"
$ echo " foo b\ar " | { read -r; echo "[$REPLY]"; }
[ foo b\ar ]
$ dash -c 'echo " foo b\ar " | { read -r; echo "[$REPLY]"; }'
read: 1: arg count
[]
They are not bash compat :)
Pick any one shell as _the_ reference shell and compare it with all other
major shells. The in"compat"ibilities (bash or not bash) will be
noticable.
Post by Denys Vlasenko
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
$ echo " foo b\ar " | { read -r REPLY; echo "[$REPLY]"; }
[foo b\ar]
$ dash -c 'echo " foo b\ar " | { read -r REPLY; echo "[$REPLY]"; }'
[foo br]
(with a bell ringing)
Bug per http://www.opengroup.org/onlinepubs/009695399/utilities/read.html
Yes. And there's still a lot of room for interpretation:

-r
Do not treat a backslash character in any special way. Consider each
backslash to be part of the input line.

But what is an input line?

Is this an input line?

bash$ read -r REPLY <<EOF
Post by Denys Vlasenko
foo b\ar
EOF
bash$ echo "[$REPLY]"
[foo b\ar]

Is this an input line too?

bash$ echo " foo b\ar " | { read REPLY; echo "[$REPLY]"; }
[foo bar]

Busybox ash imitating bash:

ash$ read -r REPLY <<EOF
Post by Denys Vlasenko
foo b\ar
EOF
ash$ echo "[$REPLY]"
[foo b\ar]

ash$ echo " foo b\ar " | { read REPLY; echo "[$REPLY]"; }
[foo bar]

What about this?

dash$ read -r REPLY <<EOF
Post by Denys Vlasenko
foo b\ar
EOF
dash$ echo "[$REPLY]"
[foo br]

dash$ echo " foo b\ar " | { read REPLY; echo "[$REPLY]"; }
[foo br]

(rings the bell in both cases)


But I thought we were discussing the bash REPLY obfuscatin, did we not?


Cheers,
--
Cristian
Denys Vlasenko
2010-05-15 18:57:53 UTC
Permalink
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
I did discover a lot of interesting details about shells
while hacking on them. One is that bare "read"
$ echo " foo b\ar " | { read -r; echo "[$REPLY]"; }
[ foo b\ar ]
$ dash -c 'echo " foo b\ar " | { read -r; echo "[$REPLY]"; }'
read: 1: arg count
[]
They are not bash compat :)
Pick any one shell as _the_ reference shell and compare it with all other
major shells. The in"compat"ibilities (bash or not bash) will be
noticable.
But bash is de-facto standard Linux shell. Not ksh or zsh.
Therefore, it makes sense to match bash behavior
where it does not require major coding.

Sure, I can stop using "read" without parameter.
But this does not affect many other shell scripts out there -
hundreds or thousands of them use this construct.

If, instead, we patch dash to understand "read" without parameter
(which can't be too hard), all those scripts start working.
Post by Cristian Ionescu-Idbohrn
But what is an input line?
Is this an input line?
bash$ read -r REPLY <<EOF
Post by Denys Vlasenko
foo b\ar
EOF
bash$ echo "[$REPLY]"
[foo b\ar]
Yes.
Post by Cristian Ionescu-Idbohrn
Is this an input line too?
bash$ echo " foo b\ar " | { read REPLY; echo "[$REPLY]"; }
[foo bar]
Yes.
Post by Cristian Ionescu-Idbohrn
What about this?
dash$ read -r REPLY <<EOF
Post by Denys Vlasenko
foo b\ar
EOF
dash$ echo "[$REPLY]"
[foo br]
Bug.
--
vda
Cristian Ionescu-Idbohrn
2010-05-15 19:28:11 UTC
Permalink
Post by Denys Vlasenko
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
They are not bash compat :)
Pick any one shell as _the_ reference shell and compare it with all
other major shells. The in"compat"ibilities (bash or not bash) will
be noticable.
But bash is de-facto standard Linux shell.
Was. Things change. Several distributions made the choice to replace
bash with dash. I was not involved in that process. I'll have to live
with it. Mind you, I don't necessarily think it's a bad decision.
Post by Denys Vlasenko
Not ksh or zsh.
True. Still, dash came along to become the default shell. And the
busybox build scripts are now broken. And that makes a lot of people
unhappy.
Post by Denys Vlasenko
Therefore, it makes sense to match bash behavior
where it does not require major coding.
Might.
Post by Denys Vlasenko
Sure, I can stop using "read" without parameter.
Please do. And please also change the shebang line on bash scripts.
That _only_ will improve things.
Post by Denys Vlasenko
But this does not affect many other shell scripts out there -
hundreds or thousands of them use this construct.
Yes. But that doesn't change the standard.
Post by Denys Vlasenko
If, instead, we patch dash to understand "read" without parameter
(which can't be too hard), all those scripts start working.
Sure. This is the dash upstream mailinglist address:

dash at vger.kernel.org

I'll avoid crossposting.

It would be great if people could come up to an agreement that serves
the user base.


Cheers,
--
Cristian
Denys Vlasenko
2010-05-16 00:41:44 UTC
Permalink
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
They are not bash compat :)
Pick any one shell as _the_ reference shell and compare it with all
other major shells. The in"compat"ibilities (bash or not bash) will
be noticable.
But bash is de-facto standard Linux shell.
Was. Things change. Several distributions made the choice to replace
bash with dash. I was not involved in that process. I'll have to live
with it. Mind you, I don't necessarily think it's a bad decision.
It's a good decision *if* dash developers will not jump on the opportunity
to become assholes, having such a large group of people their hostages now.

Seeing an email in dash archives where one developer says "standards
only require that redirections support fds 0..9", I am not very optimistic.
--
vda
Cristian Ionescu-Idbohrn
2010-05-16 14:52:21 UTC
Permalink
Post by Denys Vlasenko
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
They are not bash compat :)
Pick any one shell as _the_ reference shell and compare it with
all other major shells. The in"compat"ibilities (bash or not
bash) will be noticable.
But bash is de-facto standard Linux shell.
Was. Things change. Several distributions made the choice to replace
bash with dash. I was not involved in that process. I'll have to
live with it. Mind you, I don't necessarily think it's a bad
decision.
It's a good decision *if* dash developers will not jump on the
opportunity to become assholes, having such a large group of people
their hostages now.
True.
Post by Denys Vlasenko
Seeing an email in dash archives where one developer says "standards
only require that redirections support fds 0..9", I am not very optimistic.
I do share your worries, indeed.

If that attitude continues, I would not rule out the risk we may see a
fork of dash, sometime in a not too distant future. That said, being
somewhat restrictive with feature bloat is not bad.


Cheers,
--
Cristian
Rob Landley
2010-05-17 00:10:50 UTC
Permalink
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
They are not bash compat :)
Pick any one shell as _the_ reference shell and compare it with all
other major shells. The in"compat"ibilities (bash or not bash) will
be noticable.
But bash is de-facto standard Linux shell.
Was. Things change. Several distributions made the choice to replace
bash with dash. I was not involved in that process. I'll have to live
with it. Mind you, I don't necessarily think it's a bad decision.
For a definition of "several" meaning "one did it, and one is considering it".

None of them removed bash from their base install. They bloated the system by
adding added another shell. The reasons they did so are documented here, and
are really stupid:

https://wiki.ubuntu.com/DashAsBinSh

Their boot scripts were running too slowly, so instead of A) fixing their boot
scripts to not be horribly bloated, B) changing their boot scripts to say
#!/bin/dash instead of #!/bin/sh, they instead changed #!/bin/sh to point to a
shell that (at the time) didn't even _WORK_, and which broke hundreds of
package builds (including the Linux kernel). And don't get me started abouut
the way it was used as the default LOGIN shell for new users back when it
DIDN'T HAVE LINE EDITING SUPPORT. (They reverted that, /bin/bash is now the
default login shell.)

Then they A) created upstart and B) ported a lot of the boot logic from
scripts to C, rendering their original reason for introducing dash completely
moot. But they can't admit a mistake. I've gotten Ubuntu engineers to admit
in _person_ that it was a horrible mistake, but Cannonical spent a lot of
political capital shoved this down people's throats and they are resolutely
not looking back.

The reason debian is copying Ubuntu is that Ubuntu has both more users and
more developers than Debian does, and the tail is very definitely wagging the
dog there. Debian is chasing Ubuntu's tail so as not to be rendered
completely irrelevant. (Again.) And if that means swallowing questionable
technical decisions, so be it.
Post by Cristian Ionescu-Idbohrn
Post by Denys Vlasenko
Not ksh or zsh.
True. Still, dash came along to become the default shell.
The default of who? Fedora? SuSE? Gentoo?
Post by Cristian Ionescu-Idbohrn
And the
busybox build scripts are now broken. And that makes a lot of people
unhappy.
1) Explicitly saying #!/bin/bash fixes that.

2) As with the distros, "A lot of people" would apparently be "you" in this
instance.

Rob
--
Latency is more important than throughput. It's that simple. - Linus Torvalds
Continue reading on narkive:
Loading...