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

library_manager: add missing "depends on CONFIG_LLEXT" #9222

Closed
wants to merge 1 commit into from

Conversation

marc-hb
Copy link
Collaborator

@marc-hb marc-hb commented Jun 11, 2024

Block the possibility of compiling library_manager without CONFIG_LLEXT, which obviously fails with dozens of linker errors.

PS: I have no idea why library_manager depends on IPC_MAJOR_4. Kconfig is designed to express actual code dependencies, not "supported" configurations.

Block the possibility of compiling library_manager without CONFIG_LLEXT,
which obviously fails with dozens of linker errors.

PS: I have no idea why library_manager depends on IPC_MAJOR_4.  Kconfig
is designed to express actual code dependencies, not "supported"
configurations.

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
@marc-hb marc-hb marked this pull request as ready for review June 12, 2024 00:13
@marc-hb marc-hb marked this pull request as draft June 12, 2024 00:48
@marc-hb
Copy link
Collaborator Author

marc-hb commented Jun 12, 2024

Loadable modules tests failed in https://sof-ci.01.org/sof-pr-viewer/#/build/PR9222/build14036351 so this requires more investigation.

@pjdobrowolski
Copy link
Contributor

which build target it is affecting? Loadable modules are feature supported from mtl and up.

@@ -7,6 +7,7 @@ menu "Library Manager"
config LIBRARY_MANAGER
bool "Library Manager Support"
default n
depends on CONFIG_LLEXT
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is correct. The library manager can also be used for LMDK and IADK modules, which don't need LLEXT

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was even more incorrect because it should have been:

Suggested change
depends on CONFIG_LLEXT
depends on LLEXT

@marc-hb
Copy link
Collaborator Author

marc-hb commented Jun 12, 2024

I see a related Kconfig fix in #9199, so maybe that helped. Will try again.

Loadable modules are feature supported from mtl and up.

This does not matter. See commit message:

Kconfig is designed to express actual code dependencies, not "supported" configurations.

There is at least one, non-Intel configuration with Zephyr+IPC3 right now. I doubt it will ever want to use LLEXT but it is not Kconfig's job to stop that and it should be possible to try as long as there's no incompatibility at the code level.

Kconfig is not a project management tool or bug tracking tool.

https://docs.zephyrproject.org/latest/build/dts/dt-vs-kconfig.html

@marc-hb
Copy link
Collaborator Author

marc-hb commented Aug 23, 2024

No time to get to the bottom of this. Just filed more generic:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants