make: process include and dep for external modules by cladmi · Pull Request #8987 · RIOT-OS/RIOT

cladmi · 2018-04-19T15:46:49Z

Allow using EXTERNAL_MODULE_DIRS for complete modules with configuration and dependencies as if they were in RIOT.

It gives them the same possibilities as packages when all files are already there and do not need to be downloaded.

Contribution description

Process Makefile.include for external modules. It is included after the others
so it could overwrite some of the configuration if wanted.

Process Makefile.dep for external modules. It is included before the others so
it could be parsed before setting 'default' values to dependencies.

Compile time testing

I added a test that checks at compile time the header inclusion, linking and parsing dependencies.

Pre-contrib

I needed to make USEMODULE_INCLUDES_ an immediate variable to allow using MAKEFILE_LIST to define an include directory. Not really sure why though :/ I know why the fix makes it work, not why it is required in the first place.
In the external module makefile include, I need to use an immediate variable to evaluate MAKEFILE_LIST directly.

Issues/PRs references

Would allow using modules without modifying RIOT with a mechanism like #7523

doc/doxygen/src/creating-modules.md

kaspar030 · 2018-04-20T09:40:00Z

Nice one, and one step towards USEMODULE_GIT += https://github.com/me/module.

cladmi · 2018-04-20T13:36:34Z

I reformulated and split in two bullet points for Makefile.include and Makefile.dep

@danpetry if you want to check the English too :)

cladmi · 2018-07-04T07:34:39Z

Rebased the conflicting file.

smlng

Tested with native on macOS, samr21-xpro, and arduino-mega2560 - runs perfectly on all of them.

smlng · 2018-07-04T07:58:39Z

ping @kaspar030

cladmi · 2018-08-21T11:35:00Z

Rebased on master. As it was merged after writing this PR, I found out that currently this PR does not talk about defining BUILDDEP in the external modules and does not use it in tests. This would show more how to define an external "package" like module. I can add this in followup PR.

cladmi · 2018-09-10T14:09:58Z

Ping @kaspar030

kYc0o · 2018-09-19T13:06:40Z

@kaspar030 are you satisfied with the rewording?

kYc0o · 2018-10-08T11:31:36Z

@kaspar030 ping.

kYc0o · 2018-10-19T21:22:26Z

@jia200x would you like to have this for the release? If so, we need to dismiss @kaspar030 review.

jia200x · 2018-10-22T08:11:27Z

@jia200x would you like to have this for the release? If so, we need to dismiss @kaspar030 review.

Would be nice to have this for the Release. @kaspar030 last call?

jia200x · 2018-10-22T08:28:37Z

@jia200x would you like to have this for the release? If so, we need to dismiss @kaspar030 review.

Would be nice to have this for the Release. @kaspar030 last call?

kaspar030 · 2018-10-22T08:40:53Z

tests/external_module_dirs/README.md

+
+ * Adding a module with source code
+ * Setting a header include directory
+ * Adding dependences, which are evaluated before other modules dependencies


dependencies

kaspar030 · 2018-10-22T08:43:16Z

are you satisfied with the rewording?

Yes.

kaspar030 · 2018-10-22T10:10:00Z

are you satisfied with the rewording?

Yes.

kaspar030 · 2018-10-22T10:37:25Z

are you satisfied with the rewording?

Yes.

cladmi · 2018-10-22T12:34:54Z

Updated the last comment.

jia200x · 2018-10-23T08:23:02Z

@kaspar030 are you OK with the changes? Could you (N)ACK?

kaspar030 · 2018-10-23T08:39:34Z

lgtm

kaspar030 · 2018-10-23T08:40:25Z

@cladmi pls squash!

Process `Makefile.include` for external modules. It is included after the others so it could overwrite some of the configuration if wanted. Process `Makefile.dep` for external modules. It is included before the others so it could be parsed before setting 'default' values to dependencies.

It demonstrates: * Adding a module with source code * Setting a header include directory * Adding dependences, which are evaluated before other modules dependencies If the application compiles, everything is ok.

cladmi · 2018-10-23T11:25:02Z

Squashed, waiting for murdock.

jia200x · 2018-10-23T12:35:56Z

murdock is happy. Let's merge it then ;)

jia200x · 2018-10-23T12:37:06Z

@cladmi thank you for this contribution

cladmi · 2018-10-23T12:55:10Z

Thanks all for reviewing and merging.

cladmi added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 19, 2018

cladmi requested review from jia200x, jnohlgard and kaspar030 April 19, 2018 15:47

cladmi mentioned this pull request Apr 19, 2018

RFC: Set PSEUDOMODULES in packages Makefile.include #8984

Closed

cladmi force-pushed the pr/improve/external_modules branch from e1db08c to f55affb Compare April 20, 2018 09:04

kaspar030 requested changes Apr 20, 2018

View reviewed changes

doc/doxygen/src/creating-modules.md Outdated Show resolved Hide resolved

cladmi force-pushed the pr/improve/external_modules branch from f55affb to 33f3fe0 Compare April 20, 2018 13:35

cladmi mentioned this pull request Apr 23, 2018

make: Add 'BUILDDEPS' variable #9002

Merged

cladmi added this to the Release 2018.07 milestone Jul 4, 2018

cladmi force-pushed the pr/improve/external_modules branch from 33f3fe0 to c0b4997 Compare July 4, 2018 07:34

cladmi requested a review from smlng July 4, 2018 07:34

smlng approved these changes Jul 4, 2018

View reviewed changes

cladmi force-pushed the pr/improve/external_modules branch from c0b4997 to 38ef889 Compare July 5, 2018 17:11

cladmi mentioned this pull request Jul 6, 2018

make/flash: make flasher take the file used for flashing from command line argument #9514

Closed

cladmi modified the milestones: Release 2018.07, Release 2018.10 Aug 1, 2018

cladmi force-pushed the pr/improve/external_modules branch from 38ef889 to 03e9421 Compare August 21, 2018 11:30

cladmi mentioned this pull request Aug 21, 2018

Support for self contained modules #9811

Closed

18 tasks

jia200x mentioned this pull request Aug 29, 2018

TF: Configuration Task Force status #9856

Closed

3 tasks

cladmi mentioned this pull request Sep 6, 2018

makefiles/docker.inc.mk: Use directories in RIOT when possible #9646

Merged

cladmi force-pushed the pr/improve/external_modules branch from 03e9421 to bb89a8a Compare September 10, 2018 10:27

cladmi force-pushed the pr/improve/external_modules branch from bb89a8a to 578b66d Compare October 21, 2018 16:59

kaspar030 reviewed Oct 22, 2018

View reviewed changes

kaspar030 approved these changes Oct 23, 2018

View reviewed changes

cladmi added 2 commits October 23, 2018 13:22

tests/external_module_dir: test for EXTERNAL_MODULE_DIRS

4157a07

It demonstrates: * Adding a module with source code * Setting a header include directory * Adding dependences, which are evaluated before other modules dependencies If the application compiles, everything is ok.

cladmi force-pushed the pr/improve/external_modules branch from c084430 to 4157a07 Compare October 23, 2018 11:22

jia200x merged commit 13312ef into RIOT-OS:master Oct 23, 2018

cladmi deleted the pr/improve/external_modules branch October 23, 2018 12:56

gdoffe mentioned this pull request Feb 26, 2021

Fix EXTERNAL_MODULE_DIRS handling #16103

Closed

Conversation

cladmi commented Apr 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Compile time testing

Pre-contrib

Issues/PRs references

Uh oh!

Uh oh!

kaspar030 commented Apr 20, 2018

Uh oh!

cladmi commented Apr 20, 2018

Uh oh!

cladmi commented Jul 4, 2018

Uh oh!

smlng left a comment

Choose a reason for hiding this comment

Uh oh!

smlng commented Jul 4, 2018

Uh oh!

cladmi commented Aug 21, 2018

Uh oh!

cladmi commented Sep 10, 2018

Uh oh!

kYc0o commented Sep 19, 2018

Uh oh!

kYc0o commented Oct 8, 2018

Uh oh!

kYc0o commented Oct 19, 2018

Uh oh!

jia200x commented Oct 22, 2018

Uh oh!

jia200x commented Oct 22, 2018

Uh oh!

kaspar030 Oct 22, 2018

Choose a reason for hiding this comment

Uh oh!

kaspar030 commented Oct 22, 2018

Uh oh!

kaspar030 commented Oct 22, 2018

Uh oh!

kaspar030 commented Oct 22, 2018

Uh oh!

cladmi commented Oct 22, 2018

Uh oh!

jia200x commented Oct 23, 2018

Uh oh!

kaspar030 commented Oct 23, 2018

Uh oh!

kaspar030 commented Oct 23, 2018

Uh oh!

cladmi commented Oct 23, 2018

Uh oh!

jia200x commented Oct 23, 2018

Uh oh!

jia200x commented Oct 23, 2018

Uh oh!

cladmi commented Oct 23, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cladmi commented Apr 19, 2018 •

edited

Loading