Add dependency checks to components #8875

deepikabhavnani · 2018-11-26T23:07:02Z

Description

Build dependency for components was not captured i.e. in case Network feature / RTOS is disabled (added to .mbedignore list), the build breaks. Resolving build issues in this PR.

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

theotherjimmy · 2018-11-27T15:08:10Z

@0xc0170 No python was changed in the making of this PR. Why does it need tools input?

deepikabhavnani · 2018-11-27T16:08:02Z

No python was changed in the making of this PR. Why does it need tools input?

May be my headline is confusing ?

cmonr · 2018-11-27T19:45:44Z

@deepikabhavnani Can you rebase this to see if the continuous-integration/travis-ci/push disappears?

"ns_types.h" is not used, but included. Results in build failure when networking feature is ignored with .mbedignored "mbed_trace.h" not used.

"ns_types.h" results in build failure when networking feature is ignored with .mbedignored. ESP8266 component should be enabled only when NSAPI is present.

…present Build failures were observed when rtos is ignored with .mbedignored. Components dependent on RTOS should be guarded with MBED_CONF_RTOS_PRESENT

deepikabhavnani · 2018-11-27T20:10:41Z

Can you rebase this to see if the continuous-integration/travis-ci/push disappears

Yes it does. Thanks

SeppoTakalo

Why cannot the non-RTOS build skip the whole components directory?

I don’t like this #ifdeffing because of one legacy feature.

0xc0170 · 2018-11-29T08:00:55Z

Why cannot the non-RTOS build skip the whole components directory?

We always do this config check for mbed 2. @ARMmbed/mbed-os-tools Please review

I don’t like this #ifdeffing because of one legacy feature.

True, ifdef is not be ideal but that is current approach. We will have something different in future.

SeppoTakalo · 2018-11-29T11:40:45Z

But it is not the current way... At least not the only one.

Currently we exclude quite a many folder from non-rtos builds. See:
https://github.com/ARMmbed/mbed-os/blob/master/.travis.yml#L177

0xc0170 · 2018-11-29T12:51:59Z

I see your point (this is using .mbedignore folder (user's task to add to some folders and in the scripts actually remove the folders). We will come up with better solution.

@deepikabhavnani this should follow what we have for features, shouldn't it ? Leave it as it is, and use .mbedignore folder (update our docs to reflect this, I recall there is at least a blog about this how to compile without rtos)

deepikabhavnani · 2018-11-29T17:07:35Z

Currently we exclude quite a many folder from non-rtos builds.

That is done to test event queues without RTOS. And practically user should be able to remove just RTOS and still build without breaking anything, since we are not capturing dependencies correctly we have to remove so many folders, this is the side effect. Event queue examples should work by just removing rtos rm -r rtos.

update our docs to reflect this, I recall there is at least a blog about this how to compile without rtos

We cannot cover all combinations in docs. Users can have storage (feature+compoents) and other features even when RTOS is disabled but not network.

Trying to address these issues :#7800
#7889

True, ifdef is not be ideal but that is current approach. We will have something different in future.

For now this is the solution we have. We also need to address it in feature (network / green tea) which I will do after this.

SeppoTakalo · 2018-11-30T08:14:07Z

We cannot cover all combinations in docs. Users can have storage (feature+compoents) and other features even when RTOS is disabled but not network.

This is not acceptable.
You are on other hand assuming that developers who have never worked with Mbed 2, would magically know the rules implied by this RTOS removal. And even without documenting it. Where is this rm -r rtos documented?

And on the other hand, you are placing requirements into teams that are not approved by product managers. This is rtos-less requires support, resources. We cannot spend resources without approval.

deepikabhavnani · 2018-12-07T15:50:19Z

@SeppoTakalo - Changes here are not related to non RTOS build or rm -rf in CI, which is not correct way of testing and hence not documented as well.

Changes here are related to documented .mbedignore feature. Build breaks when feature/network is added to .mbedignore. Also if we disable SERIAL in device_has we see build errors.

Changes are inline to your previous commits, just adding a few missed dependencies.

PelionIoT/atmel-rf-driver#59
#8327

SeppoTakalo · 2018-12-10T08:07:48Z

Changes here are related to documented .mbedignore feature.

Where are these documented?

I think I tried to raise this point earlier.. If there is one documented way, why does the CI do this differently?

SeppoTakalo · 2018-12-10T08:08:42Z

@artokin We need to pull in these changes to the Atmel driver repository as well.

deepikabhavnani · 2018-12-10T18:29:59Z

Where are these documented?

https://os.mbed.com/docs/v5.10/tools/ignoring-files-from-mbed-build.html

I think I tried to raise this point earlier.. If there is one documented way, why does the CI do this differently?

Not sure why, we can definitely ignore folders using .mbedignore instead rm

cmonr · 2018-12-10T20:31:51Z

CI started

mbed-ci · 2018-12-11T00:27:36Z

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

cmonr added needs: review release-version: 5.11.1 labels Nov 27, 2018

0xc0170 requested a review from a team November 27, 2018 06:43

deepikabhavnani removed the request for review from a team November 27, 2018 16:08

deepikabhavnani changed the title ~~Add dependency checks to component build~~ Add dependency checks to components Nov 27, 2018

Deepika added 3 commits November 27, 2018 13:55

Fix header files used in 802.15.4_RF

74ad47b

"ns_types.h" is not used, but included. Results in build failure when networking feature is ignored with .mbedignored "mbed_trace.h" not used.

Fix header files used in esp8266

6c6ef87

"ns_types.h" results in build failure when networking feature is ignored with .mbedignored. ESP8266 component should be enabled only when NSAPI is present.

Components if using RTOS modules should be enabled only when RTOS is …

b32c6ba

…present Build failures were observed when rtos is ignored with .mbedignored. Components dependent on RTOS should be guarded with MBED_CONF_RTOS_PRESENT

deepikabhavnani force-pushed the fix_components branch from 9500cf6 to b32c6ba Compare November 27, 2018 19:56

0xc0170 approved these changes Nov 28, 2018

View reviewed changes

0xc0170 requested a review from SeppoTakalo November 28, 2018 13:10

SeppoTakalo reviewed Nov 29, 2018

View reviewed changes

0xc0170 requested a review from bulislaw November 30, 2018 08:41

Add Serial + EventQueue dependency to ESP8266

76220fa

deepikabhavnani force-pushed the fix_components branch from b5ba116 to 76220fa Compare November 30, 2018 21:38

SeppoTakalo approved these changes Dec 10, 2018

View reviewed changes

bulislaw approved these changes Dec 10, 2018

View reviewed changes

artokin mentioned this pull request Dec 10, 2018

Sync with Mbed OS PelionIoT/atmel-rf-driver#75

Merged

artokin mentioned this pull request Dec 10, 2018

Sync with Mbed OS PelionIoT/mcr20a-rf-driver#29

Merged

0xc0170 added needs: CI and removed needs: review labels Dec 10, 2018

cmonr added ready for merge and removed needs: CI labels Dec 11, 2018

cmonr merged commit 61ade84 into ARMmbed:master Dec 11, 2018

cmonr added release-version: 5.11.0-rc3 and removed ready for merge release-version: 5.11.1 labels Dec 11, 2018

Add dependency checks to components #8875

Add dependency checks to components #8875

Uh oh!

Conversation

deepikabhavnani commented Nov 26, 2018

Description

Pull request type

Uh oh!

theotherjimmy commented Nov 27, 2018

Uh oh!

deepikabhavnani commented Nov 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmonr commented Nov 27, 2018

Uh oh!

deepikabhavnani commented Nov 27, 2018

Uh oh!

SeppoTakalo left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Nov 29, 2018

Uh oh!

SeppoTakalo commented Nov 29, 2018

Uh oh!

0xc0170 commented Nov 29, 2018

Uh oh!

deepikabhavnani commented Nov 29, 2018

Uh oh!

SeppoTakalo commented Nov 30, 2018

Uh oh!

deepikabhavnani commented Dec 7, 2018

Uh oh!

SeppoTakalo commented Dec 10, 2018

Uh oh!

SeppoTakalo commented Dec 10, 2018

Uh oh!

deepikabhavnani commented Dec 10, 2018

Uh oh!

cmonr commented Dec 10, 2018

Uh oh!

mbed-ci commented Dec 11, 2018

Test run: SUCCESS

Uh oh!

Uh oh!

deepikabhavnani commented Nov 27, 2018 •

edited

Loading