Skip to content

examples/embedding: Build embed sources in the same make command. #16948

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AJMansfield
Copy link
Contributor

@AJMansfield AJMansfield commented Mar 17, 2025

Summary

This updates the example for the embed port to streamline the build process, reducing the number of commands needed to build it from 2 to 1.

In specific:

  • The original Makefile is moved aside to become makefile.mk.
  • It's replaced with a new Makefile that invokes micropython_embed.mk and makefile.mk as recursive sub-makes.

This indirection makes sure that the SRC += $(wildcard $(EMBED_DIR)/*/*.c) $(wildcard $(EMBED_DIR)/*/*/*.c) variable expansion is performed after the sources it's meant to pick up already exist. If the mpy recipe were added directly to the original makefile, these files wouldn't be picked up on first invocation and it would prevent the example from being built directly from clean.

Testing

This commit updates .github/workflows/examples.yml to reflect the Makefile -> makefile.mk rename, and adds tests of the combined single-command clean and build operations. I've also separately tested building the example from a number of different clean and dirty build states.

Trade-offs and Alternatives

This is a documentation change (with automated tests that verify the documentation's claims).
The minor increase in technical complexity could subtly impact the example's pedagogical utility, but the positive utility of demonstrating a complete combined source-gen and compilation build should outweigh that.

Copy link

codecov bot commented Mar 17, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.53%. Comparing base (f1018ee) to head (9b0524f).
Report is 35 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #16948      +/-   ##
==========================================
- Coverage   98.54%   98.53%   -0.01%     
==========================================
  Files         169      169              
  Lines       21877    21877              
==========================================
- Hits        21558    21556       -2     
- Misses        319      321       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Anson Mansfield <amansfield@mantaro.com>
Copy link

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
     mimxrt:    +0 +0.000% TEENSY40
        rp2:    +0 +0.000% RPI_PICO_W
       samd:    +0 +0.000% ADAFRUIT_ITSYBITSY_M4_EXPRESS
  qemu rv32:    +0 +0.000% VIRT_RV32

@AJMansfield
Copy link
Contributor Author

Coverage change seems to be spurious?

@dpgeorge dpgeorge added the examples Relates to examples/ directory in source label Mar 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
examples Relates to examples/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants