cpu/esp32: fixes LINKFLAGS in Makefile.include

[gschorcht]

Contribution description

This PR adds the ESP32 vendor libraries for WLAN to the BASELIBS variable. This avoids having to define in the variable LINKFLAGS an additional archive group containing these vendor libraries and again the RIOT module archive files with symbols referenced by these vendor libraries.

Testing procedure

Testing is done just by compiling an application that is using networkting

USEMODULE="spiffs esp_now" make BOARD=esp32-wroom-32 -C examples/gnrc_networking

The compilation has to be successful.

[cladmi]

I cannot really review the test command but I tested executing the compile command with docker.

BUILD_IN_DOCKER=1 DOCKER="sudo docker" USEMODULE="spiffs esp_now" make BOARD=esp32-wroom-32 -C examples/gnrc_networking

Compiling works both with this PR:

   text    data     bss     dec     hex filename
 132181    4960   23696  160837   27445 /data/riotbuild/riotproject/examples/gnrc_networking/bin/esp32-wroom-32/gnrc_networking.elf

and master:

   text    data     bss     dec     hex filename
 132137    4960   23696  160793   27419 /data/riotbuild/riotproject/examples/gnrc_networking/bin/esp32-wroom-32/gnrc_networking.elf

The size increased a little bit a show in the output, but I get the same output for the symbols in master and this PR using the following debug command (the one used to debug the fix linking PR):

objdump -t bin/esp32-wroom-32/*.elf | grep -v ' df ' | sort -k6 | cut -f1 -d" " --complement

@gschorcht Did you tried running the firmware and check if the behavior was still ok ?

[gschorcht]

@cladmi Yes, of course 😄

[cladmi]

By taking into account this comment explaining why it was there (#10362 (comment)) I agree with the change.

I only have a last curiosity question to try understanding, do you know why

LINKFLAGS += -lhal -lg -lc -lg

does not need to be put in BASELIBS ?

https://github.com/RIOT-OS/RIOT/pull/10460/files#diff-a264f1e32b7ec79a7db68441a4a75e90R127

[gschorcht]

@cladmi Double checked it

LINKFLAGS += -lhal -lg -lc

has not to be in the group. Archive groups have to be defined to be able to resolve circular references. The linker goes through all archives of a group again and again until it can resolve the symbols. So I think, as long as all symbols are defined in the -l... list in the order from right to left, you don't need to put them in a group.

BTW, I realized that

LINKFLAGS += -lhal -lg -lc -lg

contained libg.a two times 😟 I fixed it.

[cladmi]

@gschorcht thanks for the update, my goal is to have a reason in case someone ends up here in the future. When I tested in #10362 (comment) it also did not compile when putting them in BASELIBS and I did not debug further.

You currently have two commits with the same commit message, which is disturbing for me, and the second has no description.
Please update the second commit message to be describe the change.

I will approve after this.

[gschorcht]

@cladmi The second just removes the double entry of -lg. Should I squash them to have only one commit with the detailed description from the first one?

[cladmi]

@gschorcht I would prefer a second commit with its own description.
Also I just noticed that the commit (and PR title) are saying Makfile.include instead of Makefile.include.
You can rewrite them both directly.

[gschorcht]

@cladmi Done

[cladmi]

@gschorcht Thank you, I re-triggered murdock due to an unrelated failure.

[cladmi]

ACK, agree with the changes and justifications. I only tested compiling but as the maintainer of the board tested it is good for me :)

You can merge when murdock is green.

[cladmi]

Thank you for doing the fix and following the review :)