linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kconfig: allow for conditional dependencies
@ 2020-04-23 15:19 Nicolas Pitre
  2020-04-23 15:53 ` Jani Nikula
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Pitre @ 2020-04-23 15:19 UTC (permalink / raw)
  To: Masahiro Yamada, linux-kbuild
  Cc: Randy Dunlap, Jani Nikula, Saeed Mahameed, Arnd Bergmann, linux-kernel

This might appear to be a strange concept, but sometimes we want
a dependency to be conditionally applied. One such case is currently
expressed with:

        depends on FOO || !FOO

This pattern is strange enough to give one's pause. Given that it is
also frequent, let's make the intent more obvious with some syntaxic
sugar by effectively making dependencies optionally conditional.

This also makes the kconfig language more uniform.

Signed-off-by: Nicolas Pitre <nico@fluxnic.net>

diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
index d0111dd264..0f841e0037 100644
--- a/Documentation/kbuild/kconfig-language.rst
+++ b/Documentation/kbuild/kconfig-language.rst
@@ -114,7 +114,7 @@ applicable everywhere (see syntax).
   This is a shorthand notation for a type definition plus a value.
   Optionally dependencies for this default value can be added with "if".
 
-- dependencies: "depends on" <expr>
+- dependencies: "depends on" <expr> ["if" <expr>]
 
   This defines a dependency for this menu entry. If multiple
   dependencies are defined, they are connected with '&&'. Dependencies
@@ -130,6 +130,16 @@ applicable everywhere (see syntax).
 	bool "foo"
 	default y
 
+  The dependency definition itself may be conditional by appending "if"
+  followed by an expression. If such expression is false (n) then this
+  dependency is ignored. One possible use case is:
+
+    config FOO
+	tristate
+	depends on BAZ if BAZ != n
+
+  meaning that FOO is constrained by the value of BAZ only when it is set.
+
 - reverse dependencies: "select" <symbol> ["if" <expr>]
 
   While normal dependencies reduce the upper limit of a symbol (see
diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
index d4ca829736..1a9337d1b9 100644
--- a/scripts/kconfig/lkc.h
+++ b/scripts/kconfig/lkc.h
@@ -72,7 +72,7 @@ void menu_warn(struct menu *menu, const char *fmt, ...);
 struct menu *menu_add_menu(void);
 void menu_end_menu(void);
 void menu_add_entry(struct symbol *sym);
-void menu_add_dep(struct expr *dep);
+void menu_add_dep(struct expr *dep, struct expr *cond);
 void menu_add_visibility(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/menu.c b/scripts/kconfig/menu.c
index e436ba44c9..47928cdbc2 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -103,8 +103,18 @@ static struct expr *rewrite_m(struct expr *e)
 	return e;
 }
 
-void menu_add_dep(struct expr *dep)
+void menu_add_dep(struct expr *dep, struct expr *cond)
 {
+	if (cond) {
+		/*
+		 * We have "depends on X if Y" and we want:
+		 *	Y != n --> X
+		 *	Y == n --> y
+		 * That simplifies to: (X || (Y == n))
+		 */
+		dep = expr_alloc_or(dep,
+				expr_trans_compare(cond, E_EQUAL, &symbol_no));
+	}
 	current_entry->dep = expr_alloc_and(current_entry->dep, dep);
 }
 
diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
index 708b6c4b13..4161207da2 100644
--- a/scripts/kconfig/parser.y
+++ b/scripts/kconfig/parser.y
@@ -316,7 +316,7 @@ if_entry: T_IF expr T_EOL
 {
 	printd(DEBUG_PARSE, "%s:%d:if\n", zconf_curname(), zconf_lineno());
 	menu_add_entry(NULL);
-	menu_add_dep($2);
+	menu_add_dep($2, NULL);
 	$$ = menu_add_menu();
 };
 
@@ -412,9 +412,9 @@ help: help_start T_HELPTEXT
 
 /* depends option */
 
-depends: T_DEPENDS T_ON expr T_EOL
+depends: T_DEPENDS T_ON expr if_expr T_EOL
 {
-	menu_add_dep($3);
+	menu_add_dep($3, $4);
 	printd(DEBUG_PARSE, "%s:%d:depends on\n", zconf_curname(), zconf_lineno());
 };
 

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] kconfig: allow for conditional dependencies
  2020-04-23 15:19 [PATCH] kconfig: allow for conditional dependencies Nicolas Pitre
@ 2020-04-23 15:53 ` Jani Nikula
  2020-04-23 16:05   ` Nicolas Pitre
  0 siblings, 1 reply; 4+ messages in thread
From: Jani Nikula @ 2020-04-23 15:53 UTC (permalink / raw)
  To: Nicolas Pitre, Masahiro Yamada, linux-kbuild
  Cc: Randy Dunlap, Saeed Mahameed, Arnd Bergmann, linux-kernel

On Thu, 23 Apr 2020, Nicolas Pitre <nico@fluxnic.net> wrote:
> This might appear to be a strange concept, but sometimes we want
> a dependency to be conditionally applied. One such case is currently
> expressed with:
>
>         depends on FOO || !FOO
>
> This pattern is strange enough to give one's pause. Given that it is
> also frequent, let's make the intent more obvious with some syntaxic
> sugar by effectively making dependencies optionally conditional.
>
> This also makes the kconfig language more uniform.

Thanks, I prefer this over all the previous proposals. Versatile yet
self-explanatory.

> Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
>
> diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
> index d0111dd264..0f841e0037 100644
> --- a/Documentation/kbuild/kconfig-language.rst
> +++ b/Documentation/kbuild/kconfig-language.rst
> @@ -114,7 +114,7 @@ applicable everywhere (see syntax).
>    This is a shorthand notation for a type definition plus a value.
>    Optionally dependencies for this default value can be added with "if".
>  
> -- dependencies: "depends on" <expr>
> +- dependencies: "depends on" <expr> ["if" <expr>]
>  
>    This defines a dependency for this menu entry. If multiple
>    dependencies are defined, they are connected with '&&'. Dependencies
> @@ -130,6 +130,16 @@ applicable everywhere (see syntax).
>  	bool "foo"
>  	default y
>  
> +  The dependency definition itself may be conditional by appending "if"
> +  followed by an expression. If such expression is false (n) then this
> +  dependency is ignored. One possible use case is:
> +
> +    config FOO
> +	tristate
> +	depends on BAZ if BAZ != n

I presume this is the same as

	depends on BAZ if BAZ

which makes me wonder if that should be the example. At least current
usage for select is predominantly

	select FOO if BAR

without "!= n".

BR,
Jani.


> +
> +  meaning that FOO is constrained by the value of BAZ only when it is set.
> +
>  - reverse dependencies: "select" <symbol> ["if" <expr>]
>  
>    While normal dependencies reduce the upper limit of a symbol (see
> diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h
> index d4ca829736..1a9337d1b9 100644
> --- a/scripts/kconfig/lkc.h
> +++ b/scripts/kconfig/lkc.h
> @@ -72,7 +72,7 @@ void menu_warn(struct menu *menu, const char *fmt, ...);
>  struct menu *menu_add_menu(void);
>  void menu_end_menu(void);
>  void menu_add_entry(struct symbol *sym);
> -void menu_add_dep(struct expr *dep);
> +void menu_add_dep(struct expr *dep, struct expr *cond);
>  void menu_add_visibility(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/menu.c b/scripts/kconfig/menu.c
> index e436ba44c9..47928cdbc2 100644
> --- a/scripts/kconfig/menu.c
> +++ b/scripts/kconfig/menu.c
> @@ -103,8 +103,18 @@ static struct expr *rewrite_m(struct expr *e)
>  	return e;
>  }
>  
> -void menu_add_dep(struct expr *dep)
> +void menu_add_dep(struct expr *dep, struct expr *cond)
>  {
> +	if (cond) {
> +		/*
> +		 * We have "depends on X if Y" and we want:
> +		 *	Y != n --> X
> +		 *	Y == n --> y
> +		 * That simplifies to: (X || (Y == n))
> +		 */
> +		dep = expr_alloc_or(dep,
> +				expr_trans_compare(cond, E_EQUAL, &symbol_no));
> +	}
>  	current_entry->dep = expr_alloc_and(current_entry->dep, dep);
>  }
>  
> diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
> index 708b6c4b13..4161207da2 100644
> --- a/scripts/kconfig/parser.y
> +++ b/scripts/kconfig/parser.y
> @@ -316,7 +316,7 @@ if_entry: T_IF expr T_EOL
>  {
>  	printd(DEBUG_PARSE, "%s:%d:if\n", zconf_curname(), zconf_lineno());
>  	menu_add_entry(NULL);
> -	menu_add_dep($2);
> +	menu_add_dep($2, NULL);
>  	$$ = menu_add_menu();
>  };
>  
> @@ -412,9 +412,9 @@ help: help_start T_HELPTEXT
>  
>  /* depends option */
>  
> -depends: T_DEPENDS T_ON expr T_EOL
> +depends: T_DEPENDS T_ON expr if_expr T_EOL
>  {
> -	menu_add_dep($3);
> +	menu_add_dep($3, $4);
>  	printd(DEBUG_PARSE, "%s:%d:depends on\n", zconf_curname(), zconf_lineno());
>  };
>  

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] kconfig: allow for conditional dependencies
  2020-04-23 15:53 ` Jani Nikula
@ 2020-04-23 16:05   ` Nicolas Pitre
  2020-05-06 16:43     ` Masahiro Yamada
  0 siblings, 1 reply; 4+ messages in thread
From: Nicolas Pitre @ 2020-04-23 16:05 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Masahiro Yamada, linux-kbuild, Randy Dunlap, Saeed Mahameed,
	Arnd Bergmann, linux-kernel

On Thu, 23 Apr 2020, Jani Nikula wrote:

> On Thu, 23 Apr 2020, Nicolas Pitre <nico@fluxnic.net> wrote:
> > This might appear to be a strange concept, but sometimes we want
> > a dependency to be conditionally applied. One such case is currently
> > expressed with:
> >
> >         depends on FOO || !FOO
> >
> > This pattern is strange enough to give one's pause. Given that it is
> > also frequent, let's make the intent more obvious with some syntaxic
> > sugar by effectively making dependencies optionally conditional.
> >
> > This also makes the kconfig language more uniform.
> 
> Thanks, I prefer this over all the previous proposals. Versatile yet
> self-explanatory.
> 
> > Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
> >
> > diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
> > index d0111dd264..0f841e0037 100644
> > --- a/Documentation/kbuild/kconfig-language.rst
> > +++ b/Documentation/kbuild/kconfig-language.rst
> > @@ -114,7 +114,7 @@ applicable everywhere (see syntax).
> >    This is a shorthand notation for a type definition plus a value.
> >    Optionally dependencies for this default value can be added with "if".
> >  
> > -- dependencies: "depends on" <expr>
> > +- dependencies: "depends on" <expr> ["if" <expr>]
> >  
> >    This defines a dependency for this menu entry. If multiple
> >    dependencies are defined, they are connected with '&&'. Dependencies
> > @@ -130,6 +130,16 @@ applicable everywhere (see syntax).
> >  	bool "foo"
> >  	default y
> >  
> > +  The dependency definition itself may be conditional by appending "if"
> > +  followed by an expression. If such expression is false (n) then this
> > +  dependency is ignored. One possible use case is:
> > +
> > +    config FOO
> > +	tristate
> > +	depends on BAZ if BAZ != n
> 
> I presume this is the same as
> 
> 	depends on BAZ if BAZ
> 
> which makes me wonder if that should be the example. At least current
> usage for select is predominantly
> 
> 	select FOO if BAR
> 
> without "!= n".

Yes, it is the same thing. I prefer making the documentation a little 
more explicit than necessary so the explanation is really obvious.


Nicolas

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] kconfig: allow for conditional dependencies
  2020-04-23 16:05   ` Nicolas Pitre
@ 2020-05-06 16:43     ` Masahiro Yamada
  0 siblings, 0 replies; 4+ messages in thread
From: Masahiro Yamada @ 2020-05-06 16:43 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Jani Nikula, Linux Kbuild mailing list, Randy Dunlap,
	Saeed Mahameed, Arnd Bergmann, Linux Kernel Mailing List

On Fri, Apr 24, 2020 at 1:05 AM Nicolas Pitre <nico@fluxnic.net> wrote:
>
> On Thu, 23 Apr 2020, Jani Nikula wrote:
>
> > On Thu, 23 Apr 2020, Nicolas Pitre <nico@fluxnic.net> wrote:
> > > This might appear to be a strange concept, but sometimes we want
> > > a dependency to be conditionally applied. One such case is currently
> > > expressed with:
> > >
> > >         depends on FOO || !FOO
> > >
> > > This pattern is strange enough to give one's pause. Given that it is
> > > also frequent, let's make the intent more obvious with some syntaxic
> > > sugar by effectively making dependencies optionally conditional.
> > >
> > > This also makes the kconfig language more uniform.
> >
> > Thanks, I prefer this over all the previous proposals. Versatile yet
> > self-explanatory.
> >
> > > Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
> > >
> > > diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
> > > index d0111dd264..0f841e0037 100644
> > > --- a/Documentation/kbuild/kconfig-language.rst
> > > +++ b/Documentation/kbuild/kconfig-language.rst
> > > @@ -114,7 +114,7 @@ applicable everywhere (see syntax).
> > >    This is a shorthand notation for a type definition plus a value.
> > >    Optionally dependencies for this default value can be added with "if".
> > >
> > > -- dependencies: "depends on" <expr>
> > > +- dependencies: "depends on" <expr> ["if" <expr>]
> > >
> > >    This defines a dependency for this menu entry. If multiple
> > >    dependencies are defined, they are connected with '&&'. Dependencies
> > > @@ -130,6 +130,16 @@ applicable everywhere (see syntax).
> > >     bool "foo"
> > >     default y
> > >
> > > +  The dependency definition itself may be conditional by appending "if"
> > > +  followed by an expression. If such expression is false (n) then this
> > > +  dependency is ignored. One possible use case is:
> > > +
> > > +    config FOO
> > > +   tristate
> > > +   depends on BAZ if BAZ != n
> >
> > I presume this is the same as
> >
> >       depends on BAZ if BAZ
> >
> > which makes me wonder if that should be the example. At least current
> > usage for select is predominantly
> >
> >       select FOO if BAR
> >
> > without "!= n".
>
> Yes, it is the same thing. I prefer making the documentation a little
> more explicit than necessary so the explanation is really obvious.


For the case of 'select',

  select FOO if BAR != n

is NOT equivalent to:

  select FOO if BAR



I do not think "if <expr>" in Kconfig
is so easy to understand.
I tend to hesitate to extend it.

Sometimes, it means "the property is visible if <expr> != n".
Sometimes, not.




For the case of 'depends on',
the 'depends on' is effective if <expr> != n
because Nicolas implemented it in this way.



We can do:

    depends on X || X = n

instead of:

    depends on X || !X

        or

    depends on X if X






I guess the source of the complaint is
!X is difficult to understand
when X is tristate.

But, is there any confusion in 'X = n' ?
I think not.

-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2020-05-06 16:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-23 15:19 [PATCH] kconfig: allow for conditional dependencies Nicolas Pitre
2020-04-23 15:53 ` Jani Nikula
2020-04-23 16:05   ` Nicolas Pitre
2020-05-06 16:43     ` Masahiro Yamada

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).