Re: kconfig: diagnostics cleanups

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

 



Masahiro Yamada <masahiroy@xxxxxxxxxx> writes:

> On Wed, Dec 2, 2020 at 5:06 PM Boris Kolpackov <boris@xxxxxxxxxxxxxxxxx> wrote:
>
> > > Boris Kolpackov <boris@xxxxxxxxxxxxxxxxx> wrote:
> > >
> > > > 1. Add 'warning' word to $(warning-if) output:
> >
> > This will make the diagnostics consistent with other places in kconfig
> > where warnings are issued (see conf_warrning() in confdata.c).
> 
> 
> 'warning:' from C code and $(warning-if) are implemented
> in different layers. So, I do not think it is necessary
> to prepend 'warning:'.

What does it matter to the user, who sees the inconsistent
diagnostics, that they are implemented in different layers?


> More importantly, I cannot find a good way to print
> multiple lines of error messages when $(error ...) is hit.
> I prefer wrapping a long message in 80-columns.
> 
> 
> After all, the best way I came up with for GNU Make is to use
> $(error ) for the last line, and $(warning ) for the rest.
> 
> $(warning This is the first line)
> $(warning This is the second line)
> $(error This is the last line)
> 
> 
> masahiro@grover:~$ make
> Makefile:1: This is the first line
> Makefile:2: This is the second line
> Makefile:3: *** This is the last line.  Stop.
>  
> 
> This works, except the small flaw, "***".

IMO, there is a much bigger flaw: there is no way for the user
to know that these three lines are about the same error.

If you want this ability, then let's find a way do it properly
rather than spreading further hacks. For example, in the build
system I am working on, we have suport for multi-line diagnostics
records that to the user look like this:

Makefile:3: error: This is the first line
  This is the second line
  This is the last line

 
> If you want, you can include 'warning: ' in the message,
> but you would not be able to get rid of it if it were
> printed by default.

You can get rid of it by using $(info).

 
> In a little more thought, I'd rather go opposite;
> make $(warning-if) and $(error-if) as simple as
> just printing the given message without any prefix.
> 
> https://patchwork.kernel.org/project/linux-kbuild/patch/20201221094650.283511-1-masahiroy@xxxxxxxxxx/

Wouldn't showing the position in the Kconfig file where
the error/warning has originated be much, much more useful
than the occasional need to print multi-line messages?


> > > > 2. Print $(info) output to stderr instead of stdout.
> >
> > There are two reasons:
> >
> > 1. Error, warning, and info are different diagnostics levels. It was
> >    surprising to me that the first two go to stderr while info goes
> >    to stdout. For example, as a user, if I redirect stderr, I would
> >    naturally expect all the diagnostics to go there.
> >
> > 2. More importantly, stdout is used by terminal-based UI configurators.
> >    So depending on exactly when $(info) is issued, its output could either
> >    be clobbered by UI (so the user won't notice it) or it can clobber UI
> >    (so the user will see broken UI).
> 
> 
> I do not think this is overly problematic
> because Kconfig enters the GUI mode after
> parsing all Kconfig files.

How is me (as a user) redirecting stderr to a location of
my choosing and only ending up with half of the diagnostics
there (with the other half silently overridden by the GUI)
not problematic?

 
> Also, if my new patch lands, [...]

I hope it does not.

In one of your emails you've said that you believe the kconfig
implementation is very immature (which I wholly agree with) and
you would like to clean it up. To me one of the biggest signs
of this immaturity is various inconsistencies and the changes I
am proposing were to address some of the more user-visible ones.
It's baffling to me that you not only find my changes unnecessary
but actually propose changes which in my view would only exacerbate
the problem. It seems we understand very differently what exactly
is immature about kconfig.



[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux