[bug-gnu-libiconv] Re: pkgsrc patches for libiconv-1.11

[Thomas Klausner]

On Tue, Mar 20, 2007 at 03:00:19AM +0100, Bruno Haible wrote:
> > patch-aa: commit message was:
> > revision 1.6
> > date: 2003/09/16 13:57:19;  author: xtraeme;  state: Exp;  lines: +12 -22
> > Don't try to link to -liconv in the install process, it fixes build in
> > FreeBSD. Closes my own PR pkg/22346.
> 
> > $(LIBTOOL_LINK) $(CC) $(LDFLAGS) ... ../lib/libiconv.la ...
> 
> It is my understanding that when you link with an uninstalled libtool
> library, on many platforms, a wrapper script is created in place of a
> real executable. This wrapper script sets LD_LIBRARY_PATH and then execs
> the .libs/lt-iconv. It would be wrong to install this script in a public
> location.

Hm. Actually, I'm only used to libtool being used like:
        $(LIBTOOL_LINK) ... ../lib/libiconv.la ... -o $(BUILD_DIR_VAR)/iconv
        $(LIBTOOL_INSTALL) $(BUILD_DIR_VAR)/iconv $(DESTDIR)/$(PREFIX)/bin
or similar, i.e. separating the two steps of linking and installing.
Is the way you use libtool here documented to work?

> > You can find the PR at
> >     http://www.NetBSD.org/cgi-bin/query-pr-single.pl?number=22346
> 
> Doesn't apply to libiconv-1.9.2 and newer.

That was just the original PR for the above, it's just as additional
information for the previous one.

> > patch-ab:
> > revision 1.8
> > date: 2004/04/02 19:08:37;  author: jschauma;  state: Exp;  lines: +3 -3
> > Apparently cp(1) under Irix does not like to overwrite symbolic links.
> > This causes the package to fail to install, as this section is executed
> > a second time.
> > Use pax instead and all's peachy.
> 
> pax is not portable yet (although it's mandated by POSIX).
> 
> This Makefile rule should only be executed once, since it creates
> lib/localcharset.h.
> 
> Furthermore, the only symlinks are in the .libs directory, and 'libtool',
> not 'cp', is used to copy the libraries from .libs. So if there's a problem
> there, please report a bug in GNU libtool.

I investigated further, this is just the latest in a series of fixes for
the original problem that was noted in PR 11893:
        http://www.NetBSD.org/cgi-bin/query-pr-single.pl?number=11893
It seems that the target is run during build time, but install wants to change
permissions of the file to root/wheel (or so) which breaks the build as 
non-root.

The series of fixes was:
revision 1.1
date: 2001/01/11 10:56:34;  author: rh;  state: Exp;
Make this build without root permissions.  Fixes Luke's PR 11893.
revision 1.5
date: 2002/11/26 00:51:02;  author: grant;  state: Exp;  lines: +6 -6
don't unnecessarily pass -p to `cp', it causes a fatal error on Darwin
while trying to chown a symlink.
revision 1.7
date: 2004/03/02 05:01:38;  author: snj;  state: Exp;  lines: +3 -3
Use ${CP} instead of cp.  From Georg Schwarz in PR pkg/24634.
revision 1.8
date: 2004/04/02 19:08:37;  author: jschauma;  state: Exp;  lines: +3 -3
Apparently cp(1) under Irix does not like to overwrite symbolic links.
This causes the package to fail to install, as this section is executed
a second time.
Use pax instead and all's peachy.

So it seems some non-'make install' way to copy the files to where
libiconv needs should be used.

> > patch-ae:
> > revision 1.4
> > date: 2004/04/28 15:19:04;  author: tv;  state: Exp;  lines: +7 -5
> > nb2: use $(INSTALL), not $(INSTALL_DATA), for a library in an 
> > autoconfiscated
> > package.
> 
> On most platforms, a library does not need a+x permissions. On those where
> it does (such as HP-UX), libtool takes care of it.

The problem is that INSTALL_DATA has an -m 644 on its command line, and it
seems that libtool didn't fix that. Otherwise I can't explain this change :)

> > revision 1.5
> > date: 2006/11/14 13:55:34;  author: joerg;  state: Exp;  lines: +17 -8
> > DESTDIR support. Use modular Xorg for wv.
> 
> 'install-lib' is not a public target. Only other Makefiles in libiconv use it,
> and never with DESTDIR. So the patch is pointless.

Hm, I investigated further: the package's install target (in pkgsrc)
calls install-charset-alias to install the charset.alias, the target
is:
        # be sure to install a new charset.alias
        post-install:
                cd ${WRKSRC}/libcharset/lib && ${MAKE} libdir=${EGDIR} 
DESTDIR=${DESTDIR:Q} \
                        install-charset-alias
The EGDIR part is because pkgsrc handles this as a user-configuration file,
i.e. keeps a copy in EGDIR, installs it to ${PREFIX}/lib and removes it on
deinstallation if it's still the same as ${EGDIR}/charset.alias.
This patch was added in 2003, so perhaps it's not even necessary any longer.
Did the charset.alias installation logic change since then?

> > patch-af:
> > revision 1.1
> > date: 2004/04/28 15:19:04;  author: tv;  state: Exp;
> > nb2: use $(INSTALL), not $(INSTALL_DATA), for a library in an 
> > autoconfiscated
> > package.
> 
> See above.

Yep, see above :)

> Thanks for taking care to synchronize "downstream" and "upstream" versions of
> a package.

You're welcome. I find it annoying that many patches sit in pkgsrc
for years, so I'm trying to get rid of them now when I touch packages.

Thanks,
 Thomas