WIP RFC Replace usage of USEPKG by USEMODULE. by cladmi · Pull Request #9307 · RIOT-OS/RIOT

cladmi · 2018-06-07T15:42:48Z

Contribution description

This addresses the inconsistent use of USEPKG mentioned in #6668.
Currently USEPKG is required to tell the build system to take pkg/PKG_NAME into account.

It was argued that it makes the user aware he is using a package as he is required to put USEPKG, but packages are being pulled as dependencies for some cpu. And also, some packages, like lwip are implemented by several modules and doing USEMODULE = lwip_ipv6 triggers the inclusion of the package transparently.

This PR tries to implement the proposed solution

I propose making USEMODULE += implicitly add to USEPKG, iff pkg/ exists.

The build system modification is done around these changes:

diff --git a/Makefile.dep b/Makefile.dep
-# include package dependencies
--include $(USEPKG:%=$(RIOTPKG)/%/Makefile.dep)
+# process package dependencies
+-include $(ALL_PKGS:%=$(RIOTPKG)/%/Makefile.dep)

diff --git a/Makefile.include b/Makefile.include
 # process dependencies
 include $(RIOTBASE)/Makefile.dep
+# deduce packages from USEMODULE
+USEPKG := $(filter $(ALL_PKGS), $(USEMODULE))

and fixing packages so pkg/*/Makefile.dep can be included even if the module is not required. Like done in the normal Makefile.dep anyway.

This goes in the direction of making packages self contained like #9003

API CHANGE

It requires all packages Makefile.dep to be protected by the ifneq pattern.
It also require to remove all USEPKG in applications (which should be asserted right now).

Implementation

The commits have been designed to be incremental and always be in a working state. But this was not tested.
I count on murdock to help finding if there are issues and fixing them in the right place.

TODO

Get maintainers feedback on the solution
Check if the updated documentation is enough ! ! !
Move all easy packages dependencies to pkg/*/Makefile.dep (this could be done in subsequent PRs also)
check if if really all packages dependencies can be safely moved or document them

Issues/PRs references

pkg: USEPKG vs USEMODULE #6668

cladmi · 2018-06-07T15:59:39Z

The incremental changes also allow to possibly stop on a previous commit if preferred, or split this PR.

cladmi · 2018-06-07T16:18:46Z

I also remember there are some packages pre-cleanup included that I can do in sub PRs.

cladmi · 2018-06-11T13:44:29Z

Makefile.include


+# Detect if an application or Makefile.dep defined USEPKG
+ifneq (,$(USEPKG))
+  ifeq ($(origin USEPKG), file)


I currently need this because USEPKG is exported in vars.inc.mk and it seems already set before executing make in murdock.

I could remove the export in vars.inc.mk maybe, but I still do not get why it has a value when passing through here for the moment (I did not investigate).

There is currently a usage in sys/shell/commands that should be updated to use USEMODULE. I will try to go early in that direction.

cladmi · 2018-06-11T13:51:43Z

I fixed the broken builds, remaining task is to move packages dependencies out of the main Makefile.dep.

cladmi · 2018-06-14T18:22:41Z

Still remaining dependencies not migrated to pkg/*/Makefile.dep
They may be done later too.

lwip%

ifneq (,$(filter ieee802154 nrfmin,$(USEMODULE)))
  ...
  ifneq (,$(filter lwip%, $(USEMODULE)))
    USEMODULE += lwip_sixlowpan
  endif
endif

nordic_softdevice_ble removing modules

ifneq (,$(filter nordic_softdevice_ble,$(USEMODULE)))
  # prevent application from being a router
  # TODO: maybe find a nicer solution in future build system update
  _ROUTER_MODULES = gnrc_ipv6_router% gnrc_rpl netstats_rpl auto_init_gnrc_rpl
  ifneq (,$(filter $(_ROUTER_MODULES),$(USEMODULE)))
    $(warning nordic_softdevice_ble: Disabling router modules:\
      $(filter $(_ROUTER_MODULES),$(USEMODULE)))
  endif
  USEMODULE := $(filter-out $(_ROUTER_MODULES),$(USEMODULE))
endif

openthread dependencies which are now all defined in the test Makefile

jnohlgard · 2018-06-15T06:38:48Z

There's a weird lwip failure in the Murdock logs. Maybe needs a rebase?

cladmi · 2018-06-15T09:33:40Z

I moved the lwip dependencies to pkg/lwip/Makefile.dep and re-order them so it could also be my fault.

cladmi · 2018-06-15T10:03:39Z

Looks still broken, I will investigate. I expected this as some of the dependencies definition are order specific.
If I do not manage easily, I will keep the problematic lwip ones in the main Makefile.dep

cladmi · 2018-06-18T13:01:14Z

I moved the "default implementation" selection at the end of pkg/lwip/Makefile.dep and it should be fixed with this.

cladmi · 2018-09-04T12:15:09Z

Will need to rebase due to #9884

smlng · 2018-09-25T12:13:20Z

@cladmi how is this coming? I think this is a nice cleanup, if you need help on testing: I'm in.

cladmi · 2018-09-25T12:41:49Z

Thanks for the update. It got somehow lost in my todo list, and was waiting for the "Get maintainers feedback on the solution" and "Check if the updated documentation is enough ! ! !" parts so did not put more work on it recently.

If you like this current solution, or tell me what I should change, I can start splitting parts.

INCLUDE is now set by the packages pkg/<pkg_name>/Makefile.include file.

There is no such thing as a required order to define this.

It is assumed they are in 'RIOTPKG/pkgname' and that there is a module named 'pkgname'.

Prepare for specifying dependencies to packages only by putting them in `USEMODULE`. USEPKG would be an internal only variable similar to DIRS.

Prepare for specifying dependencies to packages only by putting them in `USEMODULE`.

… as USEPKG to USEMODULE Prepare for secifying dependencies to packages only by putting them in `USEMODULE`.

Prepare for secifying dependencies to packages only by putting them in `USEMODULE`.

Replace declaring dependency of packages using USEPKG to using USEMODULE. Setting USEPKG to the list of used packages will be done by the build system. RIOT-OS#6668 Currently, if a package has sub modules, like pkg_name_sub_module it would still require to have in the main Makefile.dep a dependency from pkg_name_sub_module to pkg_name module.

Allow to safely include all pkg/*/Makefile.dep before knowing if they are used or not. This allows using "USEMODULE += pkg_name_submodule_name" in applications and define the 'pkg_name_submodule_name' dependency to 'pkg_name' in the package itself.

This allows packages to declare there sub modules dependencies directly in the package directory instead of the main Makefile.dep. It however requires all packages to protect Makefile.dep to only set dependencies if the package is used.

Move most of the packages dependencies to pkg/*/Makefile.dep. Still some weird cases are handled in Makefile.dep. I used this command to check for non migrated packages grep $(ls -d pkg/*/ | sed 's|^pkg/|-e |;s|/||') Makefile.dep

cladmi · 2019-08-12T12:40:41Z

I did a blind stupid rebase on master. May not be working right now.

From the first version I did, I would like to change the handling to do the dependencies first to allow deprecating incompatible things earlier without breaking anything.

I see some steps to go forward with this in parallel:

cleanups
Declare ALL_PKGS
Always add USEPKG to USEMODULE
- change the usages of $(USEPKG) to $(USEMODULE).
  - do not export USEPKG anymore
- Change the packages Makefile.dep to be always with ifneq(,$(filter packagename,$(USEMODULE)) and add a sanity check for it (this will allow deprecating it and not break external code immediately).
  - With this, the packages Makefile.dep could always be parsed without side-effect, like done for all the others Makefile.dep right now.

stale · 2020-04-10T12:25:25Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

cladmi force-pushed the pkg/usemodule/usepkg branch from f9a16e2 to 6ec0f95 Compare June 7, 2018 15:43

cladmi force-pushed the pkg/usemodule/usepkg branch 5 times, most recently from 505d17c to b1faef2 Compare June 11, 2018 13:28

cladmi commented Jun 11, 2018

View reviewed changes

cladmi force-pushed the pkg/usemodule/usepkg branch from d5de2fd to 3a55494 Compare June 14, 2018 18:24

cladmi force-pushed the pkg/usemodule/usepkg branch from 3a55494 to 15d7e18 Compare June 15, 2018 09:31

cladmi force-pushed the pkg/usemodule/usepkg branch from 15d7e18 to d58a167 Compare June 18, 2018 12:59

cladmi force-pushed the pkg/usemodule/usepkg branch 2 times, most recently from 9f66e5e to bf34391 Compare July 4, 2018 08:15

This was referenced Aug 17, 2018

make: Use ld -r for intermediate linking of each module #8711

Closed

Support for self contained modules #9811

Closed

aabadie modified the milestones: Release 2019.01, Release 2019.04 Jan 25, 2019

cladmi modified the milestones: Release 2019.04, Release 2019.07 Apr 23, 2019

cladmi modified the milestones: Release 2019.07, Release 2019.10 Jun 14, 2019

cladmi removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 25, 2019

cladmi mentioned this pull request Aug 12, 2019

pkg: USEPKG vs USEMODULE #6668

Open

cladmi added 16 commits August 12, 2019 12:38

pkg/doc.txt: remove outdated information on INCLUDE

b6dbd4a

INCLUDE is now set by the packages pkg/<pkg_name>/Makefile.include file.

pkg/relic: remove wrong information about variable definition

52ce72e

There is no such thing as a required order to define this.

pkg/fatfs: Cleanup to use RIOTPKG variable

45aed72

Makefile.include: list all packages directories

6931385

It is assumed they are in 'RIOTPKG/pkgname' and that there is a module named 'pkgname'.

Makefile.dep: deduce USEPKG from modules in USEMODULE

4dd1859

Prepare for specifying dependencies to packages only by putting them in `USEMODULE`. USEPKG would be an internal only variable similar to DIRS.

Makefile.dep: replace dependencies set as USEPKG to USEMODULE

241a62c

Prepare for specifying dependencies to packages only by putting them in `USEMODULE`.

boards/cpu: replace dependencies set as USEPKG to USEMODULE

665e7f6

Prepare for specifying dependencies to packages only by putting them in `USEMODULE`.

TODO: MAYBE UPDATE THE DOC.TXT SENTENCE pkg: replace dependencies set…

19b7f49

… as USEPKG to USEMODULE Prepare for secifying dependencies to packages only by putting them in `USEMODULE`.

examples: replace dependencies set as USEPKG to USEMODULE

e82f115

Prepare for secifying dependencies to packages only by putting them in `USEMODULE`.

tests: replace dependencies set as USEPKG to USEMODULE

8731ad8

Prepare for secifying dependencies to packages only by putting them in `USEMODULE`.

sys: replace dependencies checking as USEPKG to USEMODULE

760d089

Prepare for secifying dependencies to packages only by putting them in `USEMODULE`.

makefiles/info-global.inc.mk: remove mentions of USEPKG

dbda5d3

Makefile.dep: include all packages Makefile.dep

984a589

This allows packages to declare there sub modules dependencies directly in the package directory instead of the main Makefile.dep. It however requires all packages to protect Makefile.dep to only set dependencies if the package is used.

pkgs: move packages dependencies to pkg/*/Makefile.dep

73b9fc6

Move most of the packages dependencies to pkg/*/Makefile.dep. Still some weird cases are handled in Makefile.dep. I used this command to check for non migrated packages grep $(ls -d pkg/*/ | sed 's|^pkg/|-e |;s|/||') Makefile.dep

cladmi force-pushed the pkg/usemodule/usepkg branch from 1be18e9 to 73b9fc6 Compare August 12, 2019 12:20

kb2ma removed this from the Release 2019.10 milestone Oct 8, 2019

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Apr 10, 2020

stale bot closed this May 11, 2020

Conversation

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

Uh oh!

Contribution description

API CHANGE

Implementation

TODO

Issues/PRs references

Uh oh!

cladmi commented Jun 7, 2018

Uh oh!

cladmi commented Jun 7, 2018

Uh oh!

cladmi Jun 11, 2018

Choose a reason for hiding this comment

Uh oh!

cladmi Aug 12, 2019

Choose a reason for hiding this comment

Uh oh!

cladmi commented Jun 11, 2018

Uh oh!

cladmi commented Jun 14, 2018

Uh oh!

jnohlgard commented Jun 15, 2018

Uh oh!

cladmi commented Jun 15, 2018

Uh oh!

cladmi commented Jun 15, 2018

Uh oh!

cladmi commented Jun 18, 2018

Uh oh!

cladmi commented Sep 4, 2018

Uh oh!

smlng commented Sep 25, 2018

Uh oh!

cladmi commented Sep 25, 2018

Uh oh!

cladmi commented Aug 12, 2019

Uh oh!

stale bot commented Apr 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cladmi commented Jun 7, 2018 •

edited

Loading