Skip to content
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

[tools] build_everything.py should fail immediately for unsupported target #2323

Closed
zgoda opened this issue Jul 31, 2016 · 7 comments
Closed

Comments

@zgoda
Copy link
Contributor

zgoda commented Jul 31, 2016

For not mbed-os capable platforms build process should not be attempted. Now it fails in the middle trying to compile RTOS support:

Failed to build library
./rtos/rtx/TARGET_CORTEX_M/RTX_Conf_CM.c:65:6: error: #error "no target defined"
 #    error "no target defined"
      ^
./rtos/rtx/TARGET_CORTEX_M/RTX_Conf_CM.c:105:6: error: #error "no target defined"
 #    error "no target defined"
      ^
./rtos/rtx/TARGET_CORTEX_M/RTX_Conf_CM.c:269:6: error: #error "no target defined"
 #    error "no target defined"
      ^

Found when testing PR #2285

@ciarmcom
Copy link
Member

ciarmcom commented Aug 1, 2016

ARM Internal Ref: IOTMORF-92

@0xc0170
Copy link
Contributor

0xc0170 commented Aug 9, 2016

@zgoda What are you proposing, that a script should fail ,not even trying to compile sources? The build system does not have currently a clue if rtos is supported for a platform or not, that one is in the rtos sources encoded (will be moved to configuration system most probably, the we can check if its supported or not).

cc @bridadan @theotherjimmy

@bridadan
Copy link
Contributor

bridadan commented Aug 9, 2016

@zgoda Thanks for bringing up this issue.

I should give a little background on this script and how it relates to the other build scripts in the tools.

Before this script, there was build_release.py. This was (and is) used to create binary releases for developer.mbed.org. It just compiles the HAL (no RTOS) for a list of targets and toolchains and packages it up into a library. When we first started to work on the mbed OS 5 release, I wanted to have a similar script to use in CI, so I created a separate one instead of changing the existing one.

Since then the workflow has changed a bit, and the CI now uses mbed CLI extensively. But I'd like to revisit this so you can run the full build locally that the CI does as well. There are some complications due to the way Jenkins needs the commands to be organized, but nothing that's a show stopper.

Now getting back to @0xc0170's comment, the script should probably follow the same flow that the build_release.py script does, which is build up a list of the supported targets, then build them. The list of supported targets for version 5 and version 2 are different though (due to RTOS support and other things). And the things it builds will be different as well (RTOS, tests, etc). However, I'd really like to merge this into one "release" script that knows how to build 2 and 5, and you just pick which version you want to build.

Really, both scripts should be revisited, and I think build_everything.py needs work since its fairly out of sync with the other tools.

@sg- sg- removed the mirrored label Aug 12, 2016
@zgoda
Copy link
Contributor Author

zgoda commented Aug 14, 2016

I don't feel in a position to suggest anything related to mbed os 5 yet, i'm completely new here. It just feels strange to have a tool that fails in the middle of process even if the outcome is known upfront. Shouldn't it fail immediately indicating the operation is not supported for given operands? I think so.

@bridadan
Copy link
Contributor

I think you're right @zgoda, the main issue right now is these scripts don't make it clear what part of the code they are building, so it is not sure if the platform is "supported" or "unsupported". We need to rethink how these scripts fit into the workflow so it is clear what platforms are supported in which configuration.

@sg-
Copy link
Contributor

sg- commented Sep 22, 2016

@bridadan Is this able to be closed?

@bridadan
Copy link
Contributor

@sg- I think it's probably safe to close this issue. I alluded to this above, but the build_everything.py script is pretty out of date and should honestly probably just be removed. If no one has any issue with this I'll submit a PR for it

@sg- sg- closed this as completed Sep 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants