Skip to content

tooling: Auto-init submodules in make firmware if missing #361

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.

Sign up for GitHub

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

Merged
nedseb merged 3 commits into from
Apr 9, 2026
Merged

tooling: Auto-init submodules in make firmware if missing #361

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
778a7d0
tooling: Auto-init submodules in make firmware if missing.
nedseb Apr 9, 2026
e9fa5ec
fix: Add set -e to firmware targets to fail-fast on errors.
nedseb Apr 9, 2026
3d12432
style: Silence set -e with @ prefix in firmware targets.
nedseb Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ $(MPY_DIR):

.PHONY: firmware
firmware: $(MPY_DIR) ## Build MicroPython firmware with current drivers
@set -e
@if [ ! -f "$(MPY_DIR)/lib/micropython-lib/README.md" ]; then \
echo "Initializing submodules for $(BOARD)..."; \
cd $(CURDIR)/$(MPY_DIR)/ports/stm32 && $(MAKE) BOARD=$(BOARD) submodules; \
fi
Comment on lines +102 to +105
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

Because this Makefile sets .ONESHELL, cd $(CURDIR)/$(MPY_DIR)/ports/stm32 && $(MAKE) ... will change the working directory for the remainder of the firmware recipe, which can cause surprising behavior if any later commands start using relative paths. To avoid shell state leakage, consider using $(MAKE) -C $(CURDIR)/$(MPY_DIR)/ports/stm32 BOARD=$(BOARD) submodules (and similarly for other cd ... && $(MAKE) calls), and use $(CURDIR)/$(MPY_DIR)/... in the -f check for consistency with the rest of this target.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@nedseb nedseb Apr 9, 2026

Choose a reason for hiding this comment

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

Valid point about cd leaking in .ONESHELL. However, cd ... && $(MAKE) is the pattern used throughout the Makefile (lines 97, 109, 118, 124). Changing only this occurrence would be inconsistent. The set -e added in this PR mitigates the risk: if the cd or $(MAKE) fails, the recipe stops immediately. A global refactor to $(MAKE) -C would be a good follow-up but is out of scope here.

@echo "Linking local drivers..."
rm -rf $(CURDIR)/$(MPY_DIR)/lib/micropython-steami-lib
ln -s $(CURDIR) $(CURDIR)/$(MPY_DIR)/lib/micropython-steami-lib
Expand All @@ -107,6 +112,7 @@ firmware: $(MPY_DIR) ## Build MicroPython firmware with current drivers

.PHONY: firmware-update
firmware-update: $(MPY_DIR) ## Update the MicroPython clone and board-specific submodules
@set -e
@echo "Updating micropython-steami..."
rm -rf $(CURDIR)/$(MPY_DIR)/lib/micropython-steami-lib
cd $(CURDIR)/$(MPY_DIR) && git fetch origin && git checkout $(MICROPYTHON_BRANCH) && git checkout -- lib/micropython-steami-lib && git pull --ff-only
Expand Down
Loading