modm-io / modm

modm: a C++23 library generator for AVR and ARM Cortex-M devices
https://modm.io
Mozilla Public License 2.0
748 stars 132 forks source link

Add option to specify which repositories to generate SConscript files for #1192

Closed calebchalmers closed 3 months ago

calebchalmers commented 3 months ago

Currently, the modm:scons:build module will generate an SConscript for itself and every other repository included in the build process. This causes issues if another repository has its own method of generating an SConscript, since modm will overwrite it.

This PR changes the behaviour to only generate SConscript files for modm and any additional repos specified in modm:scons:build:include_repos (comma-separated list of repository names).

Note: this is a minor breaking change since no additional repositories will be included by default, whereas previously they would have been.

calebchalmers commented 3 months ago

Tests are failing due to:

Traceback (most recent call last):
  File "/__w/modm/modm/repo.lb", line 55, in <module>
    import modm_devices.parser
ModuleNotFoundError: No module named 'modm_devices'

Any ideas what the issue is here?

salkinium commented 3 months ago

Good idea, but I don't like this implementation as it's not backward compatible and the option is not user friendly (although it could be defaulted I guess).

I'd rather you check the buildlog if any of the repos already generated a SConscript file in their output path, and then not generate ours (but still call it of course).

calebchalmers commented 3 months ago

I'd rather you check the buildlog if any of the repos already generated a SConscript file in their output path, and then not generate ours (but still call it of course).

Yeah I was trying to think of the best way to go about this. I like your idea to check for existing files, but would we need to worry about build order?

What if another repo generates its SConscript in post_build?

salkinium commented 3 months ago

We need a post_post_build step!1!! Lol no, there's an undocumented module.order : int = 0 that you can set in the init() to enforce module order. Here we could set it to 10000 to enforce it's build after all other post_build steps.

calebchalmers commented 3 months ago

Could you check if my simplification still works correctly for your use case?

Yep, works for me :+1: