buildsystem/pkg: expand packages from USEPKG early as path into PKG_PATHS

[NikLeberg]

Contribution description

This pr it a split from pr #17211 as requested by @fjmolinas.

It changes the way packages are handled. The updated rules now don't explicitly use RIOTPKG but instead the new PKG_PATHS variable. This new variable holds the path to the packages that were found in RIOTPKG and were requested by USEPKG. This makes the Makefile rules independent from the actual location of the package. The intention is (with another pr) to allow to seach in additional paths (external to riot) for packages.

Testing procedure

I've ran make static-test with no errors (don't know what exactly is tested though).
I've also compiled the hello-world example with make BOARD=esp8266-esp-12x or the suit_update example.
Regarding a specific test case, I don't think one is needed. As I only change how something is done and don't add anything new, existing tests should cover this pr. CI will tell us :)

Issues/PRs references

pr #17211

[fjmolinas]

Hmm, on a first run I do not get the same results as the CI (failing builds succeeding locally)

[fjmolinas]

Hmm, on a first run I do not get the same results as the CI (failing builds succeeding locally)

Ahh got it, its the Kconfig build that fails:

TEST_KCONFIG=1 RIOT_CI_BUILD=1 BOARD=arduino-due make -C tests/pkg_relic/ clean all -j
make: Nothing to be done for 'clean'.
Building application "tests_pkg_relic" for "arduino-due" with MCU "sam3".

/home/francisco/workspace/RIOT/tests/pkg_relic/main.c:16:10: fatal error: relic.h: No such file or directory
   16 | #include "relic.h"
      |          ^~~~~~~~~
compilation terminated.
make[1]: *** [/home/francisco/workspace/RIOT/Makefile.base:146: /home/francisco/workspace/RIOT/tests/pkg_relic/bin/arduino-due/application_tests_pkg_relic/main.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [/home/francisco/workspace/RIOT/tests/pkg_relic/../../Makefile.include:703: application_tests_pkg_relic.module] Error 2

[fjmolinas]

@NikLeberg Could you apply the following patch?:

diff --git a/Makefile.include b/Makefile.include
index 6c1369cd6d..74dd9b78a2 100644
--- a/Makefile.include
+++ b/Makefile.include
@@ -416,6 +416,10 @@ ifeq (1,$(TEST_KCONFIG))
   USEMODULE := $(KCONFIG_MODULES)
   KCONFIG_PACKAGES := $(call lowercase,$(patsubst CONFIG_PACKAGE_%,%,$(filter CONFIG_PACKAGE_%,$(.VARIABLES))))
   USEPKG := $(KCONFIG_PACKAGES)
+
+  # Locate used modules in $(RIOTPKG).
+  PKG_PATHS := $(sort $(foreach dir,$(RIOTPKG),\
+    $(foreach pkg,$(USEPKG),$(dir $(wildcard $(dir)/$(pkg)/Makefile)))))
 else
   # always select provided architecture features
   FEATURES_REQUIRED += $(filter arch_%,$(FEATURES_PROVIDED))

This should fix the builds, I will then run some test to check dependencies.

[fjmolinas]

Hmmm looking better but still some failures

[NikLeberg]

Hi @fjmolinas thanks for looking into this.
Yeah I see the failures too. They seem to all be with esp8266 boards. If I run locally I can reproduce and get following error:

[INFO] patch esp8266_sdk
Patch is empty.

The empty patch leaves git in an am session and further git commands won't work... Do you have a clue where that could come from?

[fjmolinas]

[INFO] patch esp8266_sdk
Patch is empty.

I don't think this should be an issue, and so 765f25b should not be needed. Can you be sure you are trying for a clean state?

rm -r RIOT/build

Regarding the murdock build issue, it seems to be some kind of make ordering issue, but couldn't figure it out after a short debug session, will try again as soon as posible.

[fjmolinas]

@NikLeberg can you remove 765f25b and ad this patch, it should fix the issue (hopefully).

diff --git a/pkg/esp8266_sdk/Makefile b/pkg/esp8266_sdk/Makefile
index 1453c63006..59cde6131d 100644
--- a/pkg/esp8266_sdk/Makefile
+++ b/pkg/esp8266_sdk/Makefile
@@ -20,7 +20,9 @@ ESP_SDK_LIBS = $(addprefix $(ESP8266_SDK_BUILD_DIR)/, $(ESP_SDK_COMPONENT_LIBS))

 all: $(ESP_SDK_LIBS) $(ESP8266_SDK_BUILD_DIR)/esp8266_idf_version.h

-$(ESP8266_SDK_BUILD_DIR):
+$(PKG_PREPARED): $(ESP8266_SDK_BUILD_DIR)
+
+$(ESP8266_SDK_BUILD_DIR): $(PKG_PATCHED)
        $(Q)mkdir -p $(ESP8266_SDK_BUILD_DIR)

 # Set the SDK version from the SDK hash/tag. For example "v3.1-51-g913a06a9".

[NikLeberg]

Thanks for fixing that makefile order issue @fjmolinas.
As now the build for the esp32 fails, I suspect that it also does things similar and needs a rule update with $(PKG_PREPARED) and $(PKG_PATCHED). I wanted to apply your patch to the esp32 packages.. but I don't find them?
Is Murdock running the tests from within the pr, or is it merging the pr to the latest master and then running the tests? In the second case, I probably need to rebase, do I?

[fjmolinas]

Is Murdock running the tests from within the pr, or is it merging the pr to the latest master and then running the tests? In the second case, I probably need to rebase, do I?

It's doing the second thing! If you want to rebase feel free to do so

[fjmolinas]

Is Murdock running the tests from within the pr, or is it merging the pr to the latest master and then running the tests? In the second case, I probably need to rebase, do I?

It's doing the second thing! If you want to rebase feel free to do so

Here is a patch that works for me locally, can you apply it? ( I didn't have
c3f4846 in my tree so you might have a conflict).

diff --git a/pkg/esp32_sdk/Makefile b/pkg/esp32_sdk/Makefile
index 0d0c813b5d..1538c7d02c 100644
--- a/pkg/esp32_sdk/Makefile
+++ b/pkg/esp32_sdk/Makefile
@@ -20,11 +20,13 @@ ESP32_SDK_LIBS = $(addprefix $(ESP32_SDK_BUILD_DIR)/, $(ESP32_SDK_COMPONENT_LIBS
 
 all: $(ESP32_SDK_LIBS) $(ESP32_SDK_VER_FILE)
 
-$(ESP32_SDK_BUILD_DIR):
+$(PKG_PREPARED): $(ESP32_SDK_BUILD_DIR) $(ESP32_SDK_VER_FILE)
+
+$(ESP32_SDK_BUILD_DIR): $(PKG_PATCHED)
        $(Q)mkdir -p $(ESP32_SDK_BUILD_DIR)
 
 # Set the SDK version from the SDK hash/tag. For example "v3.1-51-g913a06a9".
-$(ESP32_SDK_VER_FILE):
+$(ESP32_SDK_VER_FILE): $(PKG_PATCHED) | $(ESP32_SDK_BUILD_DIR)
        $(Q)echo "#define IDF_VER \"$(ESP32_SDK_VER_CMD)\"" > $@
 
 $(ESP32_SDK_BUILD_DIR)/lib%.a: \

If the ci is green after this, could you rebase keeping individual commits for those esp32 and esp86 patches, a descriptive commit would be much appreciated, something like:

pkg/esp32_sdk: add requirements to PKG_PREPARE

The share `build-dir` directory needs to be available for modules/packages that depend on the SDK before that package is eventually compiled.

This also includes header files such as `esp32_idf_version.h`.

Packages are downloaded, patches, prepared before any module is compiled. By adding the directory creation and header as a dependency of `PKG_PREPARE` we make sure they are included before compilation starts.

In c3f4846 you also patches esp32_sdk_libs, I think that also makes sense, but if possible add an individual commit for it :).

Thanks for sticking through!

[fjmolinas]

@NikLeberg all green! Please squash!

[NikLeberg]

@fjmolinas like so? Or should I squash all of them again into one commit?

[fjmolinas]

@fjmolinas like so? Or should I squash all of them again into one commit?

No that's perfect!

[fjmolinas]

ACK, I added run-tests to be safe.

[fjmolinas]

Non relevant tests failed, will re-run without the flag

[NikLeberg]

Uh oh.. I clicked the GitHub button "fetch upstram" on the wrong branch... Sorry. :( Will undo the auto-merge and reapply the commits.

[fjmolinas]

Rebased, and fixed authorship that was distorted in my first rebase.

[fjmolinas]

Thanks for this one @NikLeberg!