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.

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

improve CPIO creation #151

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

improve CPIO creation #151

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
60c2b4e
cmake: reduce redundancy in MakeCPIO()
Jul 6, 2022
9e818c2
cmake: use verbose cpio flags
Jul 6, 2022
f89897f
cmake: Improve CPIO creation
Sep 15, 2023
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
111 changes: 82 additions & 29 deletions cmake-tool/helpers/cpio.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,48 +39,101 @@ function(MakeCPIO output_name input_files)
if(NOT "${MAKE_CPIO_UNPARSED_ARGUMENTS}" STREQUAL "")
Copy link
Member

Choose a reason for hiding this comment

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

This pattern is repeated a lot throughout the CMake files. I recall it's partially because if (${MAKE_CPIO_UNPARSED_ARGUMENTS}) unexpectedly evaluates to false for almost all string values as CMake's described behavior (https://cmake.org/cmake/help/latest/command/if.html).

Using if(MAKE_CPIO_UNPARSED_ARGUMENTS) is shorter and you know that this tests if the variable is defined and defined to non-false values. But it would be easy to misread something like if(${MAKE_CPIO_UNPARSED_ARGUMENTS} or if("${MAKE_CPIO_UNPARSED_ARGUMENTS}") as testing for truth if you aren't more familiar with the if() gotchas . In these cases, if the variable contains a string it will always evaluate to false unless the contents match the truthy string values (eg ON, TRUE, etc). So while "${MAKE_CPIO_UNPARSED_ARGUMENTS}" STREQUAL "") is more verbose, it's probably a good pattern to leave around for explicitly testing for a non-empty string as it doesn't rely on understanding any if() testing gotchas.

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with all points. This is the outcame from digging into CMake usage and a long term plan to use the language as it seems intended nowadays. I can move this to a separate PR to make the intention more clear. There are more places where this could be changed to clarify what checks are non-trivial.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have remove the commit

message(FATAL_ERROR "Unknown arguments to MakeCPIO")
endif()

set(archive_symbol "_cpio_archive")
if(NOT "${MAKE_CPIO_CPIO_SYMBOL}" STREQUAL "")
set(archive_symbol ${MAKE_CPIO_CPIO_SYMBOL})
endif()
# Check that the reproducible flag is available. Don't use it if it isn't.

set(cpio_archive "${output_name}.cpio")

# CMake wants absolute paths when calling file(WRITE ...)
get_filename_component(
output_name_abs
"${output_name}"
ABSOLUTE
BASE_DIR
"${CMAKE_CURRENT_BINARY_DIR}"
)

# Create a script that prepares CPIO archive contents in a tmp folder and
# then builds the archive. Some cpio versions support the paramter
# "--reproducible" to create the archive with consistent inodes and device
# numbering. In addition, we set each file's the 'modified time' to the
# epoch (ie 0, but simply using 'touch -d @0' is a GNU extension of the
# POSIX standard), and the user/group values to 0:0.
# Since some application access the archive contents by index and not by
# name, the files must be put into the archive in exactly the same order as
# they are listen in 'input_files'. Using simply "ls | cpio ...." instead of
# "printf '%s\\n' \${@##*/} | cpio ..." does not guarantee preserving the
# order.
CheckCPIOArgument(cpio_reproducible_flag "--reproducible")
set(
commands
"bash;-c;cpio ${cpio_reproducible_flag} --quiet --create -H newc --file=${CMAKE_CURRENT_BINARY_DIR}/archive.${output_name}.cpio;&&"
set(cpio_archive_creator "${output_name_abs}.sh")
file(
WRITE
"${cpio_archive_creator}"
# content
"#!/bin/sh\n"
"# auto-generated file from MakeCPIO(), do not edit\n"
axel-h marked this conversation as resolved.
Show resolved Hide resolved
"set -e\n"
"TMP_DIR=${cpio_archive}.files\n"
"mkdir -p \${TMP_DIR}\n"
"cp -a \"$@\" \${TMP_DIR}/\n"
"touch -d 1970-01-01T00:00:00Z \${TMP_DIR}/*\n"
"(\n"
" cd \${TMP_DIR}\n"
" printf '%s\\n' \${@##*/} | cpio ${cpio_reproducible_flag} --quiet --create --format=newc --owner=+0:+0\n"
") > ${cpio_archive}\n"
"rm -rf \${TMP_DIR}\n"
)
foreach(file IN LISTS input_files)
# Try and generate reproducible cpio meta-data as we do this:
# - touch -d @0 file sets the modified time to 0
# - --owner=root:root sets user and group values to 0:0
# - --reproducible creates reproducible archives with consistent inodes and device numbering
list(
APPEND
commands
"bash;-c; mkdir -p temp_${output_name} && cd temp_${output_name} && cp -a ${file} . && touch -d 1970-01-01T00:00:00Z `basename ${file}` && echo `basename ${file}` | cpio --append ${cpio_reproducible_flag} --owner=+0:+0 --quiet -o -H newc --file=${CMAKE_CURRENT_BINARY_DIR}/archive.${output_name}.cpio && rm `basename ${file}` && cd ../ && rmdir temp_${output_name};&&"
)
endforeach()
list(APPEND commands "true")

# Create a "program" that makes the compiler generate and object file that
# contains the cpio archive.
# CMake wants the absolute name when creating files
set(cpio_archive_s "${output_name_abs}.S")
file(
WRITE
"${cpio_archive_s}"
# content
"# auto-generated file from MakeCPIO(), do not edit\n"
".section ._archive_cpio, \"aw\"\n"
".globl ${archive_symbol}, ${archive_symbol}_end\n"
"${archive_symbol}:\n"
".incbin \"${cpio_archive}\"\n"
"${archive_symbol}_end:\n"
)

# Re-run CMake configuration in case 'cpio_archive_creator' or
# 'cpio_archive_s' is deleted.
set_property(
DIRECTORY
APPEND
PROPERTY CMAKE_CONFIGURE_DEPENDS "${cpio_archive_creator}" "${cpio_archive_s}"
)
Comment on lines +106 to +112
Copy link
Member

Choose a reason for hiding this comment

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

I don't consider that this property is needed unless CMake implicitly adds it for all files it generates in the build directory already. Trying to detect and recover the build directory becoming corrupted isn't the responsibility of the build scripts.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a convenience feature I found quite useful when experimenting with the archive creation. In general, when debugging CMake things, I often corrupt the build folder or delete files on purpose to force partial regeneration. And my expectation from proper dependency management is, that everything in the build folder should be re-generated automatically.


# The 'cpio_archive' is no explicit parameter for the command, because it is
# hard-coded already in the specific script we have generated above. The
# 'input_files' are explicit here, because they are a CMake list that
# will be converted to parameters for the invokation,
add_custom_command(
OUTPUT ${cpio_archive}
COMMAND bash "${cpio_archive_creator}" ${input_files}
DEPENDS "${cpio_archive_creator}" ${input_files} ${MAKE_CPIO_DEPENDS}
COMMENT "Generate CPIO archive ${cpio_archive}"
)

separate_arguments(cmake_c_flags_sep NATIVE_COMMAND "${CMAKE_C_FLAGS}")
if(CMAKE_C_COMPILER_ID STREQUAL "Clang")
list(APPEND cmake_c_flags_sep "${CMAKE_C_COMPILE_OPTIONS_TARGET}${CMAKE_C_COMPILER_TARGET}")
endif()

# The 'cpio_archive' is no explicit parameter for the command, because it is
# hard-coded already in the specific 'cpio_archive_s' file we have generated
# above.
add_custom_command(
OUTPUT ${output_name}
COMMAND rm -f archive.${output_name}.cpio
COMMAND ${commands}
COMMAND
sh -c
"echo 'X.section ._archive_cpio,\"aw\"X.globl ${archive_symbol}, ${archive_symbol}_endX${archive_symbol}:X.incbin \"archive.${output_name}.cpio\"X${archive_symbol}_end:X' | tr X '\\n'"
> ${output_name}.S
COMMAND
${CMAKE_C_COMPILER} ${cmake_c_flags_sep} -c -o ${output_name} ${output_name}.S
DEPENDS ${input_files} ${MAKE_CPIO_DEPENDS}
VERBATIM
BYPRODUCTS
archive.${output_name}.cpio
${output_name}.S
${CMAKE_C_COMPILER} ${cmake_c_flags_sep} -c -o "${output_name}" "${cpio_archive_s}"
DEPENDS "${cpio_archive_creator}" "${cpio_archive_s}" "${cpio_archive}"
COMMENT "Generate CPIO archive ${output_name}"
)
endfunction(MakeCPIO)