pal_crypto.h file name conflict with Pelion client

[adamelhakham]

Description

There is a new file in the BLE stack called pal_crypto.h:
https://github.com/ARMmbed/mbed-os/blob/master/features/FEATURE_BLE/targets/TARGET_CORDIO/stack/platform/include/pal_crypto.h

• The problem is that the Pelion client has a porting layer called PAL and one of the files is called pal_Crypto.h. This causes build failures when building on a Windows machine (file system not case sensitive).
  It would be great if that file name can be changed, because Pelion Client should work out of the box.

• Another issue: When I try to remove the BLE feature (for the UBLOX_EVK_ODIN_W2 target) with the following line: "target.features_remove" : ["BLE"]
  The build fails for ARMCC 6. This seems to be another bug (unless I am not removing the BLE feature properly)

Issue request type

[ ] Question
[ ] Enhancement
[x ] Bug

[ciarmcom]

Internal Jira reference: https://jira.arm.com/browse/MBOCUSTRIA-977

[0xc0170]

@ARMmbed/mbed-os-pan Can you please review

[paul-szczepanek-arm]

The file belongs to an external lib - cordio bluetooth stack, I think the real solution is to fix the build system to require full include paths for includes that are not part of the feature being compiled.

[pan-]

To build on @paul-szczepanek-arm answer, the build system is the real offender here, we can add all the workaround in the world, this type of issue will continue to happen whenever a third party library is imported. As long as the build system continue to consider every header to be public there will be collision in common use cases (@geky @ARMmbed/mbed-os-tools ).

Current solution within the os is to fully qualify include paths; of course that doesn't prevent third party library to generate conflicts. As a workaround, I suggest we qualify paths for pal_crypto.h in the Cordio library and engage with the cordio team for a more sustainable solution. In the meantime @adamelhakham, could you make sure that include paths in Pelion client are properly qualified ?

[theotherjimmy]

fix the build system to require full include paths for includes that are not part of the feature being compiled.

That would break so many current projects, it's sort of funny to think about the developer backlash.

[donatieng]

That would break so many current projects, it's sort of funny to think about the developer backlash.

@theotherjimmy Yes - but the longer we wait before fixing this, the bigger the backlash will be when we can't afford not to do it anymore. And nothing would prevent us from leaving that "feature" enabled in user code by default, while starting doing things properly within the OS codebase.

[theotherjimmy]

@donatieng

And nothing would prevent us from leaving that "feature" enabled in user code by default, while starting doing things properly within the OS codebase.

Sounds like it needs to be configurable per-library, or something along those lines.

[bridadan]

I was going to create an issue to brainstorm some ideas about this. Seems like we need some kind of plan going forward. Having (almost) every directory being included as an include path already creates other problems with command line length.

[pan-]

@theotherjimmy Yes; I think some work can be done with the conditional build system . If library writers can define which path is visible to library consumer then the build system can make sure that headers internal to the library are not exposed to unrelated modules and public headers are not exposed to library that do not consume then.

[theotherjimmy]

@geky FYI

@pan- Great idea! I was thinking the same thing myself.

[0xc0170]

Besides good discussion around how to fix this properly, how to address this for 5.12.0-rc2/3 ? we got short time window. As cordio is external library, we might seek a fix at the pelion if this can be fixed there.

@adamelhakham, could you make sure that include paths in Pelion client are properly qualified ?

@adamelhakham we haven't yet received how this conflicting header file is included.

cc @jenia81

[adamelhakham]

@0xc0170 @pan- not sure what you mean by include path, I use mbed-cli which includes all the features included in the jsons and basically pretty much everything except for tests.
In our case, since targets.json adds the BLE feature for the UBLOX_EVK_ODIN_W2 target, this new pal_crypto.h is included by some source file in the BLE stack. However since we there is also our (Pelion Client) file called pal_Crypto,h and I used a windows machine to build the code the file that actually got included to the BLE source code is our pal_Crypto.h instead of pal_crypto.h of the BLE stack, thus failing the build.