mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Graham Roff <grahamr@qti.qualcomm.com>
To: nathan@kernel.org
Cc: arnd@arndb.de, corbet@lwn.net, grahamr@qti.qualcomm.com,
	linux-doc@vger.kernel.org, linux-kbuild@vger.kernel.org,
	linux-kernel@vger.kernel.org, nico@fluxnic.net, nsc@kernel.org
Subject: Re: [PATCH v2] Support conditional deps using "depends on X if Y"
Date: Tue,  9 Dec 2025 14:45:48 -0800	[thread overview]
Message-ID: <20251209224548.3885246-1-grahamr@qti.qualcomm.com> (raw)
In-Reply-To: <20251205182334.GB3974306@ax162>

> Other than that, this seems reasonable to me. The actual code changes
> are small and the tests prove this works properly. I won't pick up v3
> until after 6.19-rc1 is out at the least.

Thanks Nathan! I will send out a v3 with the commit text updated, and the test
code changes you suggested.

> > > On the surface, the vast majority these become more readable using the
> > > 'if' syntax.
> >
> > Agreed, the question is whether a small improvement in
> > readability is worth the complexity of having multiple
> > ways of expressing the same thing.
>
> It is a tradeoff. Sometimes it is advantageous to increase the 
> complexity in one place so other areas with more exposure to more people 
> are simplified.

Exactly, a small code update here provides a much simpler syntax for 
expressing a fairly common thing (conditional or optional dependencies).

It also makes the language more consistent since most other kconfig commands
accept a trailing "if <expr>" - I just assumed that "depends on" did as well
when first writing Kconfig files!

> >> +config TEST_COMPLEX
> >> +    bool "Test complex conditional dependency"
> >> +    depends on (FOO && BAR) if (FOO || BAR)
> >> +    default y
> 
> With the existing syntax, this could be expressed as
> 
>       depends on FOO = BAR
> 
> or
> 
>       depends on (FOO && BAR) || (!FOO && !BAR)
> 
> and I don't see how the new syntax is an improvement
> over these.

In this case I agree, the test was chosen to validate a complicated 
conditional, not to show a particularly useful real-world example.
I added much better examples earlier of where the "if" style provides
improved readability.
 
> Overall, I'm not convinced by this patch. I have no strong
> objection to anything in here, but I'm worried that extending
> the syntax adds more problems than this one solves.

Both syntaxes have their places, it comes down to which one is most
understandable to the reader. In a lot of cases it is definitely
easier to understand the intent of the expression using "if" rather
than ors and nots. Combine that with the simplicity of the change
and any problems this might add (not sure what those are though)
would seem on balance to be worth it.

Graham






  reply	other threads:[~2025-12-09 22:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-18 18:46 Graham Roff
2025-12-05  1:53 ` Nathan Chancellor
2025-12-05  8:01   ` Arnd Bergmann
2025-12-05 16:14     ` Nicolas Pitre
2025-12-05 18:23     ` Nathan Chancellor
2025-12-09 22:45       ` Graham Roff [this message]
2025-12-10 15:07       ` Jani Nikula
2025-12-13  1:40         ` Nathan Chancellor
2025-12-05  9:06 ` Jani Nikula

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=20251209224548.3885246-1-grahamr@qti.qualcomm.com \
    --to=grahamr@qti.qualcomm.com \
    --cc=arnd@arndb.de \
    --cc=corbet@lwn.net \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=nico@fluxnic.net \
    --cc=nsc@kernel.org \
    /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

all inboxes | Powered by JetHome®