Patchwork [3/7,v3] package/ncurses: Allow building wide char support

login
register
mail settings
Submitter Jeremy Kerr
Date June 17, 2014, 5:21 a.m.
Message ID <1402982507.184990.682485888591.3.gpush@pablo>
Download mbox | patch
Permalink /patch/360317/
State Superseded
Headers show

Comments

Jeremy Kerr - June 17, 2014, 5:21 a.m.
Allow ncurses to be configured with wide char support; this causes the
libraries to be built with the 'w' suffix (eg libncursesw.so,
libmenuw.so, etc), so we need to create a few symlinks.

Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

---
 package/ncurses/Config.in  |    6 ++++++
 package/ncurses/ncurses.mk |   24 ++++++++++++++++++++++--
 2 files changed, 28 insertions(+), 2 deletions(-)
Gustavo Zacarias - July 4, 2014, 4:58 p.m.
On 06/17/2014 02:21 AM, Jeremy Kerr wrote:

> Allow ncurses to be configured with wide char support; this causes the
> libraries to be built with the 'w' suffix (eg libncursesw.so,
> libmenuw.so, etc), so we need to create a few symlinks.
> 
> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>

Hi.
I'm testing this patch for bug #6950 goodness.
In principle what you do seems fine rather than going the split approach
like it's mentioned in the bug.
However the patch needs a little tweak:

> +ifeq ($(BR2_PACKAGE_NCURSES_WCHAR),y)
> +NCURSES_CONF_OPT += --enable-widec
> +NCURSES_LIB_SUFFIX = w
> +
> +define NCURSES_LINK_LIBS
> +	for lib in $(NCURSES_LIBS-y); do \
> +		ln -sf $${lib}$(NCURSES_LIB_SUFFIX).so \
> +			$(1)/usr/lib/$${lib}.so; \
> +	done
> +endef

You're basically not dealing with the static version of the libraries
here, this breaks at least tmux (it statically links in ncurses), and
i'm sure many other BR2_PREFER_STATIC_LIB=y scenarios.

A big allyespackageconfig run revealed a couple of packages breaking,
namely nano and statserial, i've already cooked patches for them and
send them shortly to the list.

Otherwise it seems all is great.
Thanks.
Regards.
Thomas De Schampheleire - July 16, 2014, 5:04 p.m.
Hi Jeremy, Gustavo,

On Fri, Jul 4, 2014 at 6:58 PM, Gustavo Zacarias
<gustavo@zacarias.com.ar> wrote:
> On 06/17/2014 02:21 AM, Jeremy Kerr wrote:
>
>> Allow ncurses to be configured with wide char support; this causes the
>> libraries to be built with the 'w' suffix (eg libncursesw.so,
>> libmenuw.so, etc), so we need to create a few symlinks.
>>
>> Signed-off-by: Jeremy Kerr <jk@ozlabs.org>
>
> Hi.
> I'm testing this patch for bug #6950 goodness.
> In principle what you do seems fine rather than going the split approach
> like it's mentioned in the bug.
> However the patch needs a little tweak:
>
>> +ifeq ($(BR2_PACKAGE_NCURSES_WCHAR),y)
>> +NCURSES_CONF_OPT += --enable-widec
>> +NCURSES_LIB_SUFFIX = w
>> +
>> +define NCURSES_LINK_LIBS
>> +     for lib in $(NCURSES_LIBS-y); do \
>> +             ln -sf $${lib}$(NCURSES_LIB_SUFFIX).so \
>> +                     $(1)/usr/lib/$${lib}.so; \
>> +     done
>> +endef
>
> You're basically not dealing with the static version of the libraries
> here, this breaks at least tmux (it statically links in ncurses), and
> i'm sure many other BR2_PREFER_STATIC_LIB=y scenarios.
>
> A big allyespackageconfig run revealed a couple of packages breaking,
> namely nano and statserial, i've already cooked patches for them and
> send them shortly to the list.
>
> Otherwise it seems all is great.


Could any of you respin and repost the patch with this comment taken
into account?

Thanks a lot,
Thomas
Jeremy Kerr - July 17, 2014, 12:24 a.m.
Hi Thomas,

> Could any of you respin and repost the patch with this comment taken
> into account?

Sure, will do.

Gustavo - is this breakage simply with BR2_PREFER_STATIC_LIB? or is
there some better way to reproduce this issue, so I can ensure my new
patch fixes this?

Thanks,


Jeremy
Gustavo Zacarias - July 17, 2014, 12:43 a.m.
On 07/16/2014 09:24 PM, Jeremy Kerr wrote:

> Sure, will do.
> 
> Gustavo - is this breakage simply with BR2_PREFER_STATIC_LIB? or is
> there some better way to reproduce this issue, so I can ensure my new
> patch fixes this?
> 
> Thanks,

Hi Jeremy, thanks for getting back.
A quick fix would be:

----
ifeq ($(BR2_PREFER_STATIC_LIB),y)
define NCURSES_LINK_LIBS
        for lib in $(NCURSES_LIBS-y); do \
                ln -sf $${lib}$(NCURSES_LIB_SUFFIX).a \
                        $(1)/usr/lib/$${lib}.a; \
        done
endef
else
define NCURSES_LINK_LIBS
        for lib in $(NCURSES_LIBS-y); do \
                ln -sf $${lib}$(NCURSES_LIB_SUFFIX).so \
                        $(1)/usr/lib/$${lib}.so; \
        done
endef
endif
----

The only scenario that i could fail with static builds is blackfin with
flat targets, the ncurses widechar build breaks in a very odd way, which
i exclude via:

----
config BR2_PACKAGE_NCURSES_WCHAR
        bool "enable wide char support"
        depends on BR2_USE_WCHAR
        # Build broken @ curses.priv.h with bad declarations
        depends on !(BR2_bfin && BR2_BINFMT_FLAT)
        help
          Enable wide char & UTF-8 support in ncurses libraries
----

Unless someone wants to dig deep into that breakage and fix it i guess
it's fine to exclude that combination.
Regards.

Patch

diff --git a/package/ncurses/Config.in b/package/ncurses/Config.in
index e8ab710..b90ec9e 100644
--- a/package/ncurses/Config.in
+++ b/package/ncurses/Config.in
@@ -10,6 +10,12 @@  config BR2_PACKAGE_NCURSES
 
 if BR2_PACKAGE_NCURSES
 
+config BR2_PACKAGE_NCURSES_WCHAR
+	bool "enable wide char support"
+	depends on BR2_USE_WCHAR
+	help
+	  Enable wide char & UTF-8 support in ncurses libraries
+
 config BR2_PACKAGE_NCURSES_TARGET_PANEL
 	bool "ncurses libpanel in target"
 	help
diff --git a/package/ncurses/ncurses.mk b/package/ncurses/ncurses.mk
index 4bba8f1..bd2aac0 100644
--- a/package/ncurses/ncurses.mk
+++ b/package/ncurses/ncurses.mk
@@ -12,7 +12,7 @@  HOST_NCURSES_DEPENDENCIES =
 NCURSES_PROGS = clear infocmp tabs tic toe tput tset
 NCURSES_LICENSE = MIT with advertising clause
 NCURSES_LICENSE_FILES = README
-NCURSES_CONFIG_SCRIPTS = ncurses5-config
+NCURSES_CONFIG_SCRIPTS = ncurses$(NCURSES_LIB_SUFFIX)5-config
 
 NCURSES_CONF_OPT = \
 	$(if $(BR2_PREFER_STATIC_LIB),--without-shared,--with-shared) \
@@ -36,6 +36,24 @@  ifeq ($(BR2_PACKAGE_BUSYBOX),y)
 	NCURSES_DEPENDENCIES += busybox
 endif
 
+ifeq ($(BR2_PACKAGE_NCURSES_WCHAR),y)
+NCURSES_CONF_OPT += --enable-widec
+NCURSES_LIB_SUFFIX = w
+
+define NCURSES_LINK_LIBS
+	for lib in $(NCURSES_LIBS-y); do \
+		ln -sf $${lib}$(NCURSES_LIB_SUFFIX).so \
+			$(1)/usr/lib/$${lib}.so; \
+	done
+endef
+
+NCURSES_LINK_TARGET_LIBS =  $(call NCURSES_LINK_LIBS, $(TARGET_DIR))
+NCURSES_LINK_STAGING_LIBS =  $(call NCURSES_LINK_LIBS, $(STAGING_DIR))
+
+NCURSES_POST_INSTALL_STAGING_HOOKS += NCURSES_LINK_STAGING_LIBS
+
+endif
+
 NCURSES_LIBS-y = libncurses
 NCURSES_LIBS-$(BR2_PACKAGE_NCURSES_TARGET_MENU) += libmenu
 NCURSES_LIBS-$(BR2_PACKAGE_NCURSES_TARGET_PANEL) += libpanel
@@ -56,7 +74,8 @@  endef
 ifneq ($(BR2_PREFER_STATIC_LIB),y)
 define NCURSES_INSTALL_TARGET_LIBS
 	for lib in $(NCURSES_LIBS-y); do \
-		cp -dpf $(NCURSES_DIR)/lib/$${lib}.so* $(TARGET_DIR)/usr/lib/; \
+		cp -dpf $(NCURSES_DIR)/lib/$${lib}$(NCURSES_LIB_SUFFIX).so* \
+			$(TARGET_DIR)/usr/lib/; \
 	done
 endef
 endif
@@ -74,6 +93,7 @@  endif
 define NCURSES_INSTALL_TARGET_CMDS
 	mkdir -p $(TARGET_DIR)/usr/lib
 	$(NCURSES_INSTALL_TARGET_LIBS)
+	$(NCURSES_LINK_TARGET_LIBS)
 	$(NCURSES_INSTALL_TARGET_PROGS)
 	ln -snf /usr/share/terminfo $(TARGET_DIR)/usr/lib/terminfo
 	mkdir -p $(TARGET_DIR)/usr/share/terminfo/x