Skip to content

Conversation

JordanYates
Copy link
Contributor

@JordanYates JordanYates commented Oct 21, 2025

Add an option to preserve the EDK folder, instead of deleting it after creating the archive. This can simplify test scripting which can now avoid immediately uncompressing the archive when compiling an extension.

If this option is enabled, default to the much faster .tar.Z archive format, since the compression ratio of the archive is not important.

@pillo79
Copy link
Contributor

pillo79 commented Oct 21, 2025

Not really against this, but archiving-but-leaving-the-folder feels a bit of a stretch. As an alternative, I would suggest adding (something like) CONFIG_LLEXT_EDK_FORMAT_DIRECTORY instead; that would skip the whole create archive + delete dir and just leave the directory in the build folder. WDYT?

teburd
teburd previously approved these changes Oct 21, 2025
@teburd
Copy link
Contributor

teburd commented Oct 21, 2025

I'm ok with this, or providing a format output option as suggseted by @pillo79

@JordanYates
Copy link
Contributor Author

Not really against this, but archiving-but-leaving-the-folder feels a bit of a stretch. As an alternative, I would suggest adding (something like) CONFIG_LLEXT_EDK_FORMAT_DIRECTORY instead; that would skip the whole create archive + delete dir and just leave the directory in the build folder. WDYT?

I did this originally, but all the cmake logic around actually triggering the generation uses the archive as the output file the scripts are generating. I believe the only way to make this work would be to have FORMAT_DIRECTORY create a dummy file for use by cmake. If that's the way you want to go, I can try that.

zephyr/CMakeLists.txt

Lines 2388 to 2438 in b677e82

if(CONFIG_LLEXT_EDK)
if(CONFIG_LLEXT_EDK_FORMAT_TAR_XZ)
set(llext_edk_extension "tar.xz")
elseif(CONFIG_LLEXT_EDK_FORMAT_TAR_ZSTD)
set(llext_edk_extension "tar.Z")
elseif(CONFIG_LLEXT_EDK_FORMAT_ZIP)
set(llext_edk_extension "zip")
else()
message(FATAL_ERROR "Unsupported LLEXT_EDK_FORMAT choice")
endif()
set(llext_edk_file ${PROJECT_BINARY_DIR}/${CONFIG_LLEXT_EDK_NAME}.${llext_edk_extension})
# TODO maybe generate flags for C CXX ASM
zephyr_get_compile_definitions_for_lang(C zephyr_defs)
zephyr_get_compile_options_for_lang(C zephyr_flags)
# Filter out non LLEXT and LLEXT_EDK flags - and add required ones
llext_filter_zephyr_flags(LLEXT_REMOVE_FLAGS ${zephyr_flags} llext_filt_flags)
llext_filter_zephyr_flags(LLEXT_EDK_REMOVE_FLAGS ${llext_filt_flags} llext_filt_flags)
set(llext_edk_cflags ${zephyr_defs} -DLL_EXTENSION_BUILD)
list(APPEND llext_edk_cflags ${llext_filt_flags})
list(APPEND llext_edk_cflags ${LLEXT_APPEND_FLAGS})
list(APPEND llext_edk_cflags ${LLEXT_EDK_APPEND_FLAGS})
build_info(llext-edk file PATH ${llext_edk_file})
build_info(llext-edk cflags VALUE ${llext_edk_cflags})
build_info(llext-edk include-dirs VALUE "$<TARGET_PROPERTY:zephyr_interface,INTERFACE_INCLUDE_DIRECTORIES>")
add_custom_command(
OUTPUT ${llext_edk_file}
# Regenerate syscalls in case CONFIG_LLEXT_EDK_USERSPACE_ONLY
COMMAND ${CMAKE_COMMAND}
-E make_directory edk/include/generated/zephyr
COMMAND
${PYTHON_EXECUTABLE}
${ZEPHYR_BASE}/scripts/build/gen_syscalls.py
--json-file ${syscalls_json} # Read this file
--base-output edk/include/generated/zephyr/syscalls # Write to this dir
--syscall-dispatch edk/include/generated/zephyr/syscall_dispatch.c # Write this file
--syscall-list ${edk_syscall_list_h}
$<$<BOOL:${CONFIG_LLEXT_EDK_USERSPACE_ONLY}>:--userspace-only>
${SYSCALL_LONG_REGISTERS_ARG}
${SYSCALL_SPLIT_TIMEOUT_ARG}
COMMAND ${CMAKE_COMMAND}
-P ${ZEPHYR_BASE}/cmake/llext-edk.cmake
DEPENDS ${logical_target_for_zephyr_elf} ${syscalls_json} build_info_yaml_saved
COMMAND_EXPAND_LISTS
)
add_custom_target(llext-edk DEPENDS ${llext_edk_file})
endif()

pillo79
pillo79 previously approved these changes Oct 22, 2025
Copy link
Contributor

@pillo79 pillo79 left a comment

Choose a reason for hiding this comment

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

... I knew it couldn't be that easy, thanks for exploring the option! LGTM as well 👍
EDIT: but fix the other stray compliance errors in the file to make CI happy.

Fix existing formatting errors causing CI to fail on new PRs.

Signed-off-by: Jordan Yates <[email protected]>
Add an option to preserve the EDK folder, instead of deleting it after
creating the archive. This can simplify test scripting which can now
avoid immediately uncompressing the archive when compiling an
extension.

If this option is enabled, default to the much faster `.tar.Z` archive
format, since the compression ratio of the archive is not important.

Signed-off-by: Jordan Yates <[email protected]>
@JordanYates JordanYates dismissed stale reviews from pillo79 and teburd via b266bba October 22, 2025 07:15
@JordanYates
Copy link
Contributor Author

Added a commit to fix old formatting errors that CI was complaining about.
Also defaulted to the much faster archive format if we're not deleting the folder, since it is a safe bet we don't care about the compression efficiency if we're not planning on using it.

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Build System area: llext Linkable Loadable Extensions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants