search

platformio / builder-framework-mbed

ARM mbed build script for PlatformIO Build System
http://platformio.org/frameworks/mbed
Apache License 2.0
4 stars 17 forks source link

Load .mbedignore from project root directory #26

Closed KKoovalsky closed 1 year ago

KKoovalsky commented 2 years ago

Description

If .mbedignore is present in the root directory of the project, it will be applied to the build.

The .mbedignore supports official file format. Moreover, the .mbedignore files used within a pure mbed project (without pio), can be used without changes as well.

Additionally, to prevent from confusion, other patterns prefixes are supported too:

mbed-os/features/cellular/*
framework-mbed/features/cellular/*
features/cellular/*

are equivalent and will result in excluding:

features/cellular/*

The following commit messages explain more details:

Background

This solution came out from this forum post. Maximilian Gerhardt suggested to use self.ignore_dirs to implement that feature. Unfortunately, there is a bug in MBED's build API.

Copper-Bot commented 2 years ago

Hello @KKoovalsky,

I just tried on my side, and it didn't work.

Bad news: It seems that PIO doesn't use the root directory as ROOT for paths related to MBED framework, but subfolders instead (related to this).

I tried to remove mbed-os AND the first subfolder on each line of the .mbedignore, and then the feature works.

So:

mbed-os/features/cellular/*
framework-mbed/features/cellular/*
features/cellular/*

must be:

cellular/*

Problem, when you have lines like these:

mbed-os/TEST_APPS/*
mbed-os/TESTS/*
mbed-os/UNITTESTS/*

... Then you can't apply the feature, because it will result as *, and then the entire framework will be excluded ^^

Right now the only solution I can think of is to remove all * lines in the self.ignore_dirs list, after mapping the file.

KKoovalsky commented 2 years ago

Is there a reason why those lines:

mbed-os/TEST_APPS/*
mbed-os/TESTS/*
mbed-os/UNITTESTS/*

Do appear in the .mbedignore? I guess those files will not be compiled during pio run anyway?

Copper-Bot commented 2 years ago

Humm maybe your right, it come from a complete .mbedignore where the author tried to analyse all folders in the MBED framework, but now you mentioned it, it should not be read by MBED anyway ...

About my problem above, in your code there already is a _remove_first_directory_name_from_patterns function, so I don't understand why it doesn't work on my side.

KKoovalsky commented 2 years ago

You are completely right in this comment https://github.com/platformio/builder-framework-mbed/pull/26#issuecomment-933312503. I was asking for the directories:

mbed-os/TEST_APPS/*
mbed-os/TESTS/*
mbed-os/UNITTESTS/*

To find out whether removal of lines with * is sufficient, or whether we should apply different logic for the directories ignored at the first level of the directory tree of the MBED framework.

Copper-Bot commented 2 years ago

Well, I think the best thing to do is to describe the ignore path from the framework root folder, and not from his subfolders.

That could resolve a lot of things, especially if there are two sub-subfolders with the same name, but with different parent folder name. Another thing, if the user wants to ignore an entire subfolder, he can't (for example mbed-os/connectivity). And that should be easier for the feature as well to describe the ignore_paths.

Unfortunately, it doesn't seem to be possible ... I tried to replace 

mbed-os/features/cellular/*

by

../features/cellular/*

but it didn't work

KKoovalsky commented 2 years ago

@Copper-Bot

Well, I think the best thing to do is to describe the ignore path from the framework root folder, and not from his subfolders. That would be cool, but unfortunately it is hard to implement. The problem is that the MBED API asks whether the relative path is ignored. So the query parameter for is_ignored is, e.g. source/TARGET_CORTEX/rtx5/Include/cmsis_os2.h for the file rtos/source/TARGET_CORTEX/rtx5/Include/cmsis_os2.h. The API cuts the first folder name on its own.

That could resolve a lot of things, especially if there are two sub-subfolders with the same name, but with different parent folder name. Is it often that folder paths overlap and if yes, is the workaround, to specify more .mbedignore paths, fixing it? Let's say, the directory tree looks like that:

a/b/c/d --- 1 # To be ignored
| --- 2 # To be ignored
b/c/d --- 3 # Must stay
| --- 4 # To be ignored

Then the workaround would be to put to the .mbedignore:

a/b/c/d/1/*
a/b/c/d/2/*
b/c/d/4/*

One more thing: https://os.mbed.com/blog/entry/Introducing-the-new-Mbed-Tools/. It has been mentioned in the answer to the issue I have raised in MBED OS: https://github.com/ARMmbed/mbed-os/issues/15125#issuecomment-933231426.

Collecting all that, my suggestion is to fix the issue related to pattern "*" that is applied when the user wants to ignore contents of first-level subdirectory (the one explained here: https://github.com/platformio/builder-framework-mbed/pull/26#issuecomment-933312503). If the old mbed-tools are going to be abandoned, let's keep this improvement simple, document what is supported and what is not supported, and swallow the limitations. I guess most cases will be covered with the PR in such a form.

Copper-Bot commented 2 years ago

I'm agree, let's keep it simple 👍

Copper-Bot commented 2 years ago

Hello @KKoovalsky,

Does the script work on your side ?

It seems that _remove_first_directory_name_from_patterns function does not work as expected, I have to manually remove the first directory (and the mbed-os/) in the .mbedignore to get the feature to work correctly.

Maybe it is a Windows issue ? (I am thinking something about / and \)

KKoovalsky commented 2 years ago

Yes, most probably you are right. Could you send your .mbedignore?

Copper-Bot commented 2 years ago

I tried to use this one :

/* Bootloader */
mbed-os/features/FEATURE_BOOTLOADER/*

/* BLE */
mbed-os/connectivity/drivers/ble/*
mbed-os/connectivity/FEATURE_BLE/*

/* Cellular */
mbed-os/connectivity/cellular/*
mbed-os/connectivity/drivers/cellular/*
mbed-os/connectivity/netsocket/source/Cellular*.*

/* Device Key */
mbed-os/drivers/device_key/*

/* Experimental */
mbed-os/platform/FEATURE_EXPERIMENTAL_API/*

/* FPGA */
mbed-os/features/frameworks/COMPONENT_FPGA_CI_TEST_SHIELD/*

/* Greentea client */
mbed-os/features/frameworks/greentea-client/*

/* LORAWAN */
mbed-os/connectivity/drivers/lora/*
mbed-os/connectivity/lorawan/*

/* LWIP */
mbed-os/connectivity/drivers/emac/*
mbed-os/connectivity/lwipstack/*

/* Mbed-client-cli */
mbed-os/features/frameworks/mbed-client-cli/*

/* MBED TLS */
mbed-os/connectivity/drivers/mbedtls/*
mbed-os/connectivity/mbedtls/*

/* Nanostack */
mbed-os/connectivity/drivers/emac/*
mbed-os/connectivity/libraries/mbed-coap/*
mbed-os/connectivity/libraries/nanostack-libservice/*
mbed-os/connectivity/libraries/ppp/*
mbed-os/connectivity/nanostack/*

/* Netsocket */
mbed-os/connectivity/drivers/emac/*
mbed-os/connectivity/netsocket/*
mbed-os/connectivity/libraries/mbed-coap/*
mbed-os/connectivity/libraries/ppp/*

/* NFC */
mbed-os/connectivity/drivers/nfc/*
mbed-os/connectivity/nfc/*

/* RF */
mbed-os/connectivity/drivers/802.15.4_RF/*

/* Storage */
mbed-os/storage/filesystem/*
mbed-os/storage/kvstore/*
mbed-os/storage/platform/*

/* Tests */
mbed-os/platform/tests/*
mbed-os/TEST_APPS/*
mbed-os/TESTS/*
mbed-os/UNITTESTS/*

/* Unity */
mbed-os/features/frameworks/unity/*

/* Utest */
mbed-os/features/frameworks/utest/*

/* USB */
mbed-os/drivers/usb/source/*
mbed-os/hal/usb/source/*
mbed-os/hal/usb/TARGET_Templates/*

/* WiFi */
mbed-os/connectivity/drivers/wifi/*
KKoovalsky commented 2 years ago

@Copper-Bot I can see that you are quite handy with the code. Could I ask you to add line:

print(self._ignore_patterns)

below that line: https://github.com/platformio/builder-framework-mbed/pull/26/files#diff-d5c2212e2381df5ec71acb92b8a3a56b0af7e54ca57b63729a54f09f306e8808R51

And run:

pio run -t clean && pio run

and upload the output? :)

Copper-Bot commented 2 years ago

@KKoovalsky here's what I get :

$ pio run -t clean && pio run
Processing nucleo_f446re (platform: ststm32; board: nucleo_f446re; framework: mbed)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Build environment is clean
========================================================================================= [SUCCESS] Took 0.94 seconds =========================================================================================

Processing nucleo_f446re (platform: ststm32; board: nucleo_f446re; framework: mbed)
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Verbose mode can be enabled via `-v, --verbose` option
CONFIGURATION: https://docs.platformio.org/page/boards/ststm32/nucleo_f446re.html
PLATFORM: ST STM32 (14.2.0) > ST Nucleo F446RE
HARDWARE: STM32F446RET6 180MHz, 128KB RAM, 512KB Flash
DEBUG: Current (stlink) On-board (stlink) External (blackmagic, cmsis-dap, jlink)
PACKAGES:
 - framework-mbed 6.60900.210318 (6.9.0)
 - toolchain-gccarmnoneeabi 1.90201.191206 (9.2.1)
Collecting mbed sources...
Loaded .mbedignore patterns from: D:\PIO\test_mbedignore\.mbedignore
['\\* bootloader *\\', 'mbed-os\\features\\feature_bootloader\\*', '\\* ble *\\', 'mbed-os\\connectivity\\drivers\\ble\\*', 'mbed-os\\connectivity\\feature_ble\\*', '\\* cellular *\\', 'mbed-os\\connectivity\\cellular\\*', 'mbed-os\\connectivity\\drivers\\cellular\\*', 'mbed-os\\connectivity\\netsocket\\source\\cellular*.*', '\\* device key *\\', 'mbed-os\\drivers\\device_key\\*', '\\* experimental *\\', 'mbed-os\\platform\\feature_experimental_api\\*', '\\* fpga *\\', 'mbed-os\\features\\frameworks\\component_fpga_ci_test_shield\\*', '\\* greentea client *\\', 'mbed-os\\features\\frameworks\\greentea-client\\*', '\\* lorawan *\\', 'mbed-os\\connectivity\\drivers\\lora\\*', 'mbed-os\\connectivity\\lorawan\\*', '\\* lwip *\\', 'mbed-os\\connectivity\\drivers\\emac\\*', 'mbed-os\\connectivity\\lwipstack\\*', '\\* mbed-client-cli *\\', 'mbed-os\\features\\frameworks\\mbed-client-cli\\*', '\\* mbed tls *\\', 'mbed-os\\connectivity\\drivers\\mbedtls\\*', 'mbed-os\\connectivity\\mbedtls\\*', '\\* nanostack *\\', 'mbed-os\\connectivity\\drivers\\emac\\*', 'mbed-os\\connectivity\\libraries\\mbed-coap\\*', 'mbed-os\\connectivity\\libraries\\nanostack-libservice\\*', 'mbed-os\\connectivity\\libraries\\ppp\\*', 'mbed-os\\connectivity\\nanostack\\*', '\\* netsocket *\\', 'mbed-os\\connectivity\\drivers\\emac\\*', 'mbed-os\\connectivity\\netsocket\\*', 'mbed-os\\connectivity\\libraries\\mbed-coap\\*', 'mbed-os\\connectivity\\libraries\\ppp\\*', '\\* nfc *\\', 'mbed-os\\connectivity\\drivers\\nfc\\*', 'mbed-os\\connectivity\\nfc\\*', '\\* rf *\\', 'mbed-os\\connectivity\\drivers\\802.15.4_rf\\*', '\\* storage *\\', 'mbed-os\\storage\\filesystem\\*', 'mbed-os\\storage\\kvstore\\*', 'mbed-os\\storage\\platform\\*', '\\* tests *\\', 'mbed-os\\platform\\tests\\*', 'mbed-os\\test_apps\\*', 'mbed-os\\tests\\*', 'mbed-os\\unittests\\*', '\\* unity *\\', 'mbed-os\\features\\frameworks\\unity\\*', '\\* utest *\\', 'mbed-os\\features\\frameworks\\utest\\*', '\\* usb *\\', 'mbed-os\\drivers\\usb\\source\\*', 'mbed-os\\hal\\usb\\source\\*', 'mbed-os\\hal\\usb\\target_templates\\*', '\\* wifi *\\', 'mbed-os\\connectivity\\drivers\\wifi\\*']
LDF: Library Dependency Finder -> http://bit.ly/configure-pio-ldf
LDF Modes: Finder ~ chain, Compatibility ~ soft
Found 0 compatible libraries
Scanning dependencies...
No dependencies
Building in release mode
Compiling .pio\build\nucleo_f446re\FrameworkMbedcmsis\CMSIS_5\CMSIS\RTOS2\RTX\Config\RTX_Config.o
Compiling .pio\build\nucleo_f446re\FrameworkMbedcmsis\CMSIS_5\CMSIS\RTOS2\RTX\Library\cmsis_os1.o
Compiling .pio\build\nucleo_f446re\FrameworkMbedcmsis\CMSIS_5\CMSIS\RTOS2\RTX\Source\TOOLCHAIN_GCC\TARGET_RTOS_M4_M7\irq_cm4f.o
Compiling .pio\build\nucleo_f446re\FrameworkMbedcmsis\CMSIS_5\CMSIS\RTOS2\RTX\Source\rtx_delay.o
Compiling .pio\build\nucleo_f446re\FrameworkMbedcmsis\CMSIS_5\CMSIS\RTOS2\RTX\Source\rtx_evflags.o
Compiling .pio\build\nucleo_f446re\FrameworkMbedcmsis\CMSIS_5\CMSIS\RTOS2\RTX\Source\rtx_evr.o
Compiling .pio\build\nucleo_f446re\FrameworkMbedcmsis\CMSIS_5\CMSIS\RTOS2\RTX\Source\rtx_kernel.o
Compiling .pio\build\nucleo_f446re\FrameworkMbedcmsis\CMSIS_5\CMSIS\RTOS2\RTX\Source\rtx_lib.o
Compiling .pio\build\nucleo_f446re\FrameworkMbedcmsis\CMSIS_5\CMSIS\RTOS2\RTX\Source\rtx_memory.o
Compiling .pio\build\nucleo_f446re\FrameworkMbedcmsis\CMSIS_5\CMSIS\RTOS2\RTX\Source\rtx_mempool.o
.......

The rest is building log.

Copper-Bot commented 2 years ago

Ok I found a solution, in this file, just replace this line:

lambda p: p.replace('mbed-os/', '').replace('framework-mbed/', ''),

by this one:

lambda p: p.replace('\\','/').replace('mbed-os/', '').replace('framework-mbed/', ''),

And then the script work on Windows 🎉 It should still work on Linux, because it uses / by default.

valeros commented 2 years ago

Hi guys, thanks for the PR and valuable discussion. If I recall correctly the Mbed build API v1 is no longer recommended for new designs as the Mbed OS project is being gradually (if not already) switched to CMake as their main build system (even though it's still in beta). Long story short, I'm afraid we will need to completely refactor the PlatformIO build script for Mbed in order to use CMake as the source of build information (source files, build flags. etc.), so I believe the current implementation will undergo significant changes. What's even more complicates the matter is that PlatformIO also supports Mbed v5 which depends on the legacy tools, so will need to give much thought on how to properly implement build scripts for the future releases of Mbed.

KKoovalsky commented 2 years ago

@valeros Are there plans for pio to switch to CMake as well? That would simplify supporting other libraries, I guess.

Does it mean that this PR is not going to be reviewed and possibly merged?

valeros commented 2 years ago

Hi @KKoovalsky! Yes, we plan to use CMake once we have spare time to refactor build scripts and integrate new releases of Mbed. Hopefully, this will also resolve a long-standing issue with the current build workflow when all framework source files are compiled regardless of what features are used in a project. We really appreciate the work you've done here, I can review and even merge the PR, but what's the point if it won't land in the next release of the PlatformIO build scripts for Mbed?

KKoovalsky commented 2 years ago

I see. I agree then, I don't want to take your time for something what may be killed soon.

Copper-Bot commented 2 years ago

If I understand correctly, judging by this discussion, it seems that, when MBED use Ninja/CMake to build project, it will not need .mbedignore anymore, since it will build only necessary files. (Yeah 🎉!)

So if PIO use CLI2 in the future (or the CMake build system behind that), then .mbedignore will not be a necessity anymore to reduce build time, since it will already be optimised by the framework. At least for MBED OS greater than 6.5, judging by the documentation. For older MBED OS 6 versions, or for MBED 5, .mbedignore could still be necessary.

Copper-Bot commented 1 year ago

Hello @valeros,

Sorry to bump again this PR, Could this feature still be merged ?

It seems that Mbed CLI 2 (or Cmake/Ninja implementation) take a very long time to get out of BETA status (if not abandoned ?).

Also @KKoovalsky could it be possible please to add the path fix for Windows in the PR for a possible merge ?

This could still be very useful for users working with MbedOS 6 framework.

I am still using Mbed CLI 1 with Mbed 6.17 (last current version of MbedOS), it still works great, but it still needs a mbedignore file to reduce compilation time. It should not be a problem for PlatformIO to update to the latest MbedOS version using the current building system.

Best regards.

Copper-Bot commented 1 year ago

Thanks @valeros it works great 🎉 And also thanks for updating Mbed to 6.17