From owner-freebsd-ports@FreeBSD.ORG Tue Aug 24 15:32:00 2010 Return-Path: Delivered-To: freebsd-ports@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B1A7D1065696 for ; Tue, 24 Aug 2010 15:32:00 +0000 (UTC) (envelope-from jdc@koitsu.dyndns.org) Received: from qmta12.westchester.pa.mail.comcast.net (qmta12.westchester.pa.mail.comcast.net [76.96.59.227]) by mx1.freebsd.org (Postfix) with ESMTP id 5CB188FC17 for ; Tue, 24 Aug 2010 15:31:59 +0000 (UTC) Received: from omta18.westchester.pa.mail.comcast.net ([76.96.62.90]) by qmta12.westchester.pa.mail.comcast.net with comcast id yAuv1e0051wpRvQ5CFY0Tl; Tue, 24 Aug 2010 15:32:00 +0000 Received: from koitsu.dyndns.org ([98.248.41.155]) by omta18.westchester.pa.mail.comcast.net with comcast id yFXy1e00S3LrwQ23eFXzEW; Tue, 24 Aug 2010 15:32:00 +0000 Received: by icarus.home.lan (Postfix, from userid 1000) id 8FD3A9B425; Tue, 24 Aug 2010 08:31:57 -0700 (PDT) Date: Tue, 24 Aug 2010 08:31:57 -0700 From: Jeremy Chadwick To: freebsd-ports@freebsd.org Message-ID: <20100824153157.GA63745@icarus.home.lan> References: <20100817060114.GA86738@icarus.home.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20100817060114.GA86738@icarus.home.lan> User-Agent: Mutt/1.5.20 (2009-06-14) Cc: Garrett Wollman , Emanuel Haupt , Alex Goncharov Subject: Re: shells/bash and the libiconv dependency mess X-BeenThere: freebsd-ports@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting software to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Aug 2010 15:32:00 -0000 On Mon, Aug 16, 2010 at 11:01:14PM -0700, Jeremy Chadwick wrote: > Let me explain what transpired in chronological order: > > On 2010/05/11, ehaupt committed the following patch: > > http://www.freebsd.org/cgi/cvsweb.cgi/ports/shells/bash/files/patch-Makefile.in > > And bumped PORTREVISION (from 0 to 1) in the Makefile. This > unconditionally made bash require libiconv, and the only justification > is "fix statically linked version". > > Those of us who use WITHOUT_NLS or who do not have libiconv already on > their systems (from another port) immediately notice the problem (bash > will no longer build): > > http://www.freebsd.org/cgi/query-pr.cgi?pr=147747 > http://www.freebsd.org/cgi/query-pr.cgi?pr=148329 > http://www.freebsd.org/cgi/query-pr.cgi?pr=149218 > > Three months goes by and finally something is committed to fix the > problem on 2010/08/06. Except the fix doesn't make any sense; all it > does is make libiconv a mandatory dependency (USE_ICONV): > > http://www.freebsd.org/cgi/cvsweb.cgi/ports/shells/bash/Makefile#rev1.123 > > This, of course, means that WITHOUT_NLS is broken and doesn't work as > it's supposed to, since libiconv is now a mandatory requirement (it > doesn't need to be): > > # make WITHOUT_NLS=true all-depends-list > /usr/ports/devel/bison > /usr/ports/converters/libiconv > /usr/ports/devel/m4 > /usr/ports/devel/libtool22 > > Why was this done the way it was? patch-Makefile.in should be removed > and instead replaced with a REINPLACE_CMD that handles the conditionals > (WITH_STATIC_BASIC, WITHOUT_NLS, etc.) in a more clean manner. > > And where are the details of the supposed "statically linked version" > problem? > > Sorry if I sound angry, but this whole situation is a mess, and > shells/bash is a very important port. If someone wants me to put my > money where my mouth is and go + clean it up I'll be happy to. Testing > all the different quirk combinations really isn't that complex. Since I didn't really get any answers regarding this predicament, I went ahead and did the necessary effort. Below are my QA notes. The patch is available here: http://jdc.parodius.com/freebsd/bash-without-nls.patch Please note the required removal of files/patch-Makefile.in. I can file a PR for all this if need be, just let me know. Test phase A ============== System should NOT have libiconv and libintl (gettext) installed. A#1) make WITHOUT_NLS=true - SHOULD NOT: pull in libiconv and gettext as dependencies - SHOULD NOT: have symbols that reference libiconv/libintl (gettext) - SHOULD: REINPLACE_CMD patch Makefile.in and config.h A#2) make WITH_STATIC_BASH=true - internally forces WITHOUT_NLS=yes (so see A#1) - SHOULD: result in a non-dynamic ELF binary of larger size A#3) make - SHOULD: pull in libiconv and gettext as dependencies - SHOULD: link to libiconv and/or libintl (gettext) - SHOULD NOT: REINPLACE_CMD patch Makefile.in and config.h Note: "make WITH_STATIC_BASH=true WITHOUT_NLS=true" is effectively the same as A#2; no point in testing that condition. Test phase B ============== System SHOULD have libiconv and libintl (gettext) pre-installed. B#4) make WITHOUT_NLS=true - SHOULD NOT: pull in libiconv and gettext as dependencies - SHOULD NOT: have symbols that reference libiconv/libintl (gettext) - SHOULD: REINPLACE_CMD patch Makefile.in and config.h B#5) make WITH_STATIC_BASH=true - internally forces WITHOUT_NLS=yes (so see B#4) - SHOULD: result in a non-dynamic ELF binary of larger size B#6) make - SHOULD: make use of libiconv and gettext as dependencies - SHOULD: link to libiconv and/or libintl (gettext) - SHOULD NOT: REINPLACE_CMD patch Makefile.in and config.h Investigative notes ===================== - The bash configure script contains comments that indicate automatically pulling in libiconv and libintl (gettext), with no deactivation mechanism (even if --disable-nls is defined -- yes really) is actually *intentional*. I strongly disagree with this non-minimalist mentality. - There is no easy way (aside from a very big patch in files/) to get the configure script to not detect libiconv.so if it exists on the system. REINPLACE_CMD fixups should be an effective workaround, but folks will still see the detection during the configure phase. Sad panda. - The combination of the post-patch and post-configure addition is what allows libiconv to not be pulled in during the build and link phase. This appears to be the only way to override configure's brain damage. Pre-modifications to shells/bash/Makefile =========================================== - Removed files/patch-Makefile.in - When WITHOUT_NLS **is not** defined, set USE_ICONV=yes - post-patch: remove @LIBICONV@ from in WRKSRC/Makefile.in when WITHOUT_NLS is defined - post-configure: remove #define HAVE_ICONV 1 from WRKSRC/config.h when WITHOUT_NLS is defined Test phase A: pkg_info ======================= bison-2.4.3,1 A parser generator from FSF, (mostly) compatible with Yacc m4-1.4.14_1,1 GNU m4 pcre-8.10 Perl Compatible Regular Expressions library perl-5.10.1_2 Practical Extraction and Report Language portaudit-0.5.15 Checks installed ports against a list of security vulnerabi postfix-2.7.1,1 A secure alternative to widely-used Sendmail rsync-3.0.7 A network file distribution/synchronization utility sudo-1.7.4.3 Allow others to run commands as root vim-lite-7.2.411 Vi "workalike", with many additional features (Lite package A#1 results ============= - Command: make WITHOUT_NLS=true - Results: success ls -l bash -rwxr-xr-x 1 root wheel 878258 Aug 24 05:47 bash size bash text data bss dec hex filename 692571 35280 17456 745307 b5f5b bash # ldd work/bash-4.1/bash work/bash-4.1/bash: libncurses.so.8 => /lib/libncurses.so.8 (0x406e1000) libc.so.7 => /lib/libc.so.7 (0x4082e000) - Command: make clean A#2 results ============= - Command: make WITH_STATIC_BASH=true - Results: success ls -l bash -rwxr-xr-x 1 root wheel 1846158 Aug 24 05:53 bash size bash text data bss dec hex filename 1444688 49972 107456 1602116 187244 bash # ldd work/bash-4.1/bash ldd: work/bash-4.1/bash: not a dynamic ELF executable # objdump --syms work/bash-4.1/bash | egrep -i 'iconv|intl' # - Command: make clean A#3 results ============= - Command: make - Results: DEPENDENCY INSTALL: libtool-2.2.6b - Results: DEPENDENCY INSTALL: libiconv-1.13.1_1 - Results: DEPENDENCY INSTALL: gettext-0.18_1 - Results: success ls -l bash -rwxr-xr-x 1 root wheel 882797 Aug 24 06:11 bash size bash text data bss dec hex filename 696304 35352 17456 749112 b6e38 bash # ldd work/bash-4.1/bash work/bash-4.1/bash: libncurses.so.8 => /lib/libncurses.so.8 (0x406e2000) libintl.so.9 => /usr/local/lib/libintl.so.9 (0x4082f000) libiconv.so.3 => /usr/local/lib/libiconv.so.3 (0x40938000) libc.so.7 => /lib/libc.so.7 (0x40b32000) - Command: make clean Test phase B: pkg_info ======================== bison-2.4.3,1 A parser generator from FSF, (mostly) compatible with Yacc gettext-0.18_1 GNU gettext package libiconv-1.13.1_1 A character set conversion library libtool-2.2.6b Generic shared library support script m4-1.4.14_1,1 GNU m4 pcre-8.10 Perl Compatible Regular Expressions library perl-5.10.1_2 Practical Extraction and Report Language portaudit-0.5.15 Checks installed ports against a list of security vulnerabi postfix-2.7.1,1 A secure alternative to widely-used Sendmail rsync-3.0.7 A network file distribution/synchronization utility sudo-1.7.4.3 Allow others to run commands as root vim-lite-7.2.411 Vi "workalike", with many additional features (Lite package B#4 results ============= - Command: make WITHOUT_NLS=true - Results: success ls -l bash -rwxr-xr-x 1 root wheel 878258 Aug 24 07:22 bash size bash text data bss dec hex filename 692571 35280 17456 745307 b5f5b bash # ldd work/bash-4.1/bash work/bash-4.1/bash: libncurses.so.8 => /lib/libncurses.so.8 (0x406e1000) libc.so.7 => /lib/libc.so.7 (0x4082e000) - Command: make clean B#5 results ============= - Command: make WITH_STATIC_BASH=true - Results: success ls -l bash -rwxr-xr-x 1 root wheel 1846158 Aug 24 07:30 bash size bash text data bss dec hex filename 1444688 49972 107456 1602116 187244 bash # ldd work/bash-4.1/bash ldd: work/bash-4.1/bash: not a dynamic ELF executable # objdump --syms work/bash-4.1/bash | egrep -i 'iconv|intl' # - Command: make clean B#6 results ============= - Command: make - Results: success ls -l bash -rwxr-xr-x 1 root wheel 882797 Aug 24 07:36 bash size bash text data bss dec hex filename 696304 35352 17456 749112 b6e38 bash # ldd work/bash-4.1/bash work/bash-4.1/bash: libncurses.so.8 => /lib/libncurses.so.8 (0x406e2000) libintl.so.9 => /usr/local/lib/libintl.so.9 (0x4082f000) libiconv.so.3 => /usr/local/lib/libiconv.so.3 (0x40938000) libc.so.7 => /lib/libc.so.7 (0x40b32000) - Command: make clean Footnotes =========== Verification of "leftover" files as a result of make install/make deinstall was not performed. I believe itetcu has some sort of automatic framework for detecting this. -- | Jeremy Chadwick jdc@parodius.com | | Parodius Networking http://www.parodius.com/ | | UNIX Systems Administrator Mountain View, CA, USA | | Making life hard for others since 1977. PGP: 4BD6C0CB |