linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Arnd Bergmann <arnd@arndb.de>, Kees Cook <keescook@chromium.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	Ulf Magnusson <ulfalizer@gmail.com>,
	Sam Ravnborg <sam@ravnborg.org>,
	Michal Marek <michal.lkml@markovi.net>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 11/23] kconfig: add 'shell-stdout' function
Date: Fri, 16 Feb 2018 11:17:52 -0800	[thread overview]
Message-ID: <CA+55aFwQLrrBeXZXK_qUXiGVFNv3qPJS4tNKFacuk0Y0WgZjCA@mail.gmail.com> (raw)
In-Reply-To: <1518806331-7101-12-git-send-email-yamada.masahiro@socionext.com>

On Fri, Feb 16, 2018 at 10:38 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> This is the second built-in function, which retrieves the first line
> of stdout from the given shell command.

This is the only part I really don't much like in your patch series.

Most of it is just lovely and looks very nice and powerful, but the
difference between "$(shell ..." and "$(shell-stdout ..." to me is
bvery ugly.

Can we *please* make "shell-stdout" go away, and make this just be "shell"?

The rule would be very simple:

 - if the result of the shell command is a failure, the result is 'n'

 - otherwise, the result is the first line of stdout

 - if the result is empty, we replace it with 'y'.

So doing $(shell true) would be 100% equivalent to $(shell echo y),
and you could still do that

          default $(shell $CC --version | grep -q gcc)

because it would just automatically do the right thing.

Basically, the only difference is how $(shell ) works in the success
case: the result won't necessarily be 'y', it will be whatever output.
But if you want to always turn it into 'y' (say, you don't have a "-q"
flag for the grep equivalent above), you can always do so with

          default $(shell $CC --version | noqgrep gcc > /dev/null)

So it seems to me that there is never any fundamental reason why we'd
want both "shell" and "shell-stdout", since "shell-stdout" is
fundamentally more powerful than "shell", and can always be used as
such (and just renamed to "shell").

Because I really think that it's just much prettier and more intuitive
to be able to say

        default "/boot/config-$(shell uname --release)"

without that "-stdout" thing.

Hmm?

But I do want to say how much I liked this series. Just this part
seemed to result in uglier Kconfig scripts.

         Linus

  reply	other threads:[~2018-02-16 19:17 UTC|newest]

Thread overview: 64+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-16 18:38 [PATCH 00/23] kconfig: move compiler capability tests to Kconfig Masahiro Yamada
2018-02-16 18:38 ` [PATCH 01/23] kbuild: remove kbuild cache Masahiro Yamada
2018-02-16 18:38 ` [PATCH 02/23] kbuild: remove CONFIG_CROSS_COMPILE support Masahiro Yamada
2018-02-16 18:38 ` [PATCH 03/23] kconfig: add xstrdup() helper Masahiro Yamada
2018-03-01 15:02   ` Masahiro Yamada
2018-02-16 18:38 ` [PATCH 04/23] kconfig: set SYMBOL_AUTO to the symbol marked with defconfig_list Masahiro Yamada
2018-03-01 15:05   ` Masahiro Yamada
2018-03-01 17:11     ` Ulf Magnusson
2018-02-16 18:38 ` [PATCH 05/23] kconfig: move and rename sym_expand_string_value() Masahiro Yamada
2018-02-16 18:38 ` [PATCH 06/23] kconfig: reference environments directly and remove 'option env=' syntax Masahiro Yamada
2018-02-18 11:15   ` Ulf Magnusson
2018-02-16 18:38 ` [PATCH 07/23] kconfig: add function support and implement 'shell' function Masahiro Yamada
2018-02-17 16:16   ` Ulf Magnusson
2018-02-19 15:57     ` Masahiro Yamada
2018-02-19 17:50       ` Ulf Magnusson
2018-02-19 20:06         ` Ulf Magnusson
2018-02-19 22:06           ` Ulf Magnusson
2018-02-16 18:38 ` [PATCH 08/23] kconfig: add 'macro' keyword to support user-defined function Masahiro Yamada
2018-02-16 19:49   ` Nicolas Pitre
2018-02-16 23:51     ` Ulf Magnusson
2018-02-17  2:30       ` Nicolas Pitre
2018-02-17  4:29         ` Ulf Magnusson
2018-02-17  4:44           ` Nicolas Pitre
2018-02-17  6:06             ` Ulf Magnusson
2018-02-16 18:38 ` [PATCH 09/23] kconfig: add 'cc-option' macro Masahiro Yamada
2018-02-16 18:38 ` [PATCH 10/23] stack-protector: test compiler capability in Kconfig and drop AUTO mode Masahiro Yamada
2018-02-21  4:39   ` Masahiro Yamada
2018-02-16 18:38 ` [PATCH 11/23] kconfig: add 'shell-stdout' function Masahiro Yamada
2018-02-16 19:17   ` Linus Torvalds [this message]
2018-02-19  4:48     ` Ulf Magnusson
2018-02-19 17:44       ` Linus Torvalds
2018-02-19 18:01         ` Linus Torvalds
2018-02-19 18:54           ` Ulf Magnusson
2018-02-21  4:59           ` Masahiro Yamada
2018-02-21 16:41             ` Ulf Magnusson
2018-02-21 17:01             ` Linus Torvalds
2018-02-16 18:38 ` [PATCH 12/23] kconfig: replace $UNAME_RELEASE with function call Masahiro Yamada
2018-02-16 18:38 ` [PATCH 13/23] kconfig: expand environments/functions in (main)menu, comment, prompt Masahiro Yamada
2018-02-16 18:38 ` [PATCH 14/23] kconfig: show compiler version text in the top comment Masahiro Yamada
2018-02-16 18:38 ` [PATCH 15/23] kconfig: add CC_IS_GCC and GCC_VERSION Masahiro Yamada
2018-02-16 18:38 ` [PATCH 16/23] kbuild: add clang-version.sh Masahiro Yamada
2018-02-16 18:38 ` [PATCH 17/23] kconfig: add CC_IS_CLANG and CLANG_VERSION Masahiro Yamada
2018-02-16 18:38 ` [PATCH 18/23] gcov: remove CONFIG_GCOV_FORMAT_AUTODETECT Masahiro Yamada
2018-02-16 18:38 ` [PATCH 19/23] kcov: imply GCC_PLUGINS and GCC_PLUGIN_SANCOV instead of select'ing them Masahiro Yamada
2018-02-16 18:38 ` [PATCH 20/23] gcc-plugins: always build plugins with C++ Masahiro Yamada
2018-02-22 18:45   ` Emese Revfy
2018-02-23 12:37     ` Masahiro Yamada
2018-02-16 18:38 ` [PATCH 21/23] gcc-plugins: move GCC version check for PowerPC to Kconfig Masahiro Yamada
2018-02-22  5:04   ` Andrew Donnellan
2018-02-16 18:38 ` [PATCH 22/23] gcc-plugins: test GCC plugin support in Kconfig Masahiro Yamada
2018-02-16 18:38 ` [PATCH 23/23] gcc-plugins: enable GCC_PLUGINS for COMPILE_TEST Masahiro Yamada
2018-02-18 22:13 ` [PATCH 00/23] kconfig: move compiler capability tests to Kconfig Sam Ravnborg
2018-02-19 15:18   ` Ulf Magnusson
2018-02-21  7:38     ` Masahiro Yamada
2018-02-21  9:56       ` Arnd Bergmann
2018-02-21 10:20         ` Masahiro Yamada
2018-02-21 10:52           ` Arnd Bergmann
2018-02-21 12:57             ` Masahiro Yamada
2018-02-21 16:03               ` Arnd Bergmann
2018-02-21 21:39               ` Ulf Magnusson
2018-03-02  5:50                 ` Masahiro Yamada
2018-03-02  9:03                   ` Ulf Magnusson
2018-03-02  9:12                     ` Ulf Magnusson
2018-02-22  3:22               ` Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CA+55aFwQLrrBeXZXK_qUXiGVFNv3qPJS4tNKFacuk0Y0WgZjCA@mail.gmail.com \
    --to=torvalds@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=keescook@chromium.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.lkml@markovi.net \
    --cc=rdunlap@infradead.org \
    --cc=sam@ravnborg.org \
    --cc=ulfalizer@gmail.com \
    --cc=yamada.masahiro@socionext.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).