config-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] Use first form of the test command


From: Issam E. Maghni
Subject: Re: [PATCH] Use first form of the test command
Date: Mon, 17 Aug 2020 05:22:21 +0200 (CEST)

> Can you elaborate?
Sure!

These are my motivations behind the patch.

1) Consistency
As Karl Berry stated [1],
> it's common practice in the basic utility scripts to use test
Also, using both '[' and 'test' in the same file is, as I see it, a bad
practice.

2) No ambiguity with '[['
A lot of newcomers confuse '[' and '[[' bashism.
https://unix.stackexchange.com/questions/306111/what-is-the-difference-between-the-bash-operators-vs-vs-vs
https://stackoverflow.com/questions/12063692/vs-in-bash-shell
https://stackoverflow.com/questions/669452/is-double-square-brackets-preferable-over-single-square-brackets-in-ba
The struggle is real.
Also, POSIX specifies that '[[' causes “unspecified results” [2].

3) Command vs. operator
For newcomers, '[' gives the illusion that it’s an operator part of the shell
and not a command, which is simply wrong. This misunderstanding is enforced by
the fact neither `man [` nor `man \[` bring you to the 'test' man page.
> Surprisingly, [ is just another command. […] Unfortunately, many users come
> across this usage first and assume the brackets are part of if itself. This
> can lead to some nonsensical statements.
https://thoughtbot.com/blog/the-unix-shells-humble-if#the--command

This is more enforced by the fact that both `bash -c 'type ['` and `dash -c
'type ['` return “[ is a shell builtin” [3]. That being said, I must admit that
it’s not easy to differentiate commands, built-ins and special built-ins [4].
For example, bash and dash do not recognise 'newgrp' as a shell built-in. Bash
documentation does not list 'times' as a special built-ins [5]. Dash does not
recognise 'fc' even if it should be a built-in.

4) Minimal bootstrapping
I’m currently building a GNU+Linux distribution, and I’m reducing to the strict
minimum the number of dependencies/commands. Right now, I’m using a subset of
BusyBox instead of GNU Coreutils during the boostrap process. So I’m building
BusyBox with 'CONFIG_TEST' option, which provides the 'test' command, but
without 'CONFIG_TEST1' option, which extends the 'test' command to accomodate
'[' and ']'. So far so good. I’ve submitted patches to CMake [6], and wrote to
Paul Smith, which kindly replaced '[' by 'test' in Make source code [7].

[1] https://lists.gnu.org/archive/html/automake-patches/2020-08/msg00004.html
[2] 
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_04
[3] 
https://unix.stackexchange.com/questions/183745/why-is-a-shell-builtin-and-a-shell-keyword
[4] https://www.gnu.org/software/bash/manual/html_node/Special-Builtins.html
[5] https://github.com/emersion/mrsh/pull/160
[6] https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4704
[7] https://git.savannah.gnu.org/cgit/make.git/commit/?id=c8a6263

I hope this is convincing enough :)

Cordially,
Issam E.

PS: This is my first patch submission to a GNU project. Big fan!



reply via email to

[Prev in Thread] Current Thread [Next in Thread]