Skip to content

cmake: execute daemon-reload and restart postinst in deb #10899

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 4 commits into
base: master
Choose a base branch
from
Open

cmake: execute daemon-reload and restart postinst in deb #10899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
910bdc0
cmake: execute daemon-reload and restart postinst in deb
aminvakil Sep 18, 2025
ba29ff5
Improve systemd daemon-reload command by adding default
aminvakil Sep 20, 2025
a712a28
Put debian/postinst.in in separate file
aminvakil Sep 20, 2025
3c3a743
Execute systemd stuff if ran by systemd
aminvakil Sep 20, 2025
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
12 changes: 10 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1485,9 +1485,10 @@ if(DPKG_PROGRAM)
${PROJECT_SOURCE_DIR}/cpack/debian/conffiles
)

set(LDCONFIG_SCRIPT_CMDS "")
if(FLB_RUN_LDCONFIG)
set(LDCONFIG_DIR ${FLB_INSTALL_LIBDIR})
file(WRITE ${PROJECT_BINARY_DIR}/scripts/postinst "
set(LDCONFIG_SCRIPT_CMDS "
mkdir -p /etc/ld.so.conf.d
echo \"${LDCONFIG_DIR}\" > /etc/ld.so.conf.d/libfluent-bit.conf
ldconfig
Expand All @@ -1496,9 +1497,16 @@ ldconfig
rm -f -- /etc/ld.so.conf.d/libfluent-bit.conf
ldconfig
")
set(CPACK_DEBIAN_RUNTIME_PACKAGE_CONTROL_EXTRA "${PROJECT_BINARY_DIR}/scripts/postinst;${PROJECT_BINARY_DIR}/scripts/prerm")
list(APPEND CPACK_DEBIAN_RUNTIME_PACKAGE_CONTROL_EXTRA "${PROJECT_BINARY_DIR}/scripts/postinst;${PROJECT_BINARY_DIR}/scripts/prerm")
endif(FLB_RUN_LDCONFIG)

configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/cpack/debian/postinst.in
${PROJECT_BINARY_DIR}/scripts/postinst
)
Comment on lines +1503 to +1506
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

postinst generation lacks executable bit; ensure correct perms and use the corrected variable.

configure_file writes 0644 by default; Debian maintainer scripts must be executable. Also switch to the corrected LDCONFIG_POSTINST_CMDS.

Apply this diff:

-  configure_file(
-    ${CMAKE_CURRENT_SOURCE_DIR}/cpack/debian/postinst.in
-    ${PROJECT_BINARY_DIR}/scripts/postinst
-  )
+  configure_file(
+    "${CMAKE_CURRENT_SOURCE_DIR}/cpack/debian/postinst.in"
+    "${PROJECT_BINARY_DIR}/scripts/postinst"
+  )
+  execute_process(COMMAND "${CMAKE_COMMAND}" -E chmod +x "${PROJECT_BINARY_DIR}/scripts/postinst")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/cpack/debian/postinst.in
${PROJECT_BINARY_DIR}/scripts/postinst
)
configure_file(
"${CMAKE_CURRENT_SOURCE_DIR}/cpack/debian/postinst.in"
"${PROJECT_BINARY_DIR}/scripts/postinst"
)
execute_process(
COMMAND "${CMAKE_COMMAND}" -E chmod +x
"${PROJECT_BINARY_DIR}/scripts/postinst"
)
🤖 Prompt for AI Agents 
In CMakeLists.txt around lines 1503-1506, the postinst file produced by
configure_file is created with default 0644 and must be executable and the
variable name should be LDCONFIG_POSTINST_CMDS; after the configure_file call,
add a file(SET_PERMISSIONS ${PROJECT_BINARY_DIR}/scripts/postinst OWNER_READ
OWNER_WRITE OWNER_EXECUTE GROUP_READ GROUP_EXECUTE WORLD_READ WORLD_EXECUTE) to
give the script the executable bit, and update any references to the old
variable to use LDCONFIG_POSTINST_CMDS.


list(APPEND CPACK_DEBIAN_RUNTIME_PACKAGE_CONTROL_EXTRA "${PROJECT_BINARY_DIR}/scripts/postinst;${PROJECT_BINARY_DIR}/scripts/prerm")

Comment on lines +1508 to +1509
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Avoid duplicate appends and unconditional prerm reference.

Append postinst once, and only append prerm when it exists (already done inside the FLB_RUN_LDCONFIG block above).

Apply this diff:

-  list(APPEND CPACK_DEBIAN_RUNTIME_PACKAGE_CONTROL_EXTRA "${PROJECT_BINARY_DIR}/scripts/postinst;${PROJECT_BINARY_DIR}/scripts/prerm")
+  list(APPEND CPACK_DEBIAN_RUNTIME_PACKAGE_CONTROL_EXTRA "${PROJECT_BINARY_DIR}/scripts/postinst")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
list(APPEND CPACK_DEBIAN_RUNTIME_PACKAGE_CONTROL_EXTRA "${PROJECT_BINARY_DIR}/scripts/postinst;${PROJECT_BINARY_DIR}/scripts/prerm")
list(APPEND CPACK_DEBIAN_RUNTIME_PACKAGE_CONTROL_EXTRA "${PROJECT_BINARY_DIR}/scripts/postinst")
🤖 Prompt for AI Agents 
In CMakeLists.txt around lines 1508-1509, the current list(APPEND
CPACK_DEBIAN_RUNTIME_PACKAGE_CONTROL_EXTRA ...) unconditionally appends both
postinst and prerm which causes duplicate appends of postinst and incorrectly
references prerm even when not present; change this to only append postinst here
(and ensure it is appended only once overall) and remove the unconditional prerm
entry so prerm is appended only inside the existing FLB_RUN_LDCONFIG block where
its presence is checked.

endif()

# RPM Generation information
Expand Down
9 changes: 9 additions & 0 deletions cpack/debian/postinst.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/bin/sh
set -e
@LDCONFIG_POSTINST_CMDS@
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Fix variable name mismatch: template expects LDCONFIG_POSTINST_CMDS but CMake sets LDCONFIG_SCRIPT_CMDS.

As written, ldconfig commands will never be injected into postinst. Align the variable names (I suggest fixing CMake to set LDCONFIG_POSTINST_CMDS).

Run this to confirm the mismatch:

🏁 Script executed:

#!/bin/bash
rg -n -C1 'LDCONFIG_.*CMDS' cpack/debian/postinst.in CMakeLists.txt

Length of output: 518

Align CMake variable with postinst template
cpack/debian/postinst.in uses @LDCONFIG_POSTINST_CMDS@ but CMakeLists.txt defines LDCONFIG_SCRIPT_CMDS; rename the CMake variable to LDCONFIG_POSTINST_CMDS so the ldconfig commands are injected.

🤖 Prompt for AI Agents 
In cpack/debian/postinst.in around line 3 the template uses
@LDCONFIG_POSTINST_CMDS@ but the CMakeLists.txt defines LDCONFIG_SCRIPT_CMDS;
rename the CMake variable in CMakeLists.txt from LDCONFIG_SCRIPT_CMDS to
LDCONFIG_POSTINST_CMDS so the ldconfig commands are injected into the postinst
template, updating any places that set or append to that variable accordingly.

if [ "$1" = "configure" ] || [ "$1" = "abort-upgrade" ] || [ "$1" = "abort-deconfigure" ] || [ "$1" = "abort-remove" ] ; then
if [ -d /run/systemd/system ]; then
systemctl --system daemon-reload >/dev/null || true
systemctl try-restart @FLB_OUT_NAME@.service >/dev/null || true
fi
fi
Loading