wcbonner / GoveeBTTempLogger

Govee H5072, H5074, H5075, H5100, H5101, H5104, H5105, H5174, H5177, H5179, H5181, H5182, and H5183 Bluetooth Low Energy Temperature and Humidity Logger
MIT License
177 stars 26 forks source link

Remove postbuild step from CMakeLists with build flag #54

Closed franzos closed 9 months ago

franzos commented 11 months ago

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:

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?

wcbonner commented 11 months ago

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.

wcbonner commented 11 months ago

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

franzos commented 11 months ago

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.

wcbonner commented 11 months ago

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

franzos commented 10 months ago

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.

wcbonner commented 9 months ago

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

See this update: https://github.com/wcbonner/GoveeBTTempLogger/commit/f34dbb229b5ee3939de35c538f842ca5932af0a5