Skip to contentSkip to conversation

Fix udftools in static linking scenarios

PATCH New Thomas Petazzoni 3 participants 10 messages Apr 22, 2018 – May 6, 2018

lore
Share & tools
lore
#0 lore raw

cover Fix udftools in static linking scenarios

Hello,

This set of 3 patches aim at fixing the build of udftools in static linking scenarios.

The first patch is a somewhat unrelated fix, which is really trivial, and not specific to static linking.

The second patch fixes the readline package to install a pkg-config file.

The third patch fixes the udftools package to use the readline pkg-config file.

The overall solution is I believe the most correct one (use a pkg-config file), but is admittedly a bit complex and requires to autoreconf two more packages. If people don't like it, I'll be happy to change it to just pass "-lncurses" when building udftools, which would achieve the same effect.

Thanks,

Thomas

Thomas Petazzoni (3):
udftool: add missing dependency on readline
readline: install a .pc file
udftools: fix static linking against readline

 ...c-readline.pc.in-fix-Requires.private-val.patch | 62 ++++++++++++++++++++++
 package/readline/readline.mk                       | 17 +++++-
 ...c-detect-readline-via-pkg-config-when-pos.patch | 60 +++++++++++++++++++++
 package/udftools/udftools.mk                       |  2 +
 4 files changed, 139 insertions(+), 2 deletions(-)
 create mode 100644 package/readline/0001-configure.ac-readline.pc.in-fix-Requires.private-val.patch
 create mode 100644 package/udftools/0002-configure.ac-detect-readline-via-pkg-config-when-pos.patch
Signature · 1 line
2.14.3
#1 lore raw

PATCH 1/3 udftool: add missing dependency on readline

udftool/Config.in selects BR2_PACKAGE_READLINE, but udftool does not depend on it. Readline is not a runtime dependency: one of the udftool program links against it.

This commit fixes this inconsistency.

---
 package/udftools/udftools.mk | 1 +
 1 file changed, 1 insertion(+)
package/udftools/udftools.mk +1 -0
index 708fef9ab7..2d3477a51c 100644
@@ -8,5 +8,6 @@ UDFTOOLS_VERSION = 2.0
 UDFTOOLS_SITE = https://github.com/pali/udftools/releases/download/$(UDFTOOLS_VERSION)
 UDFTOOLS_LICENSE = GPL-2.0+
 UDFTOOLS_LICENSE_FILES = COPYING
+UDFTOOLS_DEPENDENCIES = readline
 
 $(eval $(autotools-package))
Signature · 1 line
2.14.3
#2 lore raw

PATCH 1/3 udftool: add missing dependency on readline

Quoted 9 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

udftool/Config.in selects BR2_PACKAGE_READLINE, but udftool does not depend on it. Readline is not a runtime dependency: one of the udftool program links against it.

This commit fixes this inconsistency.

Committed, thanks.

Signature · 1 line
Bye, Peter Korsgaard
#3 lore raw

PATCH 2/3 readline: install a .pc file

In reply to Thomas Petazzoni

Some packages (such as udftools) fail to build against readline in static linking configurations. To fix this, we install the .pc file provided by readline.

Unfortunately, this .pc file is by default not correct, so a patch is added to fix it, and this patch requires to re-autoconf the package (but not autoreconf since only autoconf is used).

---
 ...c-readline.pc.in-fix-Requires.private-val.patch | 62 ++++++++++++++++++++++
 package/readline/readline.mk                       | 17 +++++-
 2 files changed, 77 insertions(+), 2 deletions(-)
 create mode 100644 package/readline/0001-configure.ac-readline.pc.in-fix-Requires.private-val.patch
package/readline/0001-configure.ac-readline.pc.in-fix-Requires.private-val.patch new file +62 -0
new file mode 100644
index 0000000000..22b479cd91
@@ -0,0 +1,62 @@
+From 7bbf2046fbcf4416ec226ecb3a2b4c6a5c263298 Mon Sep 17 00:00:00 2001
+From: Thomas Petazzoni <[email protected]>
+Date: Sun, 22 Apr 2018 23:06:28 +0200
+Subject: [PATCH] configure.ac, readline.pc.in: fix Requires.private value
+
+The provided readline.pc.in hardcodes the fact that readline depends
+on the tinfo library. However, different termcap implementation are
+supported beyond tinfo. This commit improves the configure.ac script
+to define a TERMCAP_PKG_CONFIG_LIB variable, which is then used in
+readline.pc.in.
+
+This for example allows the generated readline.pc to properly contain
+"Requires.private: ncurses" when ncurses is used as the termcap
+implementation.
+
+Signed-off-by: Thomas Petazzoni <[email protected]>
+---
+ configure.ac   | 8 ++++++++
+ readline.pc.in | 2 +-
+ 2 files changed, 9 insertions(+), 1 deletion(-)
+
+diff --git a/configure.ac b/configure.ac
+index fd0cec4..9f85f37 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -276,6 +276,13 @@ case "$BUILD_DIR" in
+ *)	;;
+ esac
+ 
++case "$TERMCAP_LIB" in
++-ltinfo)  TERMCAP_PKG_CONFIG_LIB=tinfo ;;
++-lcurses) TERMCAP_PKG_CONFIG_LIB=ncurses ;;
++-lncurses) TERMCAP_PKG_CONFIG_LIB=ncurses ;;
++-ltermcap) TERMCAP_PKG_CONFIG_LIB=termcap ;;
++esac
++
+ AC_SUBST(BUILD_DIR)
+ 
+ AC_SUBST(CFLAGS)
+@@ -292,6 +299,7 @@ AC_SUBST(host_os)
+ AC_SUBST(LIBVERSION)
+ 
+ AC_SUBST(TERMCAP_LIB)
++AC_SUBST(TERMCAP_PKG_CONFIG_LIB)
+ 
+ AC_OUTPUT([Makefile doc/Makefile examples/Makefile shlib/Makefile readline.pc],
+ [
+diff --git a/readline.pc.in b/readline.pc.in
+index fbfca8a..a7f2cf3 100644
+--- a/readline.pc.in
++++ b/readline.pc.in
+@@ -7,6 +7,6 @@ Name: Readline
+ Description: Gnu Readline library for command line editing
+ URL: http://tiswww.cwru.edu/php/chet/readline/rltop.html
+ Version: @LIBVERSION@
+-Requires.private: tinfo
++Requires.private: @TERMCAP_PKG_CONFIG_LIB@
+ Libs: -L${libdir} -lreadline
+ Cflags: -I${includedir}/readline
+-- 
+2.14.3
+
package/readline/readline.mk +15 -2
index cc5d4f2920..e00c594697 100644
@@ -7,18 +7,31 @@
 READLINE_VERSION = 7.0
 READLINE_SITE = $(BR2_GNU_MIRROR)/readline
 READLINE_INSTALL_STAGING = YES
-READLINE_DEPENDENCIES = ncurses
-HOST_READLINE_DEPENDENCIES = host-ncurses
+READLINE_DEPENDENCIES = ncurses host-autoconf
+HOST_READLINE_DEPENDENCIES = host-ncurses host-autoconf
 READLINE_CONF_ENV = bash_cv_func_sigsetjmp=yes \
 	bash_cv_wcwidth_broken=no
 READLINE_LICENSE = GPL-3.0+
 READLINE_LICENSE_FILES = COPYING
 
+# readline only uses autoconf, not automake, and therefore the regular
+# AUTORECONF = YES doesn't work.
+define READLINE_AUTOCONF
+	cd $(@D); $(HOST_DIR)/bin/autoconf
+endef
+READLINE_PRE_CONFIGURE_HOOKS += READLINE_AUTOCONF
+HOST_READLINE_PRE_CONFIGURE_HOOKS += READLINE_AUTOCONF
+
 define READLINE_PURGE_EXAMPLES
 	rm -rf $(TARGET_DIR)/usr/share/readline
 endef
 READLINE_POST_INSTALL_TARGET_HOOKS += READLINE_PURGE_EXAMPLES
 
+define READLINE_INSTALL_PC_FILE
+	$(INSTALL) -D -m 644 $(@D)/readline.pc $(STAGING_DIR)/usr/lib/pkgconfig/readline.pc
+endef
+READLINE_POST_INSTALL_STAGING_HOOKS += READLINE_INSTALL_PC_FILE
+
 define READLINE_INSTALL_INPUTRC
 	$(INSTALL) -D -m 644 package/readline/inputrc $(TARGET_DIR)/etc/inputrc
 endef
Signature · 1 line
2.14.3
#4 lore raw

PATCH 2/3 readline: install a .pc file

Hello,

Quoted 12 lines · Thomas Petazzoni
package/readline/0001-configure.ac-readline.pc.in-fix-Requires.private-val.patch new file +6 -0
new file mode 100644
index 0000000000..22b479cd91
@@ -0,0 +1,62 @@
+From 7bbf2046fbcf4416ec226ecb3a2b4c6a5c263298 Mon Sep 17 00:00:00 2001
+From: Thomas Petazzoni <[email protected]>
+Date: Sun, 22 Apr 2018 23:06:28 +0200
+Subject: [PATCH] configure.ac, readline.pc.in: fix Requires.private value
+
+The provided readline.pc.in hardcodes the fact that readline depends

package/readline/0001-configure.ac-readline.pc.in-fix-Requires.private-val.patch line 16

Quoted 10 lines
package/readline/0001-configure.ac-readline.pc.in-fix-Requires.private-val.patch new file +10 -0
+on the tinfo library. However, different termcap implementation are
+supported beyond tinfo. This commit improves the configure.ac script
+to define a TERMCAP_PKG_CONFIG_LIB variable, which is then used in
+readline.pc.in.
+
+This for example allows the generated readline.pc to properly contain
+"Requires.private: ncurses" when ncurses is used as the termcap
+implementation.
+
+Signed-off-by: Thomas Petazzoni <[email protected]>

For information, this patch has been applied upstream by the readline maintainer. See
http://git.savannah.gnu.org/cgit/readline.git/commit/?h=devel&id=39cef351237a0d4467d3dc62a7710ea5e619b7e1.
(Yes it's one of those project that use Git in a very weird way).

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
#5 lore raw

PATCH 2/3 readline: install a .pc file

In reply to Thomas Petazzoni
Quoted 11 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

Some packages (such as udftools) fail to build against readline in static linking configurations. To fix this, we install the .pc file provided by readline.

Unfortunately, this .pc file is by default not correct, so a patch is added to fix it, and this patch requires to re-autoconf the package (but not autoreconf since only autoconf is used).

Committed, thanks.

Signature · 1 line
Bye, Peter Korsgaard
#6 lore raw

PATCH 3/3 udftools: fix static linking against readline

In reply to Thomas Petazzoni

One program of udftools uses the readline library, but fails to build in static linking configurations. In order to fix this, we teach udftools configure.ac to use pkg-config to detect the readline library, and to use the proper flags provided by pkg-config.

This obviously requires an autoreconf of the package, and the addition of host-pkgconf in the dependencies.

Fixes:

http://autobuild.buildroot.net/results/113a94049d89b8f065112e5d4482667a7b7fb843/

---
 ...c-detect-readline-via-pkg-config-when-pos.patch | 60 ++++++++++++++++++++++
 package/udftools/udftools.mk                       |  3 +-
 2 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 package/udftools/0002-configure.ac-detect-readline-via-pkg-config-when-pos.patch
package/udftools/0002-configure.ac-detect-readline-via-pkg-config-when-pos.patch new file +60 -0
new file mode 100644
index 0000000000..f73db99ca2
@@ -0,0 +1,60 @@
+From bdacf0101fea1dad2c89996b27cb4b9caee9109c Mon Sep 17 00:00:00 2001
+From: Thomas Petazzoni <[email protected]>
+Date: Sun, 22 Apr 2018 22:28:09 +0200
+Subject: [PATCH] configure.ac: detect readline via pkg-config when possible
+
+pkg-config automatically handles static linking situations, where for
+example readline is linked against ncurses, and therefore -lncurses
+needs to be passed in addition to -lreadline.
+
+This proposal uses pkg-config when available. If pkg-config is not
+found, or readline is not found via pkg-config, we fallback to the
+existing AC_CHECK_LIB(). This AC_CHECK_LIB() test is modified to set
+READLINE_LIBS, like PKG_CHECK_MODULES() does. The Makefile.am
+consequently uses READLINE_LIBS instead of hardcoding -lreadline.
+
+Signed-off-by: Thomas Petazzoni <[email protected]>
+---
+ configure.ac      | 14 ++++++++++++--
+ wrudf/Makefile.am |  2 +-
+ 2 files changed, 13 insertions(+), 3 deletions(-)
+
+diff --git a/configure.ac b/configure.ac
+index 95fbba3..62b1caa 100644
+--- a/configure.ac
++++ b/configure.ac
+@@ -9,8 +9,18 @@ AC_PROG_CC
+ AC_DISABLE_SHARED
+ AM_PROG_LIBTOOL
+ 
+-dnl Checks for libraries.
+-AC_CHECK_LIB(readline, readline, [ ], AC_MSG_ERROR([cannot find -lreadline.]))
++PKG_PROG_PKG_CONFIG
++
++dnl Checks for libraries, by using pkg-config when available
++if test -n "${PKG_CONFIG}" ; then
++  PKG_CHECK_MODULES([READLINE], [readline], [readline_found=yes], [readline_found=no])
++fi
++
++if test "${readline_found}" != "yes" ; then
++  AC_CHECK_LIB(readline, readline,
++               [AC_SUBST([READLINE_LIBS], [-lreadline])],
++               AC_MSG_ERROR([cannot find -lreadline.]))
++fi
+ 
+ dnl Checks for header files.
+ AC_HEADER_STDC
+diff --git a/wrudf/Makefile.am b/wrudf/Makefile.am
+index fe1c269..e3ab85b 100644
+--- a/wrudf/Makefile.am
++++ b/wrudf/Makefile.am
+@@ -1,5 +1,5 @@
+ bin_PROGRAMS = wrudf
+-wrudf_LDADD = $(top_builddir)/libudffs/libudffs.la -lreadline
++wrudf_LDADD = $(top_builddir)/libudffs/libudffs.la $(READLINE_LIBS)
+ wrudf_SOURCES = wrudf.c wrudf-cmnd.c wrudf-desc.c wrudf-cdrw.c wrudf-cdr.c ide-pc.c wrudf.h ide-pc.h ../include/ecma_167.h ../include/osta_udf.h ../include/bswap.h
+ 
+ AM_CPPFLAGS = -I$(top_srcdir)/include -D_GNU_SOURCE -DDEBUG
+-- 
+2.14.3
+
package/udftools/udftools.mk +2 -1
index 2d3477a51c..e5bf59137e 100644
@@ -8,6 +8,7 @@ UDFTOOLS_VERSION = 2.0
 UDFTOOLS_SITE = https://github.com/pali/udftools/releases/download/$(UDFTOOLS_VERSION)
 UDFTOOLS_LICENSE = GPL-2.0+
 UDFTOOLS_LICENSE_FILES = COPYING
-UDFTOOLS_DEPENDENCIES = readline
+UDFTOOLS_AUTORECONF = YES
+UDFTOOLS_DEPENDENCIES = readline host-pkgconf
 
 $(eval $(autotools-package))
Signature · 1 line
2.14.3
#7 lore raw

PATCH 3/3 udftools: fix static linking against readline

Quoted 7 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Thomas" == Thomas Petazzoni <[email protected]> writes:

One program of udftools uses the readline library, but fails to build in static linking configurations. In order to fix this, we teach udftools configure.ac to use pkg-config to detect the readline library, and to use the proper flags provided by pkg-config.

Quoted 8 lines

This obviously requires an autoreconf of the package, and the addition of host-pkgconf in the dependencies.

Fixes:

http://autobuild.buildroot.net/results/113a94049d89b8f065112e5d4482667a7b7fb843/

Committed, thanks.

Signature · 1 line
Bye, Peter Korsgaard
#8 lore raw

cover Fix udftools in static linking scenarios

In reply to Thomas Petazzoni
Quoted 11 lines · Thomas Petazzoni

Hello,

This set of 3 patches aim at fixing the build of udftools in static linking scenarios.

The first patch is a somewhat unrelated fix, which is really trivial, and not specific to static linking.

The second patch fixes the readline package to install a pkg-config file.

Quoted 8 lines

The third patch fixes the udftools package to use the readline pkg-config file.

The overall solution is I believe the most correct one (use a pkg-config file), but is admittedly a bit complex and requires to autoreconf two more packages. If people don't like it, I'll be happy to change it to just pass "-lncurses" when building udftools, which would achieve the same effect.

Using pkg-config file is indeed a correct fix. But I think we should understand why readline doesn't install .pc file by default. This might be a reason that it is outdated and incorrect. Yes, in past we have fixed static linking with readline for many packages by passing it's private libraries.

May be if we first upstream readline changes with .pc installation in autotools files, it would be nice to use fix. Otherwise, we need to maintain patch and .pc file updated (in case of any changes in list of private libraries).

Thanks,
Rahul

Quoted 25 lines

Thanks,

Thomas

Thomas Petazzoni (3):
udftool: add missing dependency on readline
readline: install a .pc file
udftools: fix static linking against readline

 ...c-readline.pc.in-fix-Requires.private-val.patch | 62 ++++++++++++++++++++++
 package/readline/readline.mk                       | 17 +++++-
 ...c-detect-readline-via-pkg-config-when-pos.patch | 60 +++++++++++++++++++++
 package/udftools/udftools.mk                       |  2 +
 4 files changed, 139 insertions(+), 2 deletions(-)
 create mode 100644 package/readline/0001-configure.ac-readline.pc.in-fix-Requires.private-val.patch
 create mode 100644 package/udftools/0002-configure.ac-detect-readline-via-pkg-config-when-pos.patch

--
2.14.3

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

#9 lore raw

cover Fix udftools in static linking scenarios

Hello,

Quoted 10 lines · Rahul Bedarkar

Using pkg-config file is indeed a correct fix. But I think we should understand why readline doesn't install .pc file by default. This might be a reason that it is outdated and incorrect. Yes, in past we have fixed static linking with readline for many packages by passing it's private libraries.

May be if we first upstream readline changes with .pc installation in autotools files, it would be nice to use fix. Otherwise, we need to maintain patch and .pc file updated (in case of any changes in list of private libraries).

My readline patch fixing the .pc file contents was merged upstream:

https://lists.gnu.org/archive/html/bug-readline/2018-05/msg00000.html

And I've submitted another patch that does install the .pc file:

https://lists.gnu.org/archive/html/bug-readline/2018-05/msg00002.html

Best regards,

Thomas

Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com