[PATCH 2/2] Split bash compatible extensions into separate options.

Denys Vlasenko vda.linux at googlemail.com
Wed Jan 11 13:02:25 UTC 2017


On Wed, Jan 11, 2017 at 12:32 PM, Kang-Che Sung <explorer09 at gmail.com> wrote:
> On Wed, Jan 11, 2017 at 4:09 PM, Denys Vlasenko
> <vda.linux at googlemail.com> wrote:
>> On Tue, Jan 10, 2017 at 8:17 PM, Kang-Che Sung <explorer09 at gmail.com> wrote:
>>> All of the bash compatibility options will now depend on
>>> CONFIG_*_BASH_COMPAT, and CONFIG_*_BASH_COMPAT become an option that
>>> alone doesn't add any code.
>>>
>>> Splitting these options allows more flexibility in configuration
>>
>> Configuration shouldn't strive to be maximally flexible.
>>
>> It floods the user with way too many options.
>> For example, now LS has eight suboptions.
>> Imagine this instead:
>>
>> [*] ls
>> [*]   Enable -1    One column output
>> [*]   Enable -a    Include entries which start with .
>> [*]   Enable -A    Like -a, but exclude . and ..
>> [*]   Enable -C    List by columns
>> [*]   Enable -x    List by lines
>> [*]   Enable -d    List directory entries instead of contents
>> [*]   Enable -L    Follow symlinks
>> [*]   Enable -H    Follow symlinks on command line
>> [*]   Enable -R    Recurse
>> [*]   Enable -p    Append / to dir entries
>> [*]   Enable -F    Append indicator (one of */=@|) to entries
>> [*]   Enable -l    Long listing format
>> [*]     Enable -c  sort by ctime
>> [*]     Enable -t  sort by mtime
>> [*]     Enable -u  sort by atime
>> [*]   Enable -i    List inode numbers
>> [*]   Enable -n    List numeric UIDs and GIDs instead of names
>> [*]   Enable -s    List allocated blocks
>> [*]   Enable -e    List full date and time
>> [*]   Enable -h    List sizes in human readable format (1K 243M 2G)
>> [*]   Enable -r    Sort in reverse order
>> [*]   Enable -S    Sort by size
>> [*]   Enable -X    Sort by extension
>> [*]   Enable -v    Sort by version
>> [*]   Enable -w N  Assume the terminal is N columns wide
>> [*]   Enable --color
>> [*]     Produce colored ls output by default
>>
>> For every applet and every option. That would be ridiculous.
>>
>> Ridiculous we can live with, but there is a bigger problem too.
>> More options means more #ifdefs and more weird interactions
>> between different options, making code less maintainable.
>> Because of this reason, some options (e.g. in top) were even
>> _removed_ (permanently enabled) because code was too tangled.
>>
>> therefore options should be introduced if there is a reasonable
>> chance user actually needs to control them separately,
>> and if they control a nontrivial amount of code (i.e. not 20 bytes).
>>
>> If user needs bash compat, it means he is not running on an extremely
>> constrained machine (otherwise he can well spend a bit of time and rewrite
>> all his scripts to be POSIX-compatible). Since the machine is not
>> too constrained, it's okay to just enable all of them and not worry
>> about having a few more kbytes of code for a bashism he doesn't need.
>
> I don't buy your argument, unfortunately.
>
> My points:
>
> 1. I can read if any of the shell's feature is too trivial that it'll
> be not worth
> implementing or making a configure option at all. The bash-compatible
> 'source' command behavior, that I suggested you to revert, is one of the
> example. (busybox ash implemented that wrong

It implemented is just like bash used to do it back then.
Since bash reverted its behavior back to standard,
we should do so, so we did.
It has nothing to do with config options either way.

> 3. If people don't like too many Kconfig options, they don't have to.
> Nothing stops you from making them in-C macros instead:
>
>     #if ENABLE_ASH_BASH_COMPAT
>     # define ENABLE_ASH_DOLLAR_SQUOTE    /* $'...' */
>     # define ENABLE_ASH_SUBSTR_EXPANSION /* ${var:pos:len} */
>     # define ENABLE_ASH_PATTERN_SUBST    /* ${var/pattern/repl} */
>     # define ENABLE_ASH_REDIR_OUTPUT2    /* &>file ; >&file */
>     # define ENABLE_ASH_FUNCTION2        /* function keyword */
>     # define ENABLE_ASH_SOURCE           /* source keyword */
>     # define ENABLE_ASH_TEST2            /* [[ EXPR ]] */
>     # define ENABLE_ASH_OPT_PIPEFAIL     /* -o pipefail */
>     # define ENABLE_ASH_HOSTNAME_VAR     /* $HOSTNAME */
>     # define ENABLE_ASH_SHLVL_VAR        /* $SHLVL */
>     #endif

This makes sense, applied in this form, please check current git
for ash.c part.


More information about the busybox mailing list