Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg/driver_cryptocell_310: Fix Makefile #19959

Merged
merged 2 commits into from
Sep 29, 2023

Conversation

mguetschow
Copy link
Contributor

Contribution description

  • make sure to download/extract during prepare, instead of build
  • this fixes the issue of missing include dependencies for other pkgs at build time

Testing procedure

  • for an app Makefile, include both c25519 and driver_cryptocell_310
  • on master, make all fails with a missing include path from the driver_cryptocell_310 package
  • with this change, it works

Issues/PRs references

@github-actions github-actions bot added the Area: pkg Area: External package ports label Sep 29, 2023
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor nit, otherwise ready to merge IMHO

pkg/c25519/Makefile Show resolved Hide resolved
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 29, 2023
make sure to download/extract during prepare, instead of build

this fixes the issue of missing include dependencies for other pkgs at build time
@mguetschow mguetschow force-pushed the cryptocell-makefile-fix branch from 1ef4831 to 8404ad5 Compare September 29, 2023 13:09
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK.

On current master

$ BOARD=nrf52840dk USEPKG="c25519 driver_cryptocell_310" make -C examples/hello-world/ -j
make: Entering directory '/home/mlenders/Repositories/RIOT-OS/RIOT/examples/hello-world'
Building application "hello-world" for "nrf52840dk" with MCU "nrf52".

make[1]: Nothing to be done for 'prepare'.
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/pkg/c25519/ 
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/build/pkg/c25519/src -f /home/mlenders/Repositories/RIOT-OS/RIOT/Makefile.base MODULE=c25519
cc1: error: /home/mlenders/Repositories/RIOT-OS/RIOT/build/pkg/driver_cryptocell_310/include: No such file or directory [-Werror=missing-include-dirs]
cc1: all warnings being treated as errors
make[2]: *** [/home/mlenders/Repositories/RIOT-OS/RIOT/Makefile.base:146: /home/mlenders/Repositories/RIOT-OS/RIOT/examples/hello-world/bin/nrf52840dk/c25519/morph25519.o] Error 1
make[2]: *** Waiting for unfinished jobs....
cc1: error: /home/mlenders/Repositories/RIOT-OS/RIOT/build/pkg/driver_cryptocell_310/include: No such file or directory [-Werror=missing-include-dirs]
cc1: all warnings being treated as errors
make[2]: *** [/home/mlenders/Repositories/RIOT-OS/RIOT/Makefile.base:146: /home/mlenders/Repositories/RIOT-OS/RIOT/examples/hello-world/bin/nrf52840dk/c25519/c25519.o] Error 1
cc1: error: /home/mlenders/Repositories/RIOT-OS/RIOT/build/pkg/driver_cryptocell_310/include: No such file or directory [-Werror=missing-include-dirs]
cc1: all warnings being treated as errors
make[2]: *** [/home/mlenders/Repositories/RIOT-OS/RIOT/Makefile.base:146: /home/mlenders/Repositories/RIOT-OS/RIOT/examples/hello-world/bin/nrf52840dk/c25519/edsign.o] Error 1
cc1: error: /home/mlenders/Repositories/RIOT-OS/RIOT/build/pkg/driver_cryptocell_310/include: No such file or directory [-Werror=missing-include-dirs]
cc1: all warnings being treated as errors
make[2]: *** [/home/mlenders/Repositories/RIOT-OS/RIOT/Makefile.base:146: /home/mlenders/Repositories/RIOT-OS/RIOT/examples/hello-world/bin/nrf52840dk/c25519/ed25519.o] Error 1
cc1: error: /home/mlenders/Repositories/RIOT-OS/RIOT/build/pkg/driver_cryptocell_310/include: No such file or directory [-Werror=missing-include-dirs]
cc1: all warnings being treated as errors
make[2]: *** [/home/mlenders/Repositories/RIOT-OS/RIOT/Makefile.base:146: /home/mlenders/Repositories/RIOT-OS/RIOT/examples/hello-world/bin/nrf52840dk/c25519/sha512.o] Error 1
cc1: error: /home/mlenders/Repositories/RIOT-OS/RIOT/build/pkg/driver_cryptocell_310/include: No such file or directory [-Werror=missing-include-dirs]
cc1: all warnings being treated as errors
make[2]: *** [/home/mlenders/Repositories/RIOT-OS/RIOT/Makefile.base:146: /home/mlenders/Repositories/RIOT-OS/RIOT/examples/hello-world/bin/nrf52840dk/c25519/fprime.o] Error 1
cc1: error: /home/mlenders/Repositories/RIOT-OS/RIOT/build/pkg/driver_cryptocell_310/include: No such file or directory [-Werror=missing-include-dirs]
cc1: all warnings being treated as errors
make[2]: *** [/home/mlenders/Repositories/RIOT-OS/RIOT/Makefile.base:146: /home/mlenders/Repositories/RIOT-OS/RIOT/examples/hello-world/bin/nrf52840dk/c25519/f25519.o] Error 1
make[1]: *** [Makefile:20: all] Error 2
make: *** [/home/mlenders/Repositories/RIOT-OS/RIOT/examples/hello-world/../../Makefile.include:827: pkg-build] Error 2
make: Leaving directory '/home/mlenders/Repositories/RIOT-OS/RIOT/examples/hello-world'

With this PR:

BOARD=nrf52840dk USEPKG="c25519 driver_cryptocell_310" make -C examples/hello-world/ -j
make: Entering directory '/home/mlenders/Repositories/RIOT-OS/RIOT/examples/hello-world'
Building application "hello-world" for "nrf52840dk" with MCU "nrf52".

make[1]: Nothing to be done for 'prepare'.
mkdir -p /home/mlenders/Repositories/RIOT-OS/RIOT/build/pkg
failed: No route to host.
failed: No route to host.
2023-09-29 15:43:41 URL:https://nsscprodmedia.blob.core.windows.net/prod/software-and-other-downloads/sdks/nrf5/binaries/nrf5_sdk_17.1.0_ddde560.zip [131838843/131838843] -> "/home/mlenders/Repositories/RIOT-OS/RIOT/build/pkg/nRF5_SDK_17.1.0_ddde560.zip" [1]
test "cb610d724d27ef79c792511d8b889cbf2d411a3a391f5c740ea10b86139201b97eedf0c1edcf4d199c3fbd51e4cc5a4bd922c0c40d01e2f5359acd16a2b163e4  /home/mlenders/Repositories/RIOT-OS/RIOT/build/pkg/nRF5_SDK_17.1.0_ddde560.zip" =  "$(sha512sum "/home/mlenders/Repositories/RIOT-OS/RIOT/build/pkg/nRF5_SDK_17.1.0_ddde560.zip")"
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/pkg/c25519/ 
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/build/pkg/c25519/src -f /home/mlenders/Repositories/RIOT-OS/RIOT/Makefile.base MODULE=c25519
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/pkg/cmsis/ 
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/pkg/driver_cryptocell_310/ 
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/boards/common/init
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/boards/nrf52840dk
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/core
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/core/lib
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/cpu/nrf52
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/drivers
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/pkg/driver_cryptocell_310/contrib
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/sys
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/boards/common/nrf52xxxdk
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/sys/auto_init
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/sys/div
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/cpu/cortexm_common
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/sys/libc
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/drivers/periph_common
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/cpu/nrf52/periph
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/sys/malloc_thread_safe
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/sys/newlib_syscalls_default
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/cpu/nrf52/vectors
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/cpu/nrf5x_common
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/sys/preprocessor
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/sys/stdio_uart
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/cpu/nrf5x_common/periph
"make" -C /home/mlenders/Repositories/RIOT-OS/RIOT/cpu/cortexm_common/periph
   text	   data	    bss	    dec	    hex	filename
  15488	    100	   9032	  24620	   602c	/home/mlenders/Repositories/RIOT-OS/RIOT/examples/hello-world/bin/nrf52840dk/hello-world.elf
make: Leaving directory '/home/mlenders/Repositories/RIOT-OS/RIOT/examples/hello-world'

@miri64 miri64 added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: build system Area: Build system labels Sep 29, 2023
Copy link
Contributor

@Einhornhool Einhornhool left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine, works for me

@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 29, 2023
@riot-ci
Copy link

riot-ci commented Sep 29, 2023

Murdock results

✔️ PASSED

8404ad5 pkg/c25519: unify Makefile variables

Success Failures Total Runtime
7937 0 7937 14m:43s

Artifacts

@miri64
Copy link
Member

miri64 commented Sep 29, 2023

bors merge

bors bot added a commit that referenced this pull request Sep 29, 2023
19959: pkg/driver_cryptocell_310: Fix Makefile r=miri64 a=mguetschow


### Contribution description

- make sure to download/extract during prepare, instead of build
- this fixes the issue of missing include dependencies for other pkgs at build time

### Testing procedure

- for an app Makefile, include both `c25519` and `driver_cryptocell_310`
- on `master`, `make all` fails with a missing include path from the `driver_cryptocell_310` package
- with this change, it works


### Issues/PRs references

- isolated from #19954 as suggested by `@miri64` 

Co-authored-by: Mikolai Gütschow <mikolai.guetschow@tu-dresden.de>
@benpicco
Copy link
Contributor

bors cancel
bors merge

@bors
Copy link
Contributor

bors bot commented Sep 29, 2023

Canceled.

bors bot added a commit that referenced this pull request Sep 29, 2023
19959: pkg/driver_cryptocell_310: Fix Makefile r=benpicco a=mguetschow


### Contribution description

- make sure to download/extract during prepare, instead of build
- this fixes the issue of missing include dependencies for other pkgs at build time

### Testing procedure

- for an app Makefile, include both `c25519` and `driver_cryptocell_310`
- on `master`, `make all` fails with a missing include path from the `driver_cryptocell_310` package
- with this change, it works


### Issues/PRs references

- isolated from #19954 as suggested by `@miri64` 

19960: dist/tools/jlink: fix DBG_PID assignment r=benpicco a=LP-HAW



19961: ztimer_periodic: fix example in documentation r=benpicco a=benpicco



Co-authored-by: Mikolai Gütschow <mikolai.guetschow@tu-dresden.de>
Co-authored-by: LP-HAW <lars.pfau@haw-hamburg.de>
Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
@bors
Copy link
Contributor

bors bot commented Sep 29, 2023

Build failed (retrying...):

@bors
Copy link
Contributor

bors bot commented Sep 29, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 328dd59 into RIOT-OS:master Sep 29, 2023
@mguetschow mguetschow deleted the cryptocell-makefile-fix branch October 30, 2023 10:27
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.10 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: build system Area: Build system Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants