mynewt-newt icon indicating copy to clipboard operation
mynewt-newt copied to clipboard

pkg.cflags from a dependency might be required by dependent packages

Open utzig opened this issue 6 years ago • 1 comments

When building mcuboot with RSA, the mbedtls package itself defines and is built with extra cflags (-DMBEDTLS_USER_CONFIG_FILE=...). While this builds mbedtls correctly other packages that only depend on include files from mbedtls don't seem to import those cflags.

My particular case is that I am trying to override mbedtls SW AES implementation with a HW one, in this case I defined MBEDTLS_AES_ALT and defined what I needed in aes_alt.h from my package (just follows mbedtls override method). But for bootutil to not fail building I have to add a similar pkg.cflags to its package as the one existing in mbedtls.

Not sure it's a bug or design decision, or maybe there's a more "correct" way of doing this. I also tried with a few older versions that still work with core master but had the same issues...

utzig avatar Feb 08 '19 15:02 utzig

I think newt's handling of pkg.cflags isn't ideal. Here is what newt does:

    // packages get applied to every source file:
    //     * target
    //     * app (if present)
    //     * bsp
    //     * compiler (not added here)
    //
    // In the case of conflicting flags, the higher priority package's flag
    // wins.  Package priorities are assigned as follows (highest priority
    // first):
    //     * target
    //     * app (if present)
    //     * bsp
    //     * <library package>
    //     * compiler
  1. For all other packages, pkg.cflags get applied only to that package's source files.

I understand the issue you raise and I do think it is a real problem. I don't know the best solution though. I'm nervous about having dependent packages inherit cflags from the packages they depend on because I think this could have some unintended consequences. Specifically, the same cflag may do different things to different packages (e.g., -DDEBUG).

We could have a separate field in pkg.yml (e.g., pkg.inheritable_cflags) but that feels like overengineering to me, and I'm not sure it prevents the above concern.

I feel like there as to be a good solution to the problem, but I can't think of one. Any ideas?

ccollins476ad avatar Jun 22 '20 18:06 ccollins476ad