Thread (64 messages) 64 messages, 9 authors, 2018-03-02

Re: [PATCH 11/23] kconfig: add 'shell-stdout' function

From: Ulf Magnusson <hidden>
Date: 2018-02-21 16:41:18
Also in: lkml

On Wed, Feb 21, 2018 at 01:59:59PM +0900, Masahiro Yamada wrote:
2018-02-20 3:01 GMT+09:00 Linus Torvalds [off-list ref]:
quoted
On Mon, Feb 19, 2018 at 9:44 AM, Linus Torvalds
[off-list ref] wrote:
quoted
I do like your "success"/"stdout" more than "shell"/"shell-stdout",
because with that naming I don't get the feeling that one should
subsume the other.
Hmm. Thinking about it some more, I really would prefer just "$(shell
...)" everywhere.

But it would be nice if perhaps the error handling would match the
context somehow.

I'm wondering if this might tie into the whole quoting discussion in
the other thread.

Because the rule could be:

(a) unquoted $(shell ) is a bool, and failing is ok (and turns into
y/n depending on whether successful or failing)

So

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

works automatically.

(b) but with quoting, $(shell ) is a string, and failing is an error

So

  config GCC_VERSION
          int
          default "$(shell-stdout $srctree/scripts/gcc-version.sh $CC
| sed 's/^0*//')" if CC_IS_GCC
          default 0

would need those quotes, and if the shell-script returns a failure,
we'd _abort_.

GCC_VERSION is int type.

Setting aside the Kconfig internal, I prefer 50700 to "50700"

According to my common sense, I do not want to quote integers.
Yeah, definitely looks a bit weird coming from other languages. "" just
means constant (a literal value, implemented as a constant symbol, as
opposed to a symbol reference).


If I were to redesign things, something like this would be closer to
what people intuitively expect:

  - n, m, y produces three predefined tristate symbols (they already do,
    through the n/m/y -> "n"/"m"/"y" shorthand)

  - "foo" produces a symbol of type string. Get rid of "n", "m", "y"
    (would require cleanup in Kconfig files).

  - 123 produces a symbol of type int

  - 0x123 produces a symbol of type hex

  - Everything else is a symbol reference

A nice side effect would be making all undefined symbol references just
that. I think that might come in handy. Some simple type checking could
be added to expressions as well.

You could also get rid of the obscure "undefined symbols produce their
name as their string value" magic at that point if you wanted to. That's
what makes 123 work currently.

One thing that gets a bit icky is that Kconfig currently stores constant
symbols in the symbol table, meaning "123" as a string might clash with
123 as an int, if you use the string value as the key (this would be a
problem if you want to give them the proper type). I wonder if there's
even any point to storing constant symbols in the symbol table though,
as opposed to just having them appear in expressions.
IMO, I prefer to use different names for different purpose.
So, 'stdout' and 'success' look good to me.



BTW, I noticed just one built-in function is enough
because 'success' can be derived from 'stdout'.


So, my plan is, implement $(shell ...) as a built-in function.
This returns the stdout from the command.


Then, implement 'success' as a textual shorthand
by using macro, like this:

macro success $(shell ($(1) && echo y) || echo n)


macro can be expanded recursively, so cc-option
can be implemented based on 'success' macro.
Was worried about the error handling for a second there, but this looks
like it might work out pretty nicely. $(success) would never have non-0
exit status, so you could still warn for that in $(shell).

There's also the question of shared Windows/Linux support in other
projects that use Kconfig, though my feeling is kernel devs don't care.
Might be pretty easy to work around anyway, by source'ing a different
Kconfig file with macro definitions depending on the OS. Would need that
for more complicated stuff anyway.

Cheers,
Ulf
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help