-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix mbed 2 builds #3864
Fix mbed 2 builds #3864
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fine to me! I'll start the mbed 2 bot since that's what was broken
@mbed-bot: TEST HOST_OSES=ALL |
Oh! I forgot we even had that old thing. |
[Build 1277] |
I don't see that type used anywhere in retarget header file? What was the warning/error? |
The error was retarget.h is currently responsible for garunteeing that the normally non-portable sys types are defined (ssize_t, off_t...), so it seems reasonable to include stddef as well. |
@geky Can you please rebase ? Idid it locally, plus tested it. However I cant push to your fork |
NOTE: The description of this PR gives no indication that this bug broke mbed 2 for the 5.4 rc1 build. There was also no issue raised against mbed-os for which this is the fix (until I raised one today having started to prepare mbed 2 v138 for release). Thus the significance of this PR was assumed to be much lower than it actual is!! This fix SHOULD have gone into RC2 but was not prioritised due to the lack of information. This may block the mbed 2 v138 release. |
@mbed-bot: TEST HOST_OSES=ALL |
/morph test |
[Build ${MBED_BUILD_ID}] |
@mbed-bot: TEST HOST_OSES=ALL |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
[Build 1286] |
@mbed-bot: TEST HOST_OSES=ALL |
Tests results look good (IAR does build now !), retriggered the CI job. |
[Build 1288] |
lpc1768 - |
@mbed-bot: TEST HOST_OSES=ALL |
/morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
[Build 1289] |
@mbed-bot: TEST HOST_OSES=ALL |
[Build 1290] |
So we tracked down the issue... it is no longer a problem in retarget, but rather a breakage that got in through mbed 2 testing while this issue was there. Tracked down to 51aa333, @theotherjimmy is looking into the fix which we can just append to this pr. |
Thanks keep us updated. |
I accidentally broke only uARM mbed 2 builds. Here is the story: When scanning for resources, toolchains look for any `TOOLCHAIN_<classname>` folders to include. These `<classname>`s mostly match the name passed in on the command line with one exception: `uARM` on the command line maps to `ARM_MICRO` the class. This would not be a problem except for the bug that I introduced in a prior commit. The bug is that the mbed2 library builds will use the name passed in on the command line to construct `TOOLCHAIN_<cli-name>`. This will not match when scanning. I fixed it by translating the `<cli-name>` into the `<classname>`.
Fixed. |
@mbed-bot: TEST HOST_OSES=ALL |
/morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
[Build 1292] |
@mbed-bot: TEST HOST_OSES=ALL |
[Build 1293] |
@mbed-bot: TEST HOST_OSES=ALL |
[Build 1294] |
LGTM! |
Some file must have been indirectly including something that brought in
size_t
, since this type is notably missing from the mbed 2 builds. Fixed by includingstddef.h
which includes this definitions.cc @bridadan