2007-08-31 17:25:48

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus

On Thu, 19 Jul 2007 23:05:57 +0100 Simon Arlott wrote:

> On 19/07/07 17:19, Robert P. J. Day wrote:
> > On Thu, 19 Jul 2007, Randy Dunlap wrote:
> >> I think that Stefan means a patch to the kconfig source code,
> >> not the the Kconfig files. Good luck. I'd still like to see it.
> >
> > yes, i understand what he wanted now. as a first step (that
> > theoretically shouldn't change any behaviour), i'd patch the Kconfig
> > structure to add a new attribute ("maturity") which would be allowed
> > to be set to *exactly one* of a pre-defined set of values (say,
> > OBSOLETE, DEPRECATED, EXPERIMENTAL, and STILLBLEEDING). and that's
> > it, nothing more.
> >
> > don't try to do anything with any of that just yet, just add the
> > infrastructure to support the (optional) association of a maturity
> > level with a config option. that's step one.
>
> What about something like this? I'm not sure if the addition to sym_init
> is desirable... I also had to prefix _ to the name for now otherwise it
> conflicts badly with the current symbols. It probably should stop
> "depends on _BROKEN" etc. too.

Hi Simon,

(sorry for the delay)

I like this patch very much... I just can't get it to build (on
2.6.23-rc4).

Thanks.

> ---
> diff --git a/init/Kconfig b/init/Kconfig
> diff --git a/net/Kconfig b/net/Kconfig
> index cdba08c..5e2f4db 100644
> --- a/net/Kconfig
> +++ b/net/Kconfig
> @@ -6,6 +6,7 @@ menu "Networking"
>
> config NET
> bool "Networking support"
> + maturity _BROKEN
> ---help---
> Unless you really know what you are doing, you should say Y here.
> The reason is that some programs need kernel networking support even
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile
> index fb2bb30..1dea08e 100644
> --- a/scripts/kconfig/Makefile
> +++ b/scripts/kconfig/Makefile
> @@ -256,7 +256,7 @@ $(obj)/lkc_defs.h: $(src)/lkc_proto.h
> # The following requires flex/bison/gperf
> # By default we use the _shipped versions, uncomment the following line if
> # you are modifying the flex/bison src.
> -# LKC_GENPARSER := 1
> +LKC_GENPARSER := 1
>
> ifdef LKC_GENPARSER
>
> diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c
> index 1199baf..cc9be0e 100644
> --- a/scripts/kconfig/conf.c
> +++ b/scripts/kconfig/conf.c
> @@ -211,6 +211,22 @@ static int conf_sym(struct menu *menu)
>
> while (1) {
> printf("%*s%s ", indent - 1, "", menu->prompt->text);
> + switch (sym->maturity) {
> + case M_EXPERIMENTAL:
> + printf("(EXPERIMENTAL) ");
> + break;
> + case M_DEPRECATED:
> + printf("(DEPRECATED) ");
> + break;
> + case M_OBSOLETE:
> + printf("(OBSOLETE) ");
> + break;
> + case M_BROKEN:
> + printf("(BROKEN) ");
> + break;
> + default:
> + break;
> + }
> if (sym->name)
> printf("(%s) ", sym->name);
> type = sym_get_type(sym);
> diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
> index 6084525..a22b6c1 100644
> --- a/scripts/kconfig/expr.h
> +++ b/scripts/kconfig/expr.h
> @@ -60,7 +60,11 @@ struct symbol_value {
> };
>
> enum symbol_type {
> - S_UNKNOWN, S_BOOLEAN, S_TRISTATE, S_INT, S_HEX, S_STRING, S_OTHER
> + S_UNKNOWN, S_BOOLEAN, S_TRISTATE, S_INT, S_HEX, S_STRING, S_MATURITY,
> S_OTHER
> +};
> +
> +enum maturity_level {
> + M_NONE, M_EXPERIMENTAL, M_DEPRECATED, M_OBSOLETE, M_BROKEN
> };
>
> enum {
> @@ -72,6 +76,7 @@ struct symbol {
> struct symbol *next;
> char *name;
> char *help;
> + enum maturity_level maturity;
> enum symbol_type type;
> struct symbol_value curr;
> struct symbol_value def[4];
> diff --git a/scripts/kconfig/lex.zconf.c_shipped
> b/scripts/kconfig/lex.zconf.c_shipped
> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
> index 8a07ee4..9add1cd 100644
> --- a/scripts/kconfig/lkc.h
> +++ b/scripts/kconfig/lkc.h
> @@ -79,6 +79,7 @@ void menu_end_menu(void);
> void menu_add_entry(struct symbol *sym);
> void menu_end_entry(void);
> void menu_add_dep(struct expr *dep);
> +void menu_set_maturity(struct symbol *sym);
> struct property *menu_add_prop(enum prop_type type, char *prompt,
> struct expr *expr, struct expr *dep);
> struct property *menu_add_prompt(enum prop_type type, char *prompt,
> struct expr *dep);
> void menu_add_expr(enum prop_type type, struct expr *expr, struct expr
> *dep);
> diff --git a/scripts/kconfig/mconf.c b/scripts/kconfig/mconf.c
> index d0e4fa5..eaf199b 100644
> --- a/scripts/kconfig/mconf.c
> +++ b/scripts/kconfig/mconf.c
> @@ -238,6 +238,7 @@ search_help[] = N_(
> "Result:\n"
> "-----------------------------------------------------------------\n"
> "Symbol: FOO [=m]\n"
> + "Maturity: FOO\n"
> "Prompt: Foo bus is used to drive the bar HW\n"
> "Defined at drivers/pci/Kconfig:47\n"
> "Depends on: X86_LOCAL_APIC && X86_IO_APIC || IA64\n"
> @@ -359,6 +360,27 @@ static void get_symbol_str(struct gstr *r, struct
> symbol *sym)
>
> str_printf(r, "Symbol: %s [=%s]\n", sym->name,
> sym_get_string_value(sym));
> +
> + if (sym->maturity != M_NONE) {
> + str_append(r, "Maturity: ");
> + switch (sym->maturity) {
> + case M_EXPERIMENTAL:
> + str_append(r, "EXPERIMENTAL\n");
> + break;
> + case M_DEPRECATED:
> + str_append(r, "DEPRECATED\n");
> + break;
> + case M_OBSOLETE:
> + str_append(r, "OBSOLETE\n");
> + break;
> + case M_BROKEN:
> + str_append(r, "BROKEN\n");
> + break;
> + default:
> + break;
> + }
> + }
> +
> for_all_prompts(sym, prop)
> get_prompt_str(r, prop);
> hit = false;
> diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
> index f14aeac..3e37e5b 100644
> --- a/scripts/kconfig/menu.c
> +++ b/scripts/kconfig/menu.c
> @@ -104,6 +104,15 @@ void menu_add_dep(struct expr *dep)
> current_entry->dep = expr_alloc_and(current_entry->dep,
> menu_check_dep(dep));
> }
>
> +void menu_set_maturity(struct symbol *sym)
> +{
> + if (sym->type != S_MATURITY) {
> + zconf_error("'%s' is an invalid maturity level", sym->name);
> + } else {
> + current_entry->sym->maturity = sym->maturity;
> + }
> +}
> +
> void menu_set_type(int type)
> {
> struct symbol *sym = current_entry->sym;
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index c35dcc5..280ee8f 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -72,6 +72,22 @@ void sym_init(void)
> sym->type = S_STRING;
> sym->flags |= SYMBOL_AUTO;
> sym_add_default(sym, uts.release);
> +
> + sym = sym_lookup("_EXPERIMENTAL", 0);
> + sym->type = S_MATURITY;
> + sym->maturity = M_EXPERIMENTAL;
> +
> + sym = sym_lookup("_DEPRECATED", 0);
> + sym->type = S_MATURITY;
> + sym->maturity = M_DEPRECATED;
> +
> + sym = sym_lookup("_OBSOLETE", 0);
> + sym->type = S_MATURITY;
> + sym->maturity = M_OBSOLETE;
> +
> + sym = sym_lookup("_BROKEN", 0);
> + sym->type = S_MATURITY;
> + sym->maturity = M_BROKEN;
> }
>
> enum symbol_type sym_get_type(struct symbol *sym)
> @@ -100,6 +116,8 @@ const char *sym_type_name(enum symbol_type type)
> return "hex";
> case S_STRING:
> return "string";
> + case S_MATURITY:
> + return "maturity";
> case S_UNKNOWN:
> return "unknown";
> case S_OTHER:
> @@ -679,6 +697,7 @@ struct symbol *sym_lookup(const char *name, int isconst)
> memset(symbol, 0, sizeof(*symbol));
> symbol->name = new_name;
> symbol->type = S_UNKNOWN;
> + symbol->maturity = M_NONE;
> if (isconst)
> symbol->flags |= SYMBOL_CONST;
>
> diff --git a/scripts/kconfig/zconf.gperf b/scripts/kconfig/zconf.gperf
> index 9b44c80..756d559 100644
> --- a/scripts/kconfig/zconf.gperf
> +++ b/scripts/kconfig/zconf.gperf
> @@ -25,6 +25,7 @@ endif, T_ENDIF, TF_COMMAND
> depends, T_DEPENDS, TF_COMMAND
> requires, T_REQUIRES, TF_COMMAND
> optional, T_OPTIONAL, TF_COMMAND
> +maturity, T_MATURITY, TF_COMMAND
> default, T_DEFAULT, TF_COMMAND, S_UNKNOWN
> prompt, T_PROMPT, TF_COMMAND
> tristate, T_TYPE, TF_COMMAND, S_TRISTATE
> diff --git a/scripts/kconfig/zconf.y b/scripts/kconfig/zconf.y
> index 92eb02b..1b47966 100644
> --- a/scripts/kconfig/zconf.y
> +++ b/scripts/kconfig/zconf.y
> @@ -66,6 +66,7 @@ static struct menu *current_menu, *current_entry;
> %token <id>T_DEPENDS
> %token <id>T_REQUIRES
> %token <id>T_OPTIONAL
> +%token <id>T_MATURITY
> %token <id>T_PROMPT
> %token <id>T_TYPE
> %token <id>T_DEFAULT
> @@ -120,7 +121,7 @@ stmt_list:
> ;
>
> option_name:
> - T_DEPENDS | T_PROMPT | T_TYPE | T_SELECT | T_OPTIONAL | T_RANGE |
> T_DEFAULT
> + T_DEPENDS | T_MATURITY | T_PROMPT | T_TYPE | T_SELECT | T_OPTIONAL |
> T_RANGE | T_DEFAULT
> ;
>
> common_stmt:
> @@ -177,6 +178,7 @@ config_option_list:
> | config_option_list config_option
> | config_option_list symbol_option
> | config_option_list depends
> + | config_option_list maturity
> | config_option_list help
> | config_option_list option_error
> | config_option_list T_EOL
> @@ -269,6 +271,7 @@ choice_option_list:
> /* empty */
> | choice_option_list choice_option
> | choice_option_list depends
> + | choice_option_list maturity
> | choice_option_list help
> | choice_option_list T_EOL
> | choice_option_list option_error
> @@ -349,7 +352,7 @@ menu: T_MENU prompt T_EOL
> printd(DEBUG_PARSE, "%s:%d:menu\n", zconf_curname(), zconf_lineno());
> };
>
> -menu_entry: menu depends_list
> +menu_entry: menu maturity_set_opt depends_list
> {
> $$ = menu_add_menu();
> };
> @@ -430,6 +433,19 @@ depends: T_DEPENDS T_ON expr T_EOL
> printd(DEBUG_PARSE, "%s:%d:requires\n", zconf_curname(), zconf_lineno());
> };
>
> +/* maturity setting */
> +
> +maturity_set_opt:
> + /* empty */
> + | maturity
> +;
> +
> +maturity: T_MATURITY symbol T_EOL
> +{
> + menu_set_maturity($2);
> + printd(DEBUG_PARSE, "%s:%d:maturity\n", zconf_curname(), zconf_lineno());
> +};
> +
> /* prompt statement */
>
> prompt_stmt_opt:
> @@ -519,6 +535,7 @@ const char *zconf_tokenname(int token)
> case T_IF: return "if";
> case T_ENDIF: return "endif";
> case T_DEPENDS: return "depends";
> + case T_MATURITY: return "maturity";
> }
> return "<token>";
> }
> @@ -615,6 +632,9 @@ void print_symbol(FILE *out, struct menu *menu)
> case S_HEX:
> fputs(" hex\n", out);
> break;
> + case S_MATURITY:
> + fputs(" maturity\n", out);
> + break;
> default:
> fputs(" ???\n", out);
> break;
>
>
> --

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***


2007-08-31 17:39:00

by Robert P. J. Day

[permalink] [raw]
Subject: Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus

On Fri, 31 Aug 2007, Randy Dunlap wrote:

> On Thu, 19 Jul 2007 23:05:57 +0100 Simon Arlott wrote:
>
> > On 19/07/07 17:19, Robert P. J. Day wrote:
> > > On Thu, 19 Jul 2007, Randy Dunlap wrote:
> > >> I think that Stefan means a patch to the kconfig source code,
> > >> not the the Kconfig files. Good luck. I'd still like to see it.
> > >
> > > yes, i understand what he wanted now. as a first step (that
> > > theoretically shouldn't change any behaviour), i'd patch the Kconfig
> > > structure to add a new attribute ("maturity") which would be allowed
> > > to be set to *exactly one* of a pre-defined set of values (say,
> > > OBSOLETE, DEPRECATED, EXPERIMENTAL, and STILLBLEEDING). and that's
> > > it, nothing more.
> > >
> > > don't try to do anything with any of that just yet, just add the
> > > infrastructure to support the (optional) association of a maturity
> > > level with a config option. that's step one.
> >
> > What about something like this? I'm not sure if the addition to sym_init
> > is desirable... I also had to prefix _ to the name for now otherwise it
> > conflicts badly with the current symbols. It probably should stop
> > "depends on _BROKEN" etc. too.

i'm sure i'm going to get shouted down here, but i really disagree
with "BROKEN" being considered a "maturity level". IMHO, things like
EXPERIMENTAL, DEPRECATED and OBSOLETE represent maturity levels, for
what i think are obvious reasons.

something like BROKEN, though, has *nothing* to do with maturity. a
feature can be any of those maturity levels, and simultaneously be
BROKEN. i consider BROKEN to be what i call a "status", and different
status levels might be the default of normal, or KIND_OF_FLAKY or
TOTALLY_BORKED -- that's where BROKEN would fit in.

*please* don't start mixing these different attributes. it's just
going to make an irreversible mess of things. leave BROKEN out of
this for the time being.

rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

2007-08-31 18:07:13

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus

Robert P. J. Day wrote:
> On Fri, 31 Aug 2007, Randy Dunlap wrote:
>
>> On Thu, 19 Jul 2007 23:05:57 +0100 Simon Arlott wrote:
>>
>>> On 19/07/07 17:19, Robert P. J. Day wrote:
>>>> On Thu, 19 Jul 2007, Randy Dunlap wrote:
>>>>> I think that Stefan means a patch to the kconfig source code,
>>>>> not the the Kconfig files. Good luck. I'd still like to see it.
>>>> yes, i understand what he wanted now. as a first step (that
>>>> theoretically shouldn't change any behaviour), i'd patch the Kconfig
>>>> structure to add a new attribute ("maturity") which would be allowed
>>>> to be set to *exactly one* of a pre-defined set of values (say,
>>>> OBSOLETE, DEPRECATED, EXPERIMENTAL, and STILLBLEEDING). and that's
>>>> it, nothing more.
>>>>
>>>> don't try to do anything with any of that just yet, just add the
>>>> infrastructure to support the (optional) association of a maturity
>>>> level with a config option. that's step one.
>>> What about something like this? I'm not sure if the addition to sym_init
>>> is desirable... I also had to prefix _ to the name for now otherwise it
>>> conflicts badly with the current symbols. It probably should stop
>>> "depends on _BROKEN" etc. too.
>
> i'm sure i'm going to get shouted down here, but i really disagree
> with "BROKEN" being considered a "maturity level". IMHO, things like
> EXPERIMENTAL, DEPRECATED and OBSOLETE represent maturity levels, for
> what i think are obvious reasons.
>
> something like BROKEN, though, has *nothing* to do with maturity. a
> feature can be any of those maturity levels, and simultaneously be
> BROKEN. i consider BROKEN to be what i call a "status", and different
> status levels might be the default of normal, or KIND_OF_FLAKY or
> TOTALLY_BORKED -- that's where BROKEN would fit in.

BROKEN is definitely a maturity level. A more accurate description
would be BITROTTING perhaps. The code in question has passed through
bleeding -> experimental -> stable, and come out the other side.

In contrast, OBSOLETE and DEPRECATED reflect high-level status not code
quality/maturity.

Jeff



2007-08-31 19:29:22

by Jan Engelhardt

[permalink] [raw]
Subject: Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus


On Aug 31 2007 14:06, Jeff Garzik wrote:
>> something like BROKEN, though, has *nothing* to do with maturity. a
>> feature can be any of those maturity levels, and simultaneously be
>> BROKEN. i consider BROKEN to be what i call a "status", and different
>> status levels might be the default of normal, or KIND_OF_FLAKY or
>> TOTALLY_BORKED -- that's where BROKEN would fit in.
>
> BROKEN is definitely a maturity level. A more accurate description would be
> BITROTTING perhaps. The code in question has passed through bleeding ->
> experimental -> stable, and come out the other side.

Software is quite unlike the art of medicine. There, you would go from
'stable' to 'broken', then someone 'experiments' with you, and if
it's a bad day, you bleed out. :)


Jan
--

2007-08-31 20:16:26

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus

On Fri, 31 Aug 2007 14:06:33 -0400 Jeff Garzik wrote:

> Robert P. J. Day wrote:
> > On Fri, 31 Aug 2007, Randy Dunlap wrote:
> >
> >> On Thu, 19 Jul 2007 23:05:57 +0100 Simon Arlott wrote:
> >>
> >>> On 19/07/07 17:19, Robert P. J. Day wrote:
> >>>> On Thu, 19 Jul 2007, Randy Dunlap wrote:
> >>>>> I think that Stefan means a patch to the kconfig source code,
> >>>>> not the the Kconfig files. Good luck. I'd still like to see it.
> >>>> yes, i understand what he wanted now. as a first step (that
> >>>> theoretically shouldn't change any behaviour), i'd patch the Kconfig
> >>>> structure to add a new attribute ("maturity") which would be allowed
> >>>> to be set to *exactly one* of a pre-defined set of values (say,
> >>>> OBSOLETE, DEPRECATED, EXPERIMENTAL, and STILLBLEEDING). and that's
> >>>> it, nothing more.
> >>>>
> >>>> don't try to do anything with any of that just yet, just add the
> >>>> infrastructure to support the (optional) association of a maturity
> >>>> level with a config option. that's step one.
> >>> What about something like this? I'm not sure if the addition to sym_init
> >>> is desirable... I also had to prefix _ to the name for now otherwise it
> >>> conflicts badly with the current symbols. It probably should stop
> >>> "depends on _BROKEN" etc. too.
> >
> > i'm sure i'm going to get shouted down here, but i really disagree
> > with "BROKEN" being considered a "maturity level". IMHO, things like
> > EXPERIMENTAL, DEPRECATED and OBSOLETE represent maturity levels, for
> > what i think are obvious reasons.
> >
> > something like BROKEN, though, has *nothing* to do with maturity. a
> > feature can be any of those maturity levels, and simultaneously be
> > BROKEN. i consider BROKEN to be what i call a "status", and different
> > status levels might be the default of normal, or KIND_OF_FLAKY or
> > TOTALLY_BORKED -- that's where BROKEN would fit in.
>
> BROKEN is definitely a maturity level. A more accurate description
> would be BITROTTING perhaps. The code in question has passed through
> bleeding -> experimental -> stable, and come out the other side.
>
> In contrast, OBSOLETE and DEPRECATED reflect high-level status not code
> quality/maturity.

What I like about the patch is that it associates some kconfig symbol
with prompt strings, so that we don't have to edit "(EXPERIMENTAL)"
all the darn time (e.g.).

I'd be quite happy with calling it "status" rather than "maturity",
and with being able to use multiple of the status tags at one time,
such as

config FOO
depends on BAR
status OBSOLETE BROKEN


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

2007-08-31 21:02:23

by Robert P. J. Day

[permalink] [raw]
Subject: Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus

On Fri, 31 Aug 2007, Jeff Garzik wrote:

> Robert P. J. Day wrote:

> > i'm sure i'm going to get shouted down here, but i really disagree
> > with "BROKEN" being considered a "maturity level". IMHO, things
> > like EXPERIMENTAL, DEPRECATED and OBSOLETE represent maturity
> > levels, for what i think are obvious reasons.
> >
> > something like BROKEN, though, has *nothing* to do with maturity.
> > a feature can be any of those maturity levels, and simultaneously
> > be BROKEN. i consider BROKEN to be what i call a "status", and
> > different status levels might be the default of normal, or
> > KIND_OF_FLAKY or TOTALLY_BORKED -- that's where BROKEN would fit
> > in.
>
> BROKEN is definitely a maturity level.

no. it's not. end of discussion. you're wrong.

the concept of "maturity level" reflects where in the life cycle some
feature is. it will typically start as "bleeding edge" or
"experimental" or something like that, eventually stabilize to be
normal (which would be the obvious default), after which, when its
value starts to run out and it begins showing its age, it becomes
"deprecated" and eventually "obsolete" it's a natural and obvious
progression.

on the other hand, a feature can be "broken" at *any* point in that
life cycle -- that's why it is absolutely *not* a maturity level.
please don't fight with me on this, jeff. you're simply wrong.

> In contrast, OBSOLETE and DEPRECATED reflect high-level status not
> code quality/maturity.

you have this so backwards, i can't begin to think how to explain it
to you.

rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

2007-08-31 21:13:54

by Robert P. J. Day

[permalink] [raw]
Subject: Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus

On Fri, 31 Aug 2007, Randy Dunlap wrote:

> What I like about the patch is that it associates some kconfig
> symbol with prompt strings, so that we don't have to edit
> "(EXPERIMENTAL)" all the darn time (e.g.).
>
> I'd be quite happy with calling it "status" rather than "maturity",
> and with being able to use multiple of the status tags at one time,
> such as
>
> config FOO
> depends on BAR
> status OBSOLETE BROKEN

grrrrrrrr ... i already made my point in my earlier post. i'd
really, really like it if *this* attribute remained as "maturity". an
entirely *separate* attribute could be defined as a feature "status",
which would be entirely orthogonal to maturity level, so that the
above would be written as

maturity OBSOLETE
status BROKEN

there's a reason for this -- any feature should have exactly *one*
value for any attribute. that is, in terms of maturity, a feature
could be EXPERIMENTAL *or* DEPRECATED *or* OBSOLETE. it ***can't***
be more than one, as in both DEPRECATED *and* OBSOLETE. to allow that
flexibility is to descend into absurdity.

rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

2007-08-31 21:26:43

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus

On Fri, 31 Aug 2007 17:00:57 -0400 (EDT) Robert P. J. Day wrote:

> On Fri, 31 Aug 2007, Randy Dunlap wrote:
>
> > What I like about the patch is that it associates some kconfig
> > symbol with prompt strings, so that we don't have to edit
> > "(EXPERIMENTAL)" all the darn time (e.g.).
> >
> > I'd be quite happy with calling it "status" rather than "maturity",
> > and with being able to use multiple of the status tags at one time,
> > such as
> >
> > config FOO
> > depends on BAR
> > status OBSOLETE BROKEN
>
> grrrrrrrr ... i already made my point in my earlier post. i'd
> really, really like it if *this* attribute remained as "maturity". an
> entirely *separate* attribute could be defined as a feature "status",
> which would be entirely orthogonal to maturity level, so that the
> above would be written as
>
> maturity OBSOLETE
> status BROKEN
>
> there's a reason for this -- any feature should have exactly *one*
> value for any attribute. that is, in terms of maturity, a feature
> could be EXPERIMENTAL *or* DEPRECATED *or* OBSOLETE. it ***can't***
> be more than one, as in both DEPRECATED *and* OBSOLETE. to allow that
> flexibility is to descend into absurdity.


If Simon (or anyone else) continues to work on it, I'll leave this
decision up to them...


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

2007-08-31 22:01:49

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus

Robert P. J. Day wrote:
> On Fri, 31 Aug 2007, Jeff Garzik wrote:
>
>> Robert P. J. Day wrote:
>
>>> i'm sure i'm going to get shouted down here, but i really disagree
>>> with "BROKEN" being considered a "maturity level". IMHO, things
>>> like EXPERIMENTAL, DEPRECATED and OBSOLETE represent maturity
>>> levels, for what i think are obvious reasons.
>>>
>>> something like BROKEN, though, has *nothing* to do with maturity.
>>> a feature can be any of those maturity levels, and simultaneously
>>> be BROKEN. i consider BROKEN to be what i call a "status", and
>>> different status levels might be the default of normal, or
>>> KIND_OF_FLAKY or TOTALLY_BORKED -- that's where BROKEN would fit
>>> in.
>> BROKEN is definitely a maturity level.
>
> no. it's not. end of discussion. you're wrong.
>
> the concept of "maturity level" reflects where in the life cycle some
> feature is. it will typically start as "bleeding edge" or
> "experimental" or something like that, eventually stabilize to be
> normal (which would be the obvious default), after which, when its
> value starts to run out and it begins showing its age, it becomes
> "deprecated" and eventually "obsolete" it's a natural and obvious
> progression.
>
> on the other hand, a feature can be "broken" at *any* point in that
> life cycle -- that's why it is absolutely *not* a maturity level.
> please don't fight with me on this, jeff. you're simply wrong.

Get off your high horse and actually look at the patches that mark
things BROKEN.

'deprecrated' and 'obsolete' are matters of discussed opinion,
describing the utility of the code in question. 'broken' describes the
state of the code itself.

Clear difference.

Jeff, one who actually marks this stuff as such



2007-08-31 22:23:25

by Robert P. J. Day

[permalink] [raw]
Subject: Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus

On Fri, 31 Aug 2007, Jeff Garzik wrote:

> 'deprecrated' and 'obsolete' are matters of discussed opinion,
> describing the utility of the code in question. 'broken' describes
> the state of the code itself.
>
> Clear difference.

precisely. thank you for making my point for me.

rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

2007-09-01 10:57:20

by Robert P. J. Day

[permalink] [raw]
Subject: Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus

On Fri, 31 Aug 2007, Randy Dunlap wrote:

> On Thu, 19 Jul 2007 23:05:57 +0100 Simon Arlott wrote:

> > What about something like this? I'm not sure if the addition to
> > sym_init is desirable... I also had to prefix _ to the name for
> > now otherwise it conflicts badly with the current symbols. It
> > probably should stop "depends on _BROKEN" etc. too.
>
> Hi Simon,
>
> (sorry for the delay)
>
> I like this patch very much... I just can't get it to build (on
> 2.6.23-rc4).

i got a patch from simon a while back, and it failed with
"shift/reduce" conflicts. is that what you're seeing?

> > while (1) {
> > printf("%*s%s ", indent - 1, "", menu->prompt->text);
> > + switch (sym->maturity) {
> > + case M_EXPERIMENTAL:
> > + printf("(EXPERIMENTAL) ");
> > + break;
> > + case M_DEPRECATED:
> > + printf("(DEPRECATED) ");
> > + break;
> > + case M_OBSOLETE:
> > + printf("(OBSOLETE) ");
> > + break;
> > + case M_BROKEN:
> > + printf("(BROKEN) ");
> > + break;
> > + default:
> > + break;
> > + }
> > if (sym->name)
> > printf("(%s) ", sym->name);
> > type = sym_get_type(sym);

for now, simon, why not just reduce this to supporting only DEPRECATED
and OBSOLETE so that it can be at least tested as "proof of concept?"

rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================

2007-09-01 12:47:53

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus

On Sat, Sep 01, 2007 at 06:44:06AM -0400, Robert P. J. Day wrote:
>
> > > while (1) {
> > > printf("%*s%s ", indent - 1, "", menu->prompt->text);
> > > + switch (sym->maturity) {
> > > + case M_EXPERIMENTAL:
> > > + printf("(EXPERIMENTAL) ");
> > > + break;
> > > + case M_DEPRECATED:
> > > + printf("(DEPRECATED) ");
> > > + break;
> > > + case M_OBSOLETE:
> > > + printf("(OBSOLETE) ");
> > > + break;
> > > + case M_BROKEN:
> > > + printf("(BROKEN) ");
> > > + break;
> > > + default:
> > > + break;
> > > + }
> > > if (sym->name)
> > > printf("(%s) ", sym->name);
> > > type = sym_get_type(sym);
>
> for now, simon, why not just reduce this to supporting only DEPRECATED
> and OBSOLETE so that it can be at least tested as "proof of concept?"

The principle with letting a dependency add text to the promts are good.
But the implementation done by Simon with a language extension is not good.
A simple and better approach would be to use the newly added option
support for this and let the backend generate the promtps.

I have not yet tried to cook up a patch for it, but it should be
quite generaic and doable.

config EXPERIMENTAL
option appendprompt=" (EXPERIMENTAL)

config DEPRECATED
option appendprompt=" (DEPRECATED)

Then the dependency added will automatically append to the prompt.

It would probarly have be done in two steps. One that introduce a helper
function to retreive the prompt text and introduce it. And next a patch to
add a text if a symbol has a dependency on a symbol with a specific option
assigned.

Sam

2007-09-01 13:09:46

by Robert P. J. Day

[permalink] [raw]
Subject: Re: [PATCH] net/, drivers/net/ , missing EXPERIMENTAL in menus

On Sat, 1 Sep 2007, Sam Ravnborg wrote:

> On Sat, Sep 01, 2007 at 06:44:06AM -0400, Robert P. J. Day wrote:
> >
> > > > while (1) {
> > > > printf("%*s%s ", indent - 1, "", menu->prompt->text);
> > > > + switch (sym->maturity) {
> > > > + case M_EXPERIMENTAL:
> > > > + printf("(EXPERIMENTAL) ");
> > > > + break;
> > > > + case M_DEPRECATED:
> > > > + printf("(DEPRECATED) ");
> > > > + break;
> > > > + case M_OBSOLETE:
> > > > + printf("(OBSOLETE) ");
> > > > + break;
> > > > + case M_BROKEN:
> > > > + printf("(BROKEN) ");
> > > > + break;
> > > > + default:
> > > > + break;
> > > > + }
> > > > if (sym->name)
> > > > printf("(%s) ", sym->name);
> > > > type = sym_get_type(sym);
> >
> > for now, simon, why not just reduce this to supporting only DEPRECATED
> > and OBSOLETE so that it can be at least tested as "proof of concept?"
>
> The principle with letting a dependency add text to the promts are good.
> But the implementation done by Simon with a language extension is not good.
> A simple and better approach would be to use the newly added option
> support for this and let the backend generate the promtps.
>
> I have not yet tried to cook up a patch for it, but it should be
> quite generaic and doable.
>
> config EXPERIMENTAL
> option appendprompt=" (EXPERIMENTAL)
>
> config DEPRECATED
> option appendprompt=" (DEPRECATED)


but this is deviating from the basic principle that these things are
not regular Kconfig "config" settings -- they are a new beast
entirely, and need a new infrastructure to support them.

put another way, they are not a normal kernel feature to be selected
or deselected. they are, rather, *attributes* that can be *applied*
to kernel features, in the same way as is done with "bool" or
"string". making these things regular config directives defeats the
whole purpose.

rday
--
========================================================================
Robert P. J. Day
Linux Consulting, Training and Annoying Kernel Pedantry
Waterloo, Ontario, CANADA

http://crashcourse.ca
========================================================================