[PATCH v5 02/29] kconfig: Add tools support to CONFIG_IS_ENABLED()

[Tom Rini]

On Thu, Oct 07, 2021 at 03:33:32PM -0500, Alex G. wrote:
> 
> 
> On 10/7/21 2:39 PM, Tom Rini wrote:
> > On Thu, Oct 07, 2021 at 02:32:42PM -0500, Alex G. wrote:
> > > 
> > > 
> > > On 10/7/21 1:50 PM, Simon Glass wrote:
> > > > Hi Tom,
> > > > 
> > > > On Thu, 7 Oct 2021 at 12:30, Tom Rini <trini at konsulko.com> wrote:
> > > > > 
> > > > > On Thu, Oct 07, 2021 at 12:02:24PM -0600, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > > 
> > > > > > On Thu, 7 Oct 2021 at 07:42, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > 
> > > > > > > On Thu, Oct 07, 2021 at 07:32:04AM -0600, Simon Glass wrote:
> > > > > > > > Hi Tom,
> > > > > > > > 
> > > > > > > > On Wed, 6 Oct 2021 at 20:52, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > > > 
> > > > > > > > > On Wed, Oct 06, 2021 at 08:49:13PM -0600, Simon Glass wrote:
> > > > > > > > > > Hi Tom,
> > > > > > > > > > 
> > > > > > > > > > On Wed, 6 Oct 2021 at 18:26, Tom Rini <trini at konsulko.com> wrote:
> > > > > > > > > > > 
> > > > > > > > > > > On Sat, Sep 25, 2021 at 07:43:15PM -0600, Simon Glass wrote:
> > > > > > > > > > > 
> > > > > > > > > > > > At present we must separately test for the host build for many options,
> > > > > > > > > > > > since we force them to be enabled. For example, CONFIG_FIT is always
> > > > > > > > > > > > enabled in the host tools, even if CONFIG_FIT is not enabled by the
> > > > > > > > > > > > board itself.
> > > > > > > > > > > > 
> > > > > > > > > > > > It would be more convenient if we could use, for example,
> > > > > > > > > > > > CONFIG_IS_ENABLED(FIT) and get CONFIG_HOST_FIT, when building for the
> > > > > > > > > > > > host. Add support for this.
> > > > > > > > > > > > 
> > > > > > > > > > > > With this and the tools_build() function, we should be able to remove all
> > > > > > > > > > > > the #ifdefs currently needed in code that is build by tools and targets.
> > > > > > > > > > > > 
> > > > > > > > > > > > This will be even nicer when we move to using CONFIG(xxx) everywhere,
> > > > > > > > > > > > since all the #ifdef and IS_ENABLED/CONFIG_IS_ENABLED stuff will go away.
> > > > > > > > > > > > 
> > > > > > > > > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > > > > > > > > Suggested-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk> # b4f73886
> > > > > > > > > > > > Reviewed-by: Alexandru Gagniuc <mr.nuke.me at gmail.com>
> > > > > > > > > > > 
> > > > > > > > > > > The problem here is we don't include <linux/kconfig.h> automatically
> > > > > > > > > > > when building host stuff, I believe.  This is why doing this breaks
> > > > > > > > > > > test_mkimage_hashes for me on am335x_evm with:
> > > > > > > > > > > /tmp/.bm-work/am335x_evm/tools/mkimage -D -I dts -O dtb -i /tmp/.bm-work/am335x_evm -f /home/trini/work/u-boot/u-boot/test/py/tests/vboot//hash-images.its /tmp/.bm-work/am335x_evm/test.fit
> > > > > > > > > > > *** stack smashing detected ***: <unknown> terminated
> > > > > > > > > > 
> > > > > > > > > > Oh dear, and no CI coverage.
> > > > > > > > > > 
> > > > > > > > > > I was reluctant to include kconfig.h everywhere but perhaps that is
> > > > > > > > > > the best approach. Will take a look ASAP.
> > > > > > > > > 
> > > > > > > > > Maybe we need to think a bit harder too about how we structure
> > > > > > > > > intentionally shared code.
> > > > > > > > > 
> > > > > > > > > Why not, for example, for these common algorithms, rely on typical
> > > > > > > > > system headers/libraries in the tooling, which means we validated U-Boot
> > > > > > > > > vs common reference, rather than just our implementations?
> > > > > > > > 
> > > > > > > > Do you mean we use openssl for sha1, for example?
> > > > > > > 
> > > > > > > I guess, yes.  Just flat out saying we require openssl for tools, and
> > > > > > > doing our best to not make compatibility with libressl difficult, seems
> > > > > > > likely to cause less headaches for people than what we already require
> > > > > > > in terms of Python.
> > > > > > 
> > > > > > I'm OK with that, although I do think the problem identified here
> > > > > > (CONFIG_SHA256 not enabled) is somewhat sideways from that. We already
> > > > > 
> > > > > OK, I've taken what you posted on IRC and folded that in, continuing
> > > > > tests now.
> > > > > 
> > > > > > use separate code paths to run hashing. Perhaps we could make it
> > > > > > optional?
> > > > > > 
> > > > > > What about those people that complain about crypto libraries on their systems?
> > > > > 
> > > > > I'm not sure how big a problem that really is, currently.  I guess one
> > > > > thing would be to make a separate thread on it, and put it in the next
> > > > > -rc email as well, for people to explain why it would be a hardship.
> > > > > That in turn, I think, is coming down to modern vs very old openssl
> > > > > support, rather than having any at all.
> > > > 
> > > > OK I'll take a look at some point.
> > > > 
> > > > Or perhaps Alex might like to?
> > > 
> > > We just got a complain about OpenSSL yesterday [1]
> > > 
> > > Alex
> > > 
> > > [1] https://lists.denx.de/pipermail/u-boot/2021-October/462728.html
> > 
> > Oh goodness, LibreELC is a custom build system... I'll have to chime in
> > there, thanks.
> 
> I am in favor of keeping libcrapto separate. We still need our own code for
> CRC32 and other weak or non-crypto hashes, a tidbit which makes me doubt the
> wisdom of relying entirely on an external lib.
> 
> I had to make a similar decision when writing the hashes test. Originally, I
> was going to use pyCrypto, crcelk, to re-hash everything and compare to
> mkimage. It turned out to be neither necessarry nor efficient.

Is there perhaps a happy medium?  Or do we just need to think harder on
how to make the code U-Boot needs shared between target and host tools
clean and clear and obvious enough?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20211007/51bd525e/attachment.sig>