wcbonner/GoveeBTTempLogger

Remove postbuild step from CMakeLists with build flag

franzos opened this issue · 6 comments

Hi there, thank you for developing this awesome application!

I wrote a guix package for the latest release:

(define-public goveebttemplogger
  (package
    (name "goveebttemplogger")
    (version "2.20231001.1")
    (source
     (origin
       (method url-fetch)
       (uri (string-append
             "https://github.com/wcbonner/GoveeBTTempLogger/archive/refs/tags/v"
             version ".tar.gz"))
       (sha256
        (base32 "00hsyxz6v0ksq4x6199hv0da5rg4z6s9g7vnkw3r1yfv9cc8j7xx"))
       (patches (search-patches "goveebttemplogger-postbuild-sudo-fix.patch"))))
    (build-system cmake-build-system)
    (inputs (list bluez))
    (home-page "https://github.com/wcbonner/GoveeBTTempLogger")
    (synopsis "Temperature and Humidity Logger for Goove devices")
    (description
     "Govee H5074, H5075, H5100, H5174, H5177, H5179, 
H5181, H5182, and H5183 Bluetooth Low Energy Temperature and Humidity Logger")
    (license license:expat)))

I had to make a couple of changes for this to work:

  • remove POST_BUILD, not sure what's best practive here, but since this may be build on a computer that will not run it, this may be undesired (sudo setcap 'cap_net_raw,cap_net_admin+eip'...). In my case it failed because the build does not allow side-effects.
  • install to ${CMAKE_INSTALL_PREFIX}/..

Here's the diff with which I can build successfully:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index c2b3f35..4902d97 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -37,11 +37,11 @@ target_include_directories(goveebttemplogger PUBLIC
                            ${EXTRA_INCLUDES}
                            )
 
-add_custom_command(TARGET goveebttemplogger POST_BUILD
-    COMMAND sudo setcap 'cap_net_raw,cap_net_admin+eip' $<TARGET_FILE:goveebttemplogger>
-    WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
-    COMMENT "Setting Raw Priveleges on $<TARGET_FILE:goveebttemplogger>"
-)
+# add_custom_command(TARGET goveebttemplogger POST_BUILD
+#     COMMAND sudo setcap 'cap_net_raw,cap_net_admin+eip' $<TARGET_FILE:goveebttemplogger>
+#     WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
+#     COMMENT "Setting Raw Priveleges on $<TARGET_FILE:goveebttemplogger>"
+# )
 
 add_executable(gvh-organizelogs gvh-organizelogs.cpp goveebttemplogger-version.h)
 target_link_libraries(gvh-organizelogs -lbluetooth -lstdc++fs)
@@ -62,12 +62,12 @@ add_test(NAME gvh-organizelogs COMMAND gvh-organizelogs --help)
 
 install(TARGETS goveebttemplogger gvh-organizelogs
     DESTINATION bin
-    RUNTIME DESTINATION "/usr/local/bin/"
-    LIBRARY DESTINATION "/usr/local/lib/"
+    RUNTIME DESTINATION ${CMAKE_INSTALL_PREFIX}/bin
+    LIBRARY DESTINATION ${CMAKE_INSTALL_PREFIX}/lib
 )
 
 install(FILES goveebttemplogger.service
-    DESTINATION "/usr/local/lib/systemd/system"
+    DESTINATION "${CMAKE_INSTALL_PREFIX}/systemd/system"
     COMPONENT "goveebttemplogger"
 )

I'm not too familiar with cmake. I guess this should be adjustable with build flags?

I'm not too familiar with cmake. I guess this should be adjustable with build flags?

Hmm. I'm new to cmake myself so trying to optimize things is good.

I like having the net capability enabled as part of the build process because it makes debugging possible without running the debugger as root. I'll have to figure out more conditional stuff in CMake.

I'd not heard of GNU Guix before. I read a little about it at https://guix.gnu.org/blog/2018/a-packaging-tutorial-for-guix/#Guix_Package_Path

I also had to refresh my memory of what type packages CPack can create by reading https://cmake.org/cmake/help/latest/manual/cpack-generators.7.html

I'd not heard of GNU Guix before.

Still pretty niche; Maybe Nix / NixOS rings a bell, though even that is niche I suppose - even in the Linux world.

I'm thinking of changing the setcap feature to be capability dependent. I need to figure out if I can recognize if sudo setcap can be run during the CMake check dependencies process, and then only enable it if it works without user input.

Reading this thread right now about possibilities. https://stackoverflow.com/questions/72012474/integrate-granting-of-capabilities-into-the-build-process

How about the flag, I mentioned before? If you prefer this to be the default, maybe a flag to --skip_setcap. It sure seems to work fine for most if not all other users.

@franzos Does conditionally running the command based on existence of a Raspberry Pi issue file fix your issue?

See this update: f34dbb2