Skip to contentSkip to conversation

taskd: support linking against static gnutls and uuid

PATCH Superseded Ben Boeckel 3 participants 7 messages Jun 17, 2016 – Aug 5, 2016

lore
Share & tools
lore

Ben Boeckel

author
#0 lore raw

PATCH 1/1 taskd: support linking against static gnutls and uuid

The FindGnuTLS module shipped with CMake does not expose the information required for proper linking if it is a static library.

Similarly, the code which searches for the uuid library is insufficient for such a task.

Also, find_package(Threads) was missing, so pthreads was not properly linked.

Do not send this patch to taskwarrior upstream; it is not a proper fix.

Fixes:
http://autobuild.buildroot.net/results/67f574af0cc4348ffe2bce026c44766e49c29124/

Signed-off-by Ben Boeckel <[email protected]>
---
 ...ix-linking-against-deps-statically-on-arc.patch | 52 ++++++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 package/taskd/0001-buildroot-fix-linking-against-deps-statically-on-arc.patch
package/taskd/0001-buildroot-fix-linking-against-deps-statically-on-arc.patch new file +52 -0
new file mode 100644
index 0000000..56f9dfc
@@ -0,0 +1,52 @@
+From b916016eaf3a3bdb555de11610c0b7964d52da96 Mon Sep 17 00:00:00 2001
+From: Ben Boeckel <[email protected]>
+Date: Thu, 16 Jun 2016 21:58:47 -0400
+Subject: [PATCH] buildroot: fix linking against deps statically on arc
+
+Also find the Threads package so that CMAKE_THREAD_LIBS_INIT is actually
+set.
+
+Explicitly not signing this off because it should not go to taskwarrior
+upstream; deeper fixes are required.
+---
+ CMakeLists.txt     | 4 ++--
+ src/CMakeLists.txt | 1 +
+ 2 files changed, 3 insertions(+), 2 deletions(-)
+
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 9a049b0..09f3209 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -49,7 +49,7 @@ find_package (GnuTLS REQUIRED)
+ if (GNUTLS_FOUND)
+   set (HAVE_LIBGNUTLS true)
+   set (TASKD_INCLUDE_DIRS ${TASKD_INCLUDE_DIRS} ${GNUTLS_INCLUDE_DIR})
+-  set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${GNUTLS_LIBRARIES})
++  set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${PC_GNUTLS_STATIC_LIBRARIES})
+ endif (GNUTLS_FOUND)
+ 
+ check_function_exists (timegm          HAVE_TIMEGM)
+@@ -68,7 +68,7 @@ else (DARWIN OR FREEBSD OR OPENBSD)
+   find_library (UUID_LIBRARY NAMES uuid)
+   if (UUID_INCLUDE_DIR AND UUID_LIBRARY)
+     set (TASKD_INCLUDE_DIRS ${TASKD_INCLUDE_DIRS} ${UUID_INCLUDE_DIR})
+-    set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${UUID_LIBRARY})
++    set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${UUID_LIBRARY} -lintl)
+     # Look for uuid_unparse_lower
+     set (CMAKE_REQUIRED_INCLUDES  ${CMAKE_REQUIRED_INCLUDES}  ${UUID_INCLUDE_DIR})
+     set (CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES} ${UUID_LIBRARY})
+diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
+index 5820392..243b0f9 100644
+--- a/src/CMakeLists.txt
++++ b/src/CMakeLists.txt
+@@ -35,6 +35,7 @@ add_library (taskd admin.cpp
+                    util.cpp       util.h
+                    wcwidth6.cpp)
+ 
++find_package(Threads REQUIRED)
+ add_executable (taskd_executable taskd.cpp)
+ target_link_libraries (taskd_executable taskd ${TASKD_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT})
+ set_property (TARGET taskd_executable PROPERTY OUTPUT_NAME "taskd")
+-- 
+2.9.0
+
Signature · 1 line
2.9.0
#1 lore raw

PATCH 1/1 taskd: support linking against static gnutls and uuid

Hi Ben,

Quoted 25 lines · Ben Boeckel

The FindGnuTLS module shipped with CMake does not expose the information required for proper linking if it is a static library.

Similarly, the code which searches for the uuid library is insufficient for such a task.

Also, find_package(Threads) was missing, so pthreads was not properly linked.

Do not send this patch to taskwarrior upstream; it is not a proper fix.

Fixes:
http://autobuild.buildroot.net/results/67f574af0cc4348ffe2bce026c44766e49c29124/

Signed-off-by Ben Boeckel <[email protected]>
---
 ...ix-linking-against-deps-statically-on-arc.patch | 52 ++++++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 package/taskd/0001-buildroot-fix-linking-against-deps-statically-on-arc.patch
package/taskd/0001-buildroot-fix-linking-against-deps-statically-on-arc.patch new file +0 -0
new file mode 100644
index 0000000..56f9dfc

package/taskd/0001-buildroot-fix-linking-against-deps-statically-on-arc.patch line 10

Quoted 10 lines
package/taskd/0001-buildroot-fix-linking-against-deps-statically-on-arc.patch new file +10 -0
@@ -0,0 +1,52 @@
+From b916016eaf3a3bdb555de11610c0b7964d52da96 Mon Sep 17 00:00:00 2001
+From: Ben Boeckel <[email protected]>
+Date: Thu, 16 Jun 2016 21:58:47 -0400
+Subject: [PATCH] buildroot: fix linking against deps statically on arc
+
+Also find the Threads package so that CMAKE_THREAD_LIBS_INIT is actually
+set.
+
+Explicitly not signing this off because it should not go to taskwarrior
+upstream; deeper fixes are required.

But you do need to sign-off if you want Buildroot to apply this patch.

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
author
#2 lore raw

PATCH 1/1 taskd: support linking against static gnutls and uuid

Quoted 1 line · Baruch Siach

But you do need to sign-off if you want Buildroot to apply this patch.

Is the signoff on the buildroot-level patch not sufficient? The patch in question here does not modify buildroot code, but taskwarrior code.

I'm not saying "no"; I just find it a bit odd.

--Ben

#3 lore raw

PATCH 1/1 taskd: support linking against static gnutls and uuid

Hi Ben,

Quoted 5 lines · Ben Boeckel
Quoted 1 line · Baruch Siach

But you do need to sign-off if you want Buildroot to apply this patch.

Is the signoff on the buildroot-level patch not sufficient? The patch in question here does not modify buildroot code, but taskwarrior code.

Yes. See
http://nightly.buildroot.org/manual.html#_format_and_licensing_of_the_package_patches.

baruch

Signature · 3 lines
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
author
#4 lore raw

PATCH v2 1/1 taskd: support linking against static gnutls and uuid

In reply to Ben Boeckel

The FindGnuTLS module shipped with CMake does not expose the information required for proper linking if it is a static library.

Similarly, the code which searches for the uuid library is insufficient for such a task.

Also, find_package(Threads) was missing, so pthreads was not properly linked.

Do not send this patch to taskwarrior upstream; it is not a proper fix.

Fixes:
http://autobuild.buildroot.net/results/67f574af0cc4348ffe2bce026c44766e49c29124/

Signed-off-by Ben Boeckel <[email protected]>

---
Changes v1 -> v2:
- renamed patch to be 0002- for unambiguous application order
- added SoB to taskd patch (with admonition that it is a hack) (via Baruch)

---
 ...ix-linking-against-deps-statically-on-arc.patch | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 package/taskd/0002-buildroot-fix-linking-against-deps-statically-on-arc.patch
package/taskd/0002-buildroot-fix-linking-against-deps-statically-on-arc.patch new file +53 -0
new file mode 100644
index 0000000..6983f37
@@ -0,0 +1,53 @@
+From b916016eaf3a3bdb555de11610c0b7964d52da96 Mon Sep 17 00:00:00 2001
+From: Ben Boeckel <[email protected]>
+Date: Thu, 16 Jun 2016 21:58:47 -0400
+Subject: [PATCH] buildroot: fix linking against deps statically on arc
+
+Also find the Threads package so that CMAKE_THREAD_LIBS_INIT is actually
+set.
+
+Do not send upstream; it is a hack.
+
+Signed-off-by:: Ben Boeckel <[email protected]>
+---
+ CMakeLists.txt     | 4 ++--
+ src/CMakeLists.txt | 1 +
+ 2 files changed, 3 insertions(+), 2 deletions(-)
+
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 9a049b0..09f3209 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -49,7 +49,7 @@ find_package (GnuTLS REQUIRED)
+ if (GNUTLS_FOUND)
+   set (HAVE_LIBGNUTLS true)
+   set (TASKD_INCLUDE_DIRS ${TASKD_INCLUDE_DIRS} ${GNUTLS_INCLUDE_DIR})
+-  set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${GNUTLS_LIBRARIES})
++  set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${PC_GNUTLS_STATIC_LIBRARIES})
+ endif (GNUTLS_FOUND)
+ 
+ check_function_exists (timegm          HAVE_TIMEGM)
+@@ -68,7 +68,7 @@ else (DARWIN OR FREEBSD OR OPENBSD)
+   find_library (UUID_LIBRARY NAMES uuid)
+   if (UUID_INCLUDE_DIR AND UUID_LIBRARY)
+     set (TASKD_INCLUDE_DIRS ${TASKD_INCLUDE_DIRS} ${UUID_INCLUDE_DIR})
+-    set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${UUID_LIBRARY})
++    set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${UUID_LIBRARY} -lintl)
+     # Look for uuid_unparse_lower
+     set (CMAKE_REQUIRED_INCLUDES  ${CMAKE_REQUIRED_INCLUDES}  ${UUID_INCLUDE_DIR})
+     set (CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES} ${UUID_LIBRARY})
+diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
+index 5820392..243b0f9 100644
+--- a/src/CMakeLists.txt
++++ b/src/CMakeLists.txt
+@@ -35,6 +35,7 @@ add_library (taskd admin.cpp
+                    util.cpp       util.h
+                    wcwidth6.cpp)
+ 
++find_package(Threads REQUIRED)
+ add_executable (taskd_executable taskd.cpp)
+ target_link_libraries (taskd_executable taskd ${TASKD_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT})
+ set_property (TARGET taskd_executable PROPERTY OUTPUT_NAME "taskd")
+-- 
+2.9.0
+
Signature · 1 line
2.9.0
#5 lore raw

PATCH v2 1/1 taskd: support linking against static gnutls and uuid

Hi Ben, all,

Quoted 11 lines · Ben Boeckel

The FindGnuTLS module shipped with CMake does not expose the information required for proper linking if it is a static library.

Similarly, the code which searches for the uuid library is insufficient for such a task.

Also, find_package(Threads) was missing, so pthreads was not properly linked.

Do not send this patch to taskwarrior upstream; it is not a proper fix.

I would have done 2 distinct patches, one for pthread since it may be not-static-build specific (though it could be triggered more easily on static-build), and a second one fixing the missing dependencies for gnutls and uuid in case of static-build.

Quoted 35 lines

Fixes:
http://autobuild.buildroot.net/results/67f574af0cc4348ffe2bce026c44766e49c29124/

Signed-off-by Ben Boeckel <[email protected]>

---
Changes v1 -> v2:
- renamed patch to be 0002- for unambiguous application order
- added SoB to taskd patch (with admonition that it is a hack) (via Baruch)

---
 ...ix-linking-against-deps-statically-on-arc.patch | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)
 create mode 100644 package/taskd/0002-buildroot-fix-linking-against-deps-statically-on-arc.patch
package/taskd/0002-buildroot-fix-linking-against-deps-statically-on-arc.patch new file +16 -0
new file mode 100644
index 0000000..6983f37
@@ -0,0 +1,53 @@
+From b916016eaf3a3bdb555de11610c0b7964d52da96 Mon Sep 17 00:00:00 2001
+From: Ben Boeckel <[email protected]>
+Date: Thu, 16 Jun 2016 21:58:47 -0400
+Subject: [PATCH] buildroot: fix linking against deps statically on arc
+
+Also find the Threads package so that CMAKE_THREAD_LIBS_INIT is actually
+set.
+
+Do not send upstream; it is a hack.
+
+Signed-off-by:: Ben Boeckel <[email protected]>
+---
+ CMakeLists.txt     | 4 ++--
+ src/CMakeLists.txt | 1 +
+ 2 files changed, 3 insertions(+), 2 deletions(-)
+

package/taskd/0002-buildroot-fix-linking-against-deps-statically-on-arc.patch line 26

Quoted 10 lines
package/taskd/0002-buildroot-fix-linking-against-deps-statically-on-arc.patch new file +10 -0
+diff --git a/CMakeLists.txt b/CMakeLists.txt
+index 9a049b0..09f3209 100644
+--- a/CMakeLists.txt
++++ b/CMakeLists.txt
+@@ -49,7 +49,7 @@ find_package (GnuTLS REQUIRED)
+ if (GNUTLS_FOUND)
+   set (HAVE_LIBGNUTLS true)
+   set (TASKD_INCLUDE_DIRS ${TASKD_INCLUDE_DIRS} ${GNUTLS_INCLUDE_DIR})
+-  set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${GNUTLS_LIBRARIES})
++  set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${PC_GNUTLS_STATIC_LIBRARIES})

This fortunately works because FindGnuTLS.cmake provided by CMake use pkg_check_modules on non-windows platform to find GNU-TLS [1], but this is a bit... hackish.
A cleaner way would be to explicitly (re-)call pkg_check_modules in this if-block.

package/taskd/0002-buildroot-fix-linking-against-deps-statically-on-arc.patch line 35

Quoted 9 lines
package/taskd/0002-buildroot-fix-linking-against-deps-statically-on-arc.patch new file +9 -0
+ endif (GNUTLS_FOUND)
+
+ check_function_exists (timegm          HAVE_TIMEGM)
+@@ -68,7 +68,7 @@ else (DARWIN OR FREEBSD OR OPENBSD)
+   find_library (UUID_LIBRARY NAMES uuid)
+   if (UUID_INCLUDE_DIR AND UUID_LIBRARY)
+     set (TASKD_INCLUDE_DIRS ${TASKD_INCLUDE_DIRS} ${UUID_INCLUDE_DIR})
+-    set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${UUID_LIBRARY})
++    set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${UUID_LIBRARY} -lintl)

Here, I think the right fix would be using pkg_check_modules for uuid, then use the _STATIC_LIBRARIES variable to fill TASKD_LIBRARIES.

package/taskd/0002-buildroot-fix-linking-against-deps-statically-on-arc.patch line 47

Quoted 12 lines
package/taskd/0002-buildroot-fix-linking-against-deps-statically-on-arc.patch new file +12 -0
+     # Look for uuid_unparse_lower
+     set (CMAKE_REQUIRED_INCLUDES  ${CMAKE_REQUIRED_INCLUDES}  ${UUID_INCLUDE_DIR})
+     set (CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES} ${UUID_LIBRARY})
+diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
+index 5820392..243b0f9 100644
+--- a/src/CMakeLists.txt
++++ b/src/CMakeLists.txt
+@@ -35,6 +35,7 @@ add_library (taskd admin.cpp
+                    util.cpp       util.h
+                    wcwidth6.cpp)
+
++find_package(Threads REQUIRED)

It seems taskd can not depend on threads [2,3].
So, if Threads is actually mandatory, I'd prefer the REQUIRED flag to be added to [2], otherwise figure out exactly what happens (check with upstream).

Quoted 9 lines
package/taskd/0002-buildroot-fix-linking-against-deps-statically-on-arc.patch new file +6 -1
+ add_executable (taskd_executable taskd.cpp)
+ target_link_libraries (taskd_executable taskd ${TASKD_LIBRARIES} ${CMAKE_THREAD_LIBS_INIT})
+ set_property (TARGET taskd_executable PROPERTY OUTPUT_NAME "taskd")
+--
+2.9.0
+
--

2.9.0

[1] https://github.com/Kitware/CMake/blob/master/Modules/FindGnuTLS.cmake#L42
[2] https://github.com/taskwarrior/taskd/blob/s1.1.0/CMakeLists.txt#L62
[3] https://github.com/taskwarrior/taskd/blob/s1.1.0/src/Thread.h#L31

Regards,

Signature · 1 line
Samuel
author
#6 lore raw

PATCH v2 1/1 taskd: support linking against static gnutls and uuid

[ So it seems that my original message never made it to the list; not
sure why. Resending. ]

Quoted 4 lines · Samuel Martin

I would have done 2 distinct patches, one for pthread since it may be not-static-build specific (though it could be triggered more easily on static-build), and a second one fixing the missing dependencies for gnutls and uuid in case of static-build.

See below.

Quoted 5 lines
Quoted 2 lines
+-  set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${GNUTLS_LIBRARIES})
++  set (TASKD_LIBRARIES    ${TASKD_LIBRARIES}    ${PC_GNUTLS_STATIC_LIBRARIES})

This fortunately works because FindGnuTLS.cmake provided by CMake use pkg_check_modules on non-windows platform to find GNU-TLS [1], but this is a bit... hackish.

Acknowledged :) .

Quoted 2 lines

A cleaner way would be to explicitly (re-)call pkg_check_modules in this if-block.

I need to fix FindGnuTLS upstream (on my list, but not high on it :/ ). There's further problems that even with the pkg_check_modules call, it still won't be fully correct due to full paths in the --libs output:

    https://gitlab.kitware.com/cmake/cmake/issues/16154

FindGnuTLS then needs to tell whether the found library is static or not and set GNUTLS_LIBRARIES using the right variable.

I also have a task here to write a proper FindUUID for taskd (and I suppose the rest of their suite) to use.

Quoted 2 lines

[2] https://github.com/taskwarrior/taskd/blob/s1.1.0/CMakeLists.txt#L62
[3] https://github.com/taskwarrior/taskd/blob/s1.1.0/src/Thread.h#L31

This code has changed in 1.2.0 and is actually gone. I think the threads missing might actually be fallout from gnutls' use of a full pthread path from pkg-config (which CMake misses due to the issue linked above).

--Ben