boards/mips: remove use of APPDEPS, un-export globally APPDEPS #14385

aabadie · 2020-06-29T08:36:28Z

Contribution description

This PR remove the use of APPDEPS to build the pic32 boards. The module generated from the $(CPU_MODEL).S files can be added to the build based on the regulars USEMODULE + DIRS variables.

There's also no need to export APPDEPS, since it's only used in the global Makefile.include:

RIOT/Makefile.include

Line 445 in ef4835a

BASELIBS += $(APPDEPS)

RIOT/Makefile.include

Line 549 in ef4835a

BUILDDEPS += $(APPDEPS)

RIOT/Makefile.include

Line 580 in ef4835a

    
           _SUBMAKE_LIBS = $(filter-out $(BINDIR)/$(APPLICATION_MODULE).a $(APPDEPS), $(BASELIBS))

As a result, it seems that APPDEPS doesn't need to be exported globally and this PR changes this as well (with a static check)

Testing procedure

A green Murdock

Issues/PRs references

None

…ency

fjmolinas · 2020-06-30T12:43:36Z

dist/tools/buildsystem_sanity_check/check.sh

@@ -109,9 +109,9 @@ UNEXPORTED_VARIABLES+=('JLINK_DEVICE' 'JLINK_IF')
 UNEXPORTED_VARIABLES+=('JLINK_PRE_FLASH' 'JLINK_RESET_FILE')
 UNEXPORTED_VARIABLES+=('GIT_CACHE' 'GIT_CACHE_DIR')
 UNEXPORTED_VARIABLES+=('LINKXX')
+UNEXPORTED_VARIABLES+=('APPDEPS' 'BUILDDEPS' 'DEBUGDEPS')


'BUILDDEPS' 'DEBUGDEPS' seem unrelated?

Yes but they are also not exported. I added them here as a guard, just in case. I can add them in a separate commit if you prefer?

Yes, I would prefer a different commit

fjmolinas · 2020-06-30T12:52:40Z

I wanted to test this by checking the compiled hex but I seem to fail to compile for me, any ideas?

"make" -C /data/riotbuild/riotbase/sys/tsrb
"make" -C /data/riotbuild/riotbase/sys/xtimer
objcopy: Unable to recognise the format of the input file `/data/riotbuild/riotbase/examples/asymcute_mqttsn/bin/6lowpan-clicker/asymcute_mqttsn.elf'
objcopy: --change-section-lma .startdata+0xffffffff80000000 never used
objcopy: --change-section-lma .bss+0xffffffff80000000 never used
objcopy: --change-section-lma .data+0xffffffff80000000 never used
objcopy: --change-section-lma .rodata+0xffffffff80000000 never used
objcopy: --change-section-lma .dtors+0xffffffff80000000 never used
objcopy: --change-section-lma .ctors+0xffffffff80000000 never used
objcopy: --change-section-lma .jcr+0xffffffff80000000 never used
objcopy: --change-section-lma .eh_frame+0xffffffff80000000 never used
objcopy: --change-section-lma .fini+0xffffffff80000000 never used
objcopy: --change-section-lma .init+0xffffffff80000000 never used
objcopy: --change-section-lma .text+0xffffffff80000000 never used
objcopy: --change-section-lma .exception_vector+0xffffffff80000000 never used
objcopy: --change-section-lma .bootflash+0xffffffff60000000 never used
/data/riotbuild/riotbase/Makefile.include:591: recipe for target '/data/riotbuild/riotbase/examples/asymcute_mqttsn/bin/6lowpan-clicker/asymcute_mqttsn.bin' failed
make: *** [/data/riotbuild/riotbase/examples/asymcute_mqttsn/bin/6lowpan-clicker/asymcute_mqttsn.bin] Error 1
/home/francisco/workspace/RIOT/makefiles/docker.inc.mk:304: recipe for target '..in-docker-container' failed
make: *** [..in-docker-container] Error 2
make: Leaving directory '/home/francisco/workspace/RIOT/examples/asymcute_mqttsn'

aabadie · 2020-06-30T17:14:46Z

I wanted to test this by checking the compiled hex but I seem to fail to compile for me, any ideas?

I have the same issue but it's the same on master.

aabadie · 2020-06-30T17:23:06Z

@fjmolinas, I just noticed that it builds just fine when setting RIOT_CI_BUILD=1 (with this PR and in master).

this PR:

RIOT_VERSION=test RIOT_CI_BUILD=1 make BOARD=pic32-wifire -C examples/hello-world --no-print-directory 
Building application "hello-world" for "pic32-wifire" with MCU "mips_pic32mz".

   text	   data	    bss	    dec	    hex	filename
 109280	   2780	   7000	 119060	  1d114	/work/riot/RIOT/examples/hello-world/bin/pic32-wifire/hello-world.elf

master:

RIOT_VERSION=test RIOT_CI_BUILD=1 make BOARD=pic32-wifire -C examples/hello-world --no-print-directory 
Building application "hello-world" for "pic32-wifire" with MCU "mips_pic32mz".

   text	   data	    bss	    dec	    hex	filename
 109280	   2780	   7000	 119060	  1d114	/work/riot/RIOT/examples/hello-world/bin/pic32-wifire/hello-world.elf

aabadie · 2020-06-30T19:25:15Z

Using git bisect run make BOARD=pic32-wifire -C examples/hello-world between master and 2020.04, I found that the issue was introduced by 47a5805 in #14159.

With the following diff:

git diff
diff --git a/Makefile.include b/Makefile.include
index 8056a026f3..271b9f847f 100644
--- a/Makefile.include
+++ b/Makefile.include
@@ -532,7 +532,7 @@ ifeq ($(RIOT_CI_BUILD),1)
   # Don't build BINFILE on the CI to save some computation time
   BUILD_FILES += $(ELFFILE) $(FLASHFILE)
 else
-  BUILD_FILES += $(ELFFILE) $(BINFILE) $(FLASHFILE)
+  BUILD_FILES += $(ELFFILE) $(FLASHFILE)
 endif
 
 # variables used to compile and link c++

The build works fine on master and this PR:

RIOT_VERSION=test make BOARD=pic32-wifire -C examples/hello-world/ --no-print-directory 
Building application "hello-world" for "pic32-wifire" with MCU "mips_pic32mz".

"make" -C /work/riot/RIOT/boards/pic32-wifire
"make" -C /work/riot/RIOT/core
"make" -C /work/riot/RIOT/cpu/mips_pic32mz
"make" -C /work/riot/RIOT/cpu/mips32r2_common
"make" -C /work/riot/RIOT/cpu/mips32r2_common/newlib_syscalls_mips_uhi
"make" -C /work/riot/RIOT/cpu/mips32r2_common/periph
"make" -C /work/riot/RIOT/cpu/mips_pic32_common
"make" -C /work/riot/RIOT/cpu/mips_pic32_common/periph
"make" -C /work/riot/RIOT/cpu/mips_pic32mz/p32mz2048efg100
"make" -C /work/riot/RIOT/drivers
"make" -C /work/riot/RIOT/drivers/periph_common
"make" -C /work/riot/RIOT/sys
"make" -C /work/riot/RIOT/sys/auto_init
"make" -C /work/riot/RIOT/sys/stdio_uart
   text	   data	    bss	    dec	    hex	filename
 109280	   2780	   7000	 119060	  1d114	/work/riot/RIOT/examples/hello-world/bin/pic32-wifire/hello-world.elf

That also explains why it works when setting RIOT_CI_BUILD=1 to the build: the bin file is not generated in this case.

fjmolinas · 2020-06-30T19:29:05Z

Using git bisect run make BOARD=pic32-wifire -C examples/hello-world between master and 2020.04, I found that the issue was introduced by 47a5805 in #14159.

Wow, nice catch, would not have expected that. But why does the BINFILE cause the issue?

aabadie · 2020-06-30T19:32:18Z

But why does the BINFILE cause the issue?

My analysis is that the objcopy used by mips is the default host objcopy, not the toolchain one (because it's broken). And the host objcopy is not able to output a binary for mips architecture:

RIOT/cpu/mips_pic32mz/Makefile.include

Line 14 in e1d27cb

OBJCOPY = objcopy #use system objcopy as toolchain one is broken.

aabadie · 2020-06-30T19:33:05Z

I think we should just skip BINFILE generation for MIPS until we have a fully working toolchain for it.

aabadie · 2020-07-01T06:09:02Z

see #14405 @fjmolinas

fjmolinas · 2020-07-01T07:13:16Z

Did as in #14385 (comment) forcing the HEXFILE to be built, compared both, no diff.

fjmolinas

ACK!

aabadie added 4 commits June 28, 2020 12:13

cpu/mips_pic32_common: add cpu_model specific assembly symbols depend…

ffa28e4

…ency

boards/pic32: remove useless use of APPDEPS

b640ba2

makefiles/vars.inc.mk: don't export APPDEPS globally

df51ff7

ci/build_system_check: ensure no export of APPDEPS/BUILDDEPS/DEBUGDEPS

88452ab

aabadie 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 Jun 29, 2020

aabadie requested a review from kaspar030 as a code owner June 29, 2020 08:36

aabadie requested a review from fjmolinas June 29, 2020 08:36

fjmolinas reviewed Jun 30, 2020

View reviewed changes

aabadie mentioned this pull request Jul 1, 2020

cpu/mips: disable BINFILE generation #14405

Merged

fjmolinas approved these changes Jul 1, 2020

View reviewed changes

fjmolinas merged commit 4188867 into RIOT-OS:master Jul 1, 2020

aabadie mentioned this pull request Jul 1, 2020

MIPS: toolchain objcopy doesn't work and no .bin can be generated #14410

Closed

aabadie deleted the pr/make/appdeps_export branch July 1, 2020 12:35

miri64 added this to the Release 2020.07 milestone Jul 2, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

boards/mips: remove use of APPDEPS, un-export globally APPDEPS #14385

boards/mips: remove use of APPDEPS, un-export globally APPDEPS #14385

aabadie commented Jun 29, 2020

fjmolinas Jun 30, 2020

aabadie Jun 30, 2020

fjmolinas Jul 1, 2020

fjmolinas commented Jun 30, 2020

aabadie commented Jun 30, 2020

aabadie commented Jun 30, 2020

aabadie commented Jun 30, 2020 •

edited

Loading

fjmolinas commented Jun 30, 2020

aabadie commented Jun 30, 2020 •

edited

Loading

aabadie commented Jun 30, 2020 •

edited

Loading

aabadie commented Jul 1, 2020

fjmolinas commented Jul 1, 2020

fjmolinas left a comment

boards/mips: remove use of APPDEPS, un-export globally APPDEPS #14385

boards/mips: remove use of APPDEPS, un-export globally APPDEPS #14385

Conversation

aabadie commented Jun 29, 2020

Contribution description

Testing procedure

Issues/PRs references

fjmolinas Jun 30, 2020

Choose a reason for hiding this comment

aabadie Jun 30, 2020

Choose a reason for hiding this comment

fjmolinas Jul 1, 2020

Choose a reason for hiding this comment

fjmolinas commented Jun 30, 2020

aabadie commented Jun 30, 2020

aabadie commented Jun 30, 2020

aabadie commented Jun 30, 2020 • edited Loading

fjmolinas commented Jun 30, 2020

aabadie commented Jun 30, 2020 • edited Loading

aabadie commented Jun 30, 2020 • edited Loading

aabadie commented Jul 1, 2020

fjmolinas commented Jul 1, 2020

fjmolinas left a comment

Choose a reason for hiding this comment

aabadie commented Jun 30, 2020 •

edited

Loading

aabadie commented Jun 30, 2020 •

edited

Loading

aabadie commented Jun 30, 2020 •

edited

Loading