Compilation things#385
Conversation
… defined in module but invoked in CLI code
… flow of libmilkscript best as I could. Fixes the ctests.
…dules, _compute libraries are now near-duplicate targets and unneeded.
There was a problem hiding this comment.
Pull request overview
This PR refactors module discovery/registration in milk to eliminate reliance on ELF constructors and remove the hard link-time dependency from modules to CLIcore. Modules now export a MILK_MODULE_INFO descriptor (via MILK_MODULE()), and CLIcore/libmilkscript drives registration post-dlopen() by looking up __milk_module_info. As a consequence, the PR also removes most _compute library variants and updates standalone linkage to reduce build times.
Changes:
- Introduce
MILK_MODULE_INFO+MILK_MODULE()descriptor-based module registration, and migrate modules away fromINIT_MODULE_LIB(). - Update module loading to perform “new-style” registration after
dlopen()and adjustmilkscript_init()to load core modules by shared library name. - Remove/replace
_computelibraries across multiple plugin/core modules and update CMake standalone linkage and CFITSIO gating.
Reviewed changes
Copilot reviewed 55 out of 55 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| src/sequencer/sequencer.c | Switch sequencer module to MILK_MODULE() descriptor registration. |
| src/milk_module_example/milk_module_example.c | Update example module to new MILK_MODULE() pattern. |
| src/engine/libfps/fpsCTRL/fpsCTRL_TUI.c | Remove dead preprocessor block. |
| src/engine/libfps/core/fps_struct_create.c | Adjust FPS struct creation; removes prior module list population. |
| src/coremods/COREMOD_tools/COREMOD_tools.c | Migrate COREMOD_tools to MILK_MODULE() registration. |
| src/coremods/COREMOD_tools/CMakeLists.txt | CFITSIO option gating and replace removed compute-only outputs with static-LTO archive. |
| src/coremods/COREMOD_memory/COREMOD_memory.c | Migrate COREMOD_memory to MILK_MODULE() registration. |
| src/coremods/COREMOD_memory/CMakeLists.txt | Simplify includes/linking, remove compute-only libraries, add static-LTO archive. |
| src/coremods/COREMOD_iofits/COREMOD_iofits.c | Update global init and migrate to MILK_MODULE() registration. |
| src/coremods/COREMOD_iofits/CMakeLists.txt | Remove compute-only variants; add static-LTO archive rules. |
| src/coremods/COREMOD_arith/COREMOD_arith.c | Migrate COREMOD_arith to MILK_MODULE() registration; remove legacy init wrapper. |
| src/coremods/COREMOD_arith/CMakeLists.txt | Remove compute-only variants; add static-LTO archive rules. |
| src/cli/libmilkscript/milkscript_api.c | Replace explicit libinit_* calls with load_module_shared() by library name. |
| src/cli/libmilkscript/CMakeLists.txt | Gate pkg_check_modules(CFITSIO ...) on USE_CFITSIO and tighten conditions. |
| src/cli/libmilkscript/CLIcore_modules.c | Add new-style module registration via __milk_module_info after dlopen(). |
| src/cli/CLIcore/CMakeLists.txt | Tighten CFITSIO condition (USE_CFITSIO && CFITSIO_FOUND). |
| src/cli/CLIcore/CLIcore.h | Add MILK_MODULE_INFO/MILK_MODULE() and adjust legacy macros; modifies data symbol handling. |
| src/cli/CLIcore/CLIcore_standalone.h | Add standalone MILK_MODULE() no-op and simplify legacy dep macros. |
| plugins/milk-extra-src/ZernikePolyn/CMakeLists.txt | Remove compute-only target; link standalones to main shared library. |
| plugins/milk-extra-src/statistic/CMakeLists.txt | Remove compute-only target; add static-LTO archive output and update deps. |
| plugins/milk-extra-src/linopt_imtools/linRM_from_inout.c | Guard FITS includes/saves under USE_CFITSIO. |
| plugins/milk-extra-src/linopt_imtools/linopt_imtools.c | Migrate to MILK_MODULE() registration. |
| plugins/milk-extra-src/linopt_imtools/CMakeLists.txt | Remove compute-only target; link standalones to main shared library. |
| plugins/milk-extra-src/linARfilterPred/linARfilterPred.c | Migrate to MILK_MODULE() registration. |
| plugins/milk-extra-src/linARfilterPred/CMakeLists.txt | Remove compute-only targets; link standalones to main libs and milklinalgebra. |
| plugins/milk-extra-src/linalgebra/linalgebra.h | Remove constructor declaration from header. |
| plugins/milk-extra-src/linalgebra/linalgebra.c | Migrate to MILK_MODULE() registration; move CUDA init to constructor helper. |
| plugins/milk-extra-src/linalgebra/CMakeLists.txt | Remove compute-only targets; link standalones to main shared library. |
| plugins/milk-extra-src/info/info.h | Remove large commented-out legacy prototypes. |
| plugins/milk-extra-src/info/info.c | Migrate to MILK_MODULE() registration; remove large commented-out legacy code. |
| plugins/milk-extra-src/info/CMakeLists.txt | Remove compute-only target. |
| plugins/milk-extra-src/image_gen/image_gen.h | Remove constructor declaration from header. |
| plugins/milk-extra-src/image_gen/image_gen.c | Migrate to MILK_MODULE() registration; refactor CLI registration block. |
| plugins/milk-extra-src/image_gen/CMakeLists.txt | Remove compute-only targets; add static-LTO archive output and update standalone links. |
| plugins/milk-extra-src/image_format/image_format.h | Remove constructor declaration from header. |
| plugins/milk-extra-src/image_format/image_format.c | Migrate to MILK_MODULE() registration; remove large commented-out legacy code. |
| plugins/milk-extra-src/image_format/combineHDR.c | Guard FITS I/O under USE_CFITSIO with a runtime error path otherwise. |
| plugins/milk-extra-src/image_format/CMakeLists.txt | Link standalone to milkimagefilter instead of removed compute lib. |
| plugins/milk-extra-src/image_filter/image_filter.h | Normalize include guards and remove constructor declaration. |
| plugins/milk-extra-src/image_filter/image_filter.c | Migrate to MILK_MODULE() registration. |
| plugins/milk-extra-src/image_filter/CMakeLists.txt | Remove compute-only targets. |
| plugins/milk-extra-src/image_basic/image_basic.h | Remove constructor declaration and large commented-out legacy prototypes. |
| plugins/milk-extra-src/image_basic/image_basic.c | Migrate to MILK_MODULE() registration; remove large commented-out legacy code. |
| plugins/milk-extra-src/image_basic/CMakeLists.txt | Remove compute-only targets. |
| plugins/milk-extra-src/fft/wisdom.c | Refactor FFTWMT preprocessor branching; wisdom import/export currently commented out. |
| plugins/milk-extra-src/fft/fft.h | Remove constructor declaration from header. |
| plugins/milk-extra-src/fft/fft.c | Migrate to MILK_MODULE() registration and add FFTW init/cleanup constructors. |
| plugins/milk-extra-src/fft/CMakeLists.txt | Remove compute-only target; link standalones to main shared library. |
| plugins/milk-extra-src/clustering/mindiffscan.c | Add CFITSIO guards around FITS I/O (currently inconsistent macro usage). |
| plugins/milk-extra-src/clustering/CMakeLists.txt | Remove compute-only target; link standalone to main shared library. |
| plugins/milk-extra-src/clustering/clustering.c | Migrate to MILK_MODULE() registration. |
| milk-examples/example03_fps/example03fps_module.c | Update example module to MILK_MODULE() registration. |
| docs/dependency_graph.md | Update dependency graph for COREMOD_memory CFITSIO relationship. |
| cmake/MilkStandalone.cmake | Update standalone link sets to remove _compute core module dependencies; adjust CFITSIO handling. |
|
|
||
| // auto-generate libinit_<modulename> | ||
| // initialize INITSTATUS_<modulename> | ||
| INIT_MODULE_LIB(fft) | ||
| CLIADDCMD_milk_fft__pup2foc(); | ||
| } |
| //#ifdef MILK_MODULE | ||
| // if (fftwf_import_wisdom_from_file(fp) == 0) | ||
| // { | ||
| // PRINT_WARNING("Error reading wisdom"); | ||
| // } |
There was a problem hiding this comment.
This was already broken. MILK_MODULE doesn't exist in the codebase in framework-dev.
Not fixing.
| //#ifdef MILK_MODULE | ||
| // if (fftw_import_wisdom_from_file(fp) == 0) | ||
| // { | ||
| // PRINT_WARNING("Error reading wisdom"); | ||
| // } | ||
| //#endif |
| __attribute__((weak)) DATA data = { .core = { 0 } }; | ||
| //extern DATA data; |
There was a problem hiding this comment.
This is wrong and ((weak)) is the corner to delinking CLICore.
| #ifdef CFITSIO | ||
| printf("\nsaving to filesystem\n"); | ||
| save_fl_fits("distmat", "distmat.fits"); | ||
| #endif |
|
|
||
| MILK_MODULE(COREMOD_tools, init_module_CLI, NULL); | ||
|
|
||
| #endif /* ELSE MILK_NO_CLI */ |
| // vdeo: well that's unfortunate but this stale #define | ||
| // that wasn't used uses the same name as something | ||
| // I just made !! | ||
| //#ifdef MILK_MODULE | ||
| // fftwf_export_wisdom_to_file(fp); | ||
| //#endif | ||
| fclose(fp); |
| //#ifdef MILK_MODULE | ||
| // fftw_export_wisdom_to_file(fp); | ||
| //#endif | ||
| fclose(fp); |
| milkdata | ||
| milkprocessinfo | ||
| ImageStreamIO | ||
| milkCOREMODmemory_compute | ||
| milkCOREMODtools_compute | ||
| milkCOREMODarith_compute | ||
| milkCOREMODiofits_compute | ||
| ${CFITSIO_LIBRARIES} | ||
| milkCOREMODmemory | ||
| milkCOREMODtools | ||
| milkCOREMODarith |
Resolve the way by which modules are linked to milk-script / milk-cli
Deprecation of the INIT_MILK_MODULE macro and definition of another one MILK_MODULE instead.
Migrate the modules from milk (but not cacao)
As a consequence, there's no linkage dependency from modules to CLICore.
As such, the _compute variants of the libraries have little technical interest, and we can shave 30% of compile time, so I removed them.
I also cached something of fps_standalone_stubs into an O library during compilation, which avoids it being recompiled by every single fpsexec.