ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.67k stars 2.98k forks source link

Incorrect handling of build system components #9189

Closed screamerbg closed 4 years ago

screamerbg commented 5 years ago

Description

The build system incorrectly handles a use case where "component_add": ["SD"] is "emitted" from a library, e.g. FEATURE_BOOTLOADER/mbed_lib.json. The build system will correctly add -DCOMPONENT_SD=1 macro, but it will not add relevant component folders to the include paths, thus fails to compile the codebase.

To reproduce:

  1. mbed import http://github.com/screamerbg/pelion-integration
  2. cd pelion-integration + mbed update 48bd908 - don't use master which already has a workaround in place for this issue.
  3. mbed compile -t ARM -m UBLOX_C030_U201 -c -v
  4. Observe that -DCOMPONENT_SD=1 is in the reported macros on the console, but the mbed-os/components/COMPONENT_SD path doesn't exist in the includes file in BUILD\UBLOX_C030_U201\ARM\.includes_<hash>.txt
  5. Edit mbed_app.json and add "target.components_add" : ["SD"], for entry UBLOX_C030_U201
  6. mbed compile -t ARM -m UBLOX_C030_U201 -c -v
  7. Observe that once again -DCOMPONENT_SD=1 is in the reported macros on the console, but this time the mbed-os/components/COMPONENT_SD path correctly is added to the includes file in BUILD\UBLOX_C030_U201\ARM\.includes_<hash>.txt

The codebase to reproduce this uses mbed-os-5.11.0 release.

Issue request type

[ ] Question
[ ] Enhancement
[X] Bug
cmonr commented 5 years ago

@screamerbg I take it the above instructions should be used with a particular sha for the pelion-integration repo? Also, is this for the ARMmbed repo, or your fork?

screamerbg commented 5 years ago

@cmonr Correct, although you can reproduce it with any code where target.components_add is "emitted" from a library (mbed_lib.json), but the application config (mbed_app.json) doesn't declare it, resulting the behavior explained above.

I've discussed this with @theotherjimmy in person about couple of weeks ago.

screamerbg commented 5 years ago

Bump @theotherjimmy @cmonr

ciarmcom commented 4 years ago

@screamerbg thank you for raising this issue.Please take a look at the following comments:

Could you add some more detail to the description? A good description should be at least 25 words. What target(s) are you using? What toolchain(s) are you using? What version of Mbed OS are you using (tag or sha)? It would help if you could also specify the versions of any tools you are using? How can we reproduce your issue?

NOTE: If there are fields which are not applicable then please just add 'n/a' or 'None'.This indicates to us that at least all the fields have been considered. Please update the issue header with the missing information, the issue will not be mirroredto our internal defect tracking system or investigated until this has been fully resolved.

adbridge commented 4 years ago

As this applies to the old tools I'm going to close this. @Patater can you add this to your list of things to ensure it isn't an issue with the new tools...