All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnout Vandecappelle <arnout@mind.be>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH v2 6/7] tpm2-tools: do not enforce dependency on tpm2-abrmd
Date: Thu, 24 Jan 2019 17:50:31 +0100	[thread overview]
Message-ID: <b169a705-b11b-cd83-4b74-f593f1d2e2fc@mind.be> (raw)
In-Reply-To: <20190117155827.GD2556@scaer>



On 17/01/2019 16:58, Yann E. MORIN wrote:
> But with imply, if you do something like:
> 
>     make distclean
>     make menuconfig
>         --> enable tpm2-tools
> 
> Then tpm2-abrmd is enabled.
> 
> But if you now go with:
> 
>     make distclean
>     make defconfig
>     make menuconfig
>         --> enable tpm2-tools
> 
> Then tpm2-abrmd is not enabled, because it was already disabled in the
> .config.

 This, for me, is the crux of the matter. I agree with Yann that this is
confusing. Especially because 'make some-defconfig; make menuconfig' is the
usual workflow. So the value of this imply is almost nothing in practice.

 So let me take this occasion to review the cases of imply that we already have
(obviously they're not yet written with the imply keyword).

BR2_ARC_ATOMIC_EXT
BR2_TARGET_ROOTFS_JFFS2_NOCLEANMARKER
BR2_PACKAGE_LUA_32BITS
BR2_PACKAGE_OPUS_FIXED_POINT
BR2_TOOLCHAIN_EXTERNAL_HAS_SSP
BR2_TOOLCHAIN_EXTERNAL_INET_RPC

 These are not confusing IMO because they only become visible after selecting
some other option that is not enabled by default.


BR2_PACKAGE_IFUPDOWN_SCRIPTS

 This one is somewhat less confusing because in the 'make defconfig; make
menuconfig' scenario, the option will already be enabled. However, if you later
on switch to a custom skeleton, the ifupdown-scripts will stay enabled. Still,
that is very similar to the situation for packages that got select'ed: once you
remove the option that caused that package to be enabled, it will stay enabled
even after you disable the option that triggered it.


 In conclusion, we currently already have some confusion caused by 'make
defconfig; make menuconfig' situations, but currently they only go in one
direction: something that was enabled will stay enabled even if you don't need
it any more. Using 'imply' in the way proposed by Peter would introduce a
different kind of confusion: options that don't get enabled though they should be.

 I would say, the design of defaults in Kconfig is simply wrong. To make it work
well, there should be tracking of whether a value was set automatically or by
the user. But let's not go there :-)

 So, that doesn't mean that imply should be banned entirely. It could still be
useful in some cases, like the ifupdown scripts.

 For the situations like the one in this patch, I would say that we could relax
a little the 'avoid extra per-package configuration options'. In fact, extra
configuration options in Config.in don't cost that much. They don't really make
the menus larger because they're only visible when the package is selected. And
the .mk handling is the same as for an automatic optional dependency. That said,
in this specific case of tpm2-tools, I have the feeling that an additional
option is not appropriate. Since the two packages are right next to each other,
that is almost the same as having the suboption. So I would go for the help text
instead.


 Regards,
 Arnout

  parent reply	other threads:[~2019-01-24 16:50 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-15 10:15 [Buildroot] [PATCH v2 1/7] tpm2-tss: do not enforce -fstack-protector-all Peter Korsgaard
2019-01-15 10:15 ` [Buildroot] [PATCH v2 2/7] tpm2-tss: fix build with BR2_FORTIFY_SOURCE_1 Peter Korsgaard
2019-01-16 13:26   ` Peter Korsgaard
2019-01-25  7:28   ` Peter Korsgaard
2019-01-15 10:15 ` [Buildroot] [PATCH v2 3/7] tpm2-tools: always disable hardening options Peter Korsgaard
2019-01-16 13:25   ` Peter Korsgaard
2019-01-25  7:28   ` Peter Korsgaard
2019-01-15 10:15 ` [Buildroot] [PATCH v2 4/7] tpm2-abrmd: do not enforce -fstack-protector-all Peter Korsgaard
2019-01-16 13:25   ` Peter Korsgaard
2019-01-25  7:29   ` Peter Korsgaard
2019-01-15 10:15 ` [Buildroot] [PATCH v2 5/7] tpm2-abrmd: fix build with BR2_FORTIFY_SOURCE_1 Peter Korsgaard
2019-01-16 13:25   ` Peter Korsgaard
2019-01-25  7:29   ` Peter Korsgaard
2019-01-15 10:15 ` [Buildroot] [PATCH v2 6/7] tpm2-tools: do not enforce dependency on tpm2-abrmd Peter Korsgaard
2019-01-15 20:43   ` Yann E. MORIN
2019-01-16 11:43     ` Peter Korsgaard
2019-01-17 15:58       ` Yann E. MORIN
2019-01-17 19:01         ` Peter Korsgaard
2019-01-28 21:23           ` Yann E. MORIN
2019-01-28 22:08             ` Peter Korsgaard
2019-01-24 16:50         ` Arnout Vandecappelle [this message]
2019-01-25 15:03           ` Peter Korsgaard
2019-01-15 10:15 ` [Buildroot] [PATCH v2 7/7] tpm2-abrmd: S80tpm2-abrmd: create pid file at startup Peter Korsgaard
2019-01-16 13:26   ` Peter Korsgaard
2019-01-25  7:30   ` Peter Korsgaard
2019-01-16 13:25 ` [Buildroot] [PATCH v2 1/7] tpm2-tss: do not enforce -fstack-protector-all Peter Korsgaard
2019-01-25  7:28 ` Peter Korsgaard

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=b169a705-b11b-cd83-4b74-f593f1d2e2fc@mind.be \
    --to=arnout@mind.be \
    --cc=buildroot@busybox.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.