Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 528022 - dev-libs/icu: icu-config may be broken if current /bin/sh != build time /bin/sh or if echo does not support -n
Summary: dev-libs/icu: icu-config may be broken if current /bin/sh != build time /bin/...
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: [OLD] Library (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Gentoo Office Team
URL: http://bugs.icu-project.org/trac/tick...
Whiteboard:
Keywords: PATCH
: 529224 529376 530184 (view as bug list)
Depends on:
Blocks: nonbash 525454 535924
  Show dependency tree
 
Reported: 2014-11-02 15:51 UTC by Alexander Tsoy
Modified: 2015-01-10 15:04 UTC (History)
8 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
Replace 'echo -n'/'echo \c' with printf (icu-remove-bashisms.patch,5.82 KB, patch)
2014-12-29 01:12 UTC, Rémi Cardona
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Tsoy 2014-11-02 15:51:28 UTC
icu-config is an autogenerated script tuned for the shell used by configure at build time. Autoconf-generated configure scripts normally use bash and hence we most often have the following:

# grep ^ECHO_N /usr/bin/icu-config 
ECHO_N="-n"


But if "echo" builtin in the /bin/sh shell behaves differently, then the output is broken. For example /bin/sh -> dash:

<dash-0.5.8.1-r2 (OK):

vm3020 ~ # icu-config --ldflags-icuio
 -licuio 
#

dash-0.5.8.1-r2 ("echo" builtin doesn't support "-n" and "-e" options):

vm3020 ~ # icu-config --ldflags-icuio
-n  -licuio 

#
Comment 1 SpanKY gentoo-dev 2014-11-02 18:19:12 UTC
it should use `printf` everywhere and not use `echo` at all
Comment 2 Alexander Tsoy 2014-11-02 22:30:25 UTC
(In reply to Alexander Tsoy from comment #0)
> icu-config is an autogenerated script tuned for the shell used by configure
> at build time. Autoconf-generated configure scripts normally use bash and
> hence we most often have the following:
> 
> # grep ^ECHO_N /usr/bin/icu-config 
> ECHO_N="-n"

1. The above statement not entirely correct. configure in icu uses custom test for newline option which ignores CONFIG_SHELL and always use /bin/sh. From configure.ac:

# Use custom echo test for newline option
# Current autoconf (2.65) gives incorrect echo newline option
# for icu-config
# This may be removed later - mow (June 17, 2010)
ICU_ECHO_C= ICU_ECHO_N= ICU_ECHO_T=
case `/bin/sh -c "echo -n x"` in
-n*)
  case `/bin/sh -c "echo 'x\c'"` in
  *c*) ICU_ECHO_T=' ';;     # ECHO_T is single tab character.
  *)   ICU_ECHO_C='\c';;
  esac;;
*)
  ICU_ECHO_N='-n';;
esac
AC_SUBST(ICU_ECHO_N)
AC_SUBST(ICU_ECHO_C)
AC_SUBST(ICU_ECHO_T)


2. icu-config is not compatible with dash-0.5.8.1-r2 in any case. It always adds extra newline to the output. For example, it breaks compilation of dev-tex/bibtexu:

# sed -n '216,218p' /var/tmp/portage/dev-tex/bibtexu-3.71_p20120701/work/texlive-20120701-source/texk/bibtexu/Makefile
ICU_LIBS = -L/usr/lib64 
 -licuio  -licui18n -licuuc -licudata 
-lpthread -ldl -lm

So I get the following error:

>>> Compiling source in /var/tmp/portage/dev-tex/bibtexu-3.71_p20120701/work/texlive-20120701-source/texk/bibtexu ...
make -j8 
Makefile:217: *** missing separator.  Stop
Comment 3 Chris Mayo 2014-11-15 16:52:38 UTC
Also causes issue with grub2-mkconfig from sys-boot/grub-2.02_beta2-r2:

# grub2-mkconfig -o grub/grub.cfg
Generating grub configuration file ...
Found linux image: -n
basename: invalid option -- 'n'
Try 'basename --help' for more information.


echo -n used in /etc/grub.d/10_linux:

machine=`uname -m`
case "x$machine" in
    xi?86 | xx86_64)
	list=`for i in /boot/vmlinuz-* /vmlinuz-* /boot/kernel-* ; do
                  if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
              done` ;;
    *) 
	list=`for i in /boot/vmlinuz-* /boot/vmlinux-* /vmlinuz-* /vmlinux-* /boot/kernel-* ; do
                  if grub_file_is_not_garbage "$i" ; then echo -n "$i " ; fi
	     done` ;;
esac


man dash has:

echo [-n ] args...
    Print the arguments on the standard output, separated by spaces. Unless the -n option is present, a newline is output following the arguments.
Comment 4 Alexis Ballier gentoo-dev 2014-11-17 13:49:29 UTC
*** Bug 529224 has been marked as a duplicate of this bug. ***
Comment 5 Alexis Ballier gentoo-dev 2014-11-24 09:38:17 UTC
*** Bug 530184 has been marked as a duplicate of this bug. ***
Comment 6 Alexis Ballier gentoo-dev 2014-12-01 13:21:18 UTC
*** Bug 529376 has been marked as a duplicate of this bug. ***
Comment 7 Joe Sapp (RETIRED) gentoo-dev 2014-12-02 02:07:07 UTC
(In reply to Chris Mayo from comment #3)
> Also causes issue with grub2-mkconfig from sys-boot/grub-2.02_beta2-r2:

Unrelated, I know, but the grub problem should be fixed upstream:
https://savannah.gnu.org/bugs/index.php?43668
Comment 8 Rémi Cardona gentoo-dev 2014-12-27 14:51:42 UTC
Confirming. I'm working on a patch.
Comment 9 Rémi Cardona gentoo-dev 2014-12-29 01:12:59 UTC
Created attachment 392612 [details, diff]
Replace 'echo -n'/'echo \c' with printf

The heart of this patch is in config/icu-config-bottom where I've replaced all uses of echo/ECHO_N/ECHO_C with `printf "%s"`.

The rest of the changes are cleanups since the ECHO_{N,C} variables were only used in icu-config.
Comment 10 Rémi Cardona gentoo-dev 2015-01-03 16:28:58 UTC
May I commit directly?
Comment 11 Andreas K. Hüttel gentoo-dev 2015-01-03 19:39:04 UTC
(In reply to Rémi Cardona from comment #10)
> May I commit directly?

I guess yes, lgtm. Please also try to submit this upstream.
Comment 12 Rémi Cardona gentoo-dev 2015-01-07 07:41:14 UTC
+*icu-53.1-r1 (07 Jan 2015)
+
+  07 Jan 2015; Rémi Cardona <remi@gentoo.org> +icu-53.1-r1.ebuild,
+  +files/icu-remove-bashisms.patch:
+  Add patch to fix icu-config with non-bash /bin/sh, see bug #528022.
+

Revbump because 1) it's not a build failure per se 2) it changes on-disk files 3) -r0 already is already stable on a couple of arches.

FTR, the patch is already submitted in upstream's trac (see URL field) and has been acknowledged as valid, though I can't tell how and when it will be applied.

Thanks
Comment 13 Maxim Britov 2015-01-07 11:59:10 UTC
$ icu-config --ldflags-libsonly --ldflags-system
-licui18n -licuuc -licudata-lpthread -ldl -lm
Comment 14 Maxim Britov 2015-01-07 12:37:01 UTC
"-licudata-lpthread" wrong and fail app-text/texlive-core
Comment 15 Steffen Hau 2015-01-07 13:02:55 UTC
The patch removes the blanks after each Variable, e.g:
-           echo $ECHO_N "${CFLAGS} ${ECHO_C}"
+           printf "%s" "${CFLAGS}"

I've workaround this issue by adding a blank after %s.

"icu-config --ldflags-libsonly --ldflags-system" output is now:
-licui18n -licuuc -licudata -lpthread -ldl -lm
Comment 16 Rémi Cardona gentoo-dev 2015-01-07 23:00:38 UTC
I've added the missing spaces back to the patch (without a revbump, should we do one?) Sorry for breaking things.
Comment 17 Alexis Ballier gentoo-dev 2015-01-08 10:13:18 UTC
(In reply to Rémi Cardona from comment #16)
> (without a revbump, should we do one?)

yes please
Comment 18 Martin Wegner 2015-01-09 17:06:18 UTC
(In reply to Alexis Ballier from comment #17)
> (In reply to Rémi Cardona from comment #16)
> > (without a revbump, should we do one?)
> 
> yes please

Yes, please, otherwise depending bugs such as bug #535924 won't get fixed. Rebuilding icu by hand fixed bug #535924 here for me.
Comment 19 Rémi Cardona gentoo-dev 2015-01-09 21:57:25 UTC
Revbumped.