Skip to content

llext: make EDK Kconfig menu visible only when LLEXT is enabled #84850

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

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

llext: make EDK Kconfig menu visible only when LLEXT is enabled #84850

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions subsys/llext/Kconfig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ module = LLEXT
module-str = llext
source "subsys/logging/Kconfig.template.log_config"

endif

menu "Linkable loadable Extension Development Kit (EDK)"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Turn this into a menuconfig?

Copy link
Collaborator

@pillo79 pillo79 Jan 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This was initially the case; then AFAICS this review comment resulted in CONFIG_LLEXT_EDK being removed during a review cycle.
(FWIW I'm all for re-adding it or making it dependent on CONFIG_LLEXT... just recalling history here).

Copy link
Collaborator

Choose a reason for hiding this comment

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

CC @tejlmand

Copy link
Collaborator

@tejlmand tejlmand Feb 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I guess you're looking for this comment: #69831 (comment)
Notice it was just an observation, not a request for change 😉

Main reason for that comment was because the llext-edk build target was a custom one, which needs to be called explicitly, thus indirectly calling this target means the user wants the output from llext-edk.

But I see the point that having llext config settings available if user has no intention of using llext-edk can be confusing.
The early assumption of just being able to call an llext-edk build target when needed might not make as much sense anymore if the user anyway has to make several decisions regarding llext-edk export options, which seems to be the case.

I'm fine with adding back the config part, and if doing so, then I also think the llext-edk target creation should be safeguarded, so that it's clear to users that there are extra settings to consider.
Of course that will imply that one cannot just call the llext-edk target on any build, but must remember to enable the config first.
I don't know how commen that is, @pillo79 any input ?

Updated: correct writing llext-edk instead of llext

Copy link
Collaborator

@pillo79 pillo79 Feb 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

As Ederson said just below, LLEXT and the EDK are somehow independent of each other.
Actually, in my PR I set the default to y if LLEXT since that was the de-facto situation, but I would also have preferred keeping that off unless explicitly asked for. There's already documentation on the EDK, I think if you are looking for that functionality, shouldn't be difficult to find out you have to enable it. @edersondisouza @teburd would you be OK with this?

I also have no problem on renaming LLEXT_EDK to EDK to make that extra clear; will patch #85624 to see how that looks like EDIT: approved, so will push a new PR with that change 😉

Copy link
Collaborator

Choose a reason for hiding this comment

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

As #84850 (comment) just below, LLEXT and the EDK are somehow independent of each other.

Sorry @pillo79 , I of course meant llext-edk in my earlier message, as only llext-edk has a dedicated build target.
So the question I tried to ask, how common is it to just call llext-edk on a build, and not having to adjust any of the llext-edk Kconfig settings for some reason.
I guess it will be ok for users to have to first set a CONFIG_LLEXT_EDK=y before being able to execute the target.


config LLEXT_EDK_NAME
Expand All @@ -135,3 +133,5 @@ config LLEXT_EDK_USERSPACE_ONLY
not contain the routing code, and only generate the userspace one.

endmenu

endif # LLEXT
Loading