Fix CI for branch feature-public-headers #11093
Conversation
|
@gpsimenos, thank you for your changes. |
evedon
added
release-version: feature branch
needs: CI
and removed
needs: review
labels
Test run: FAILEDSummary: 4 of 4 test jobs failed Failed test jobs:
|
gpsimenos
changed the title
Test run: FAILEDSummary: 4 of 4 test jobs failed Failed test jobs:
|
… into feature-public-headers
Add missing header file inclusion.
The "bare-metal" profile which is defined in mbed-os/platform/bare_metal/mbed_lib.json only includes only the `platform` and `drivers` dirs in the build. The problem is due to the fact that USB related modules are now included in the build as part of the `drivers` library. They previously were not included given they were outside the `drivers` directory (in the `usb` dir). The solution is to stick an mbed_lib.json file under `mbed-os/drivers/source/usb` in order to make it a library so it does not get automatically included given the `requires` statement in mbed-os/platform/bare_metal/mbed_lib.json
The Cmake file described hardcoded the relative path to the source file being tested. Changed it to be the new location of the source file.
Test run: FAILEDSummary: 3 of 11 test jobs failed Failed test jobs:
|
Modify compilation API to provide a list of paths to exclude from the build.
|
@madchutney @mark-edgeworth @SeppoTakalo @evedon could you please trigger the CI test again? Thanks. |
Test run: SUCCESSSummary: 11 of 11 test jobs passed |
| # a compilation queue. | ||
| compile_queue = ( | ||
| files_to_compile | ||
| if not exclude_paths |
There was a problem hiding this comment.
I think the ternary operator here makes the code more difficult to read, a simple if then else would be clearer IMO.
| if not exclude_paths | ||
| else [ | ||
| file_to_compile | ||
| for exclude_path in exclude_paths |
There was a problem hiding this comment.
I'm not sure this list comprehension does what you want. It works ok when there is a single exclude path, but when there are multiple exclude paths the file to compile with be included once for every exclude path.
I don't think it would be a bad idea to break out this functionality into a small function and unit test.
There was a problem hiding this comment.
The scope of this change is to provide a short-term workaround to maintain Mbed OS 2 compatibility until legacy build support is removed.
If the current code works when there is a single exclude path then let's add a comment to that effect and let's convert exclude_paths to a single exclude_path.
| @@ -397,14 +397,27 @@ def get_arch_file(self, objects): | |||
|
|
|||
| # THIS METHOD IS BEING CALLED BY THE MBED ONLINE BUILD SYSTEM | |||
| # ANY CHANGE OF PARAMETERS OR RETURN VALUES WILL BREAK COMPATIBILITY | |||
| def compile_sources(self, resources, inc_dirs=None): | |||
| def compile_sources(self, resources, inc_dirs=None, exclude_paths=None): | |||
There was a problem hiding this comment.
This addition of the exclude_paths parameter will mean that it will be included the statement:
# THIS METHOD IS BEING CALLED BY THE MBED ONLINE BUILD SYSTEM
# ANY CHANGE OF PARAMETERS OR RETURN VALUES WILL BREAK COMPATIBILITY
It would be possible to remove this parameter when legacy build support is removed but I'm sure by then we would have forgotten. My suggestion would be to create a private function which has this parameter but leave this interface as it is, e.g.
def compile_legacy_sources(self, resources, inc_dirs=None, exclude_paths=None)
return _compile_sources(resources, inc_dirs=inc_dirs, exclude_paths=exclude_paths)
def compile_sources(self, resources, inc_dirs=None)
return _compile_sources(resources, inc_dirs=inc_dirs)
def _compile_sources(self, resources, inc_dirs=None, exclude_paths=None)
...
I would also ask whether there is a different between dirs and paths the two different terminologies in the parameter list might be confusing.
* Fix rtos include path in NRFCordioHCIDriver * Flatten USB driver directory structure * Add missing include for us_ticker * Add more missing includes for us_ticker * Fix mbed_hal_fpga_ci_test_shield/uart test * Fix bare-metal build * Fix Watchdog UNITTEST * Fix Mbed OS 2 build for Public/Internal headers relocating
* Fix rtos include path in NRFCordioHCIDriver * Flatten USB driver directory structure * Add missing include for us_ticker * Add more missing includes for us_ticker * Fix mbed_hal_fpga_ci_test_shield/uart test * Fix bare-metal build * Fix Watchdog UNITTEST * Fix Mbed OS 2 build for Public/Internal headers relocating
Description
This PR fixes build errors in the "Feature public headers" PR.
Pull request type
Reviewers
Release Notes