Bug#807289: pkg-config-crosswrapper does not set up PKG_CONFIG_LIBDIR during package cross builds

2016-01-05 Thread Tollef Fog Heen
]] Helmut Grohne 

Hi,

> On Mon, Jan 04, 2016 at 10:13:22PM +0100, Tollef Fog Heen wrote:
> > Right, this is the same as doko reported in #807946.  Do you have any
> > concerns about his patch there or can I use that instead of your much
> > more comprehensive one?
> 
> I confirm that doko filed a duplicate. His patch will practically solve
> the issue in most cases, but is technically wrong. pkg-config's
> architecture is not necessarily the same as DEB_BUILD_ARCH (but it will
> be in many cases). Nothing prevents you from installing pkg-config:i386
> on an amd64 system and then x86_64-linux-gnu-pkg-config will be broken.

Right, that's a good point.

An easier way here seems to be to either ask dpkg what architecture
pkg-config has (dpkg -s pkg-config | awk '$1 == "Architecture:" { print
$2 }') or teach pkg-config to report its own architecture.  The latter
is a bit more work, but quite doable, the former seems quite easy, and I
think, pretty robust?

Cheers,
-- 
Tollef Fog Heen
UNIX is user friendly, it's just picky about who its friends are



Bug#807289: pkg-config-crosswrapper does not set up PKG_CONFIG_LIBDIR during package cross builds

2016-01-05 Thread Helmut Grohne
Hi Tollef,

On Tue, Jan 05, 2016 at 09:16:17AM +0100, Tollef Fog Heen wrote:
> An easier way here seems to be to either ask dpkg what architecture
> pkg-config has (dpkg -s pkg-config | awk '$1 == "Architecture:" { print
> $2 }') or teach pkg-config to report its own architecture.  The latter
> is a bit more work, but quite doable, the former seems quite easy, and I
> think, pretty robust?

I respectfully disagree with that assessment. Running dpkg and awk in
the cross wrapper adds considerable complexity at runtime. A failure in
this mode of operation will be very hard to diagnose whereas a failure
to produce a cross wrapper that knows pkg-config's architecture is easy
to diagnose (by looking at the generated file). Can we please not add
this complexity to the runtime?

As a compromise, we could move such parsing into the dpkg hook, where it
still could break at installation time, but could not incur weired
failure modes at runtime. We need to be careful here as pkg-config is
usually not configured when the dpkg hook is run.

Given that you can immediately see when something went wrong by looking
at the binary package with my patch applied, I believe that it is more
robust than both aforementioned approaches. Yet, if robustness is not
the aim, we can go another route.

I was initially optimizing for two goals: technical correctness and
keeping the run time complexity low.  What are your goals?

Helmut



Bug#807289: pkg-config-crosswrapper does not set up PKG_CONFIG_LIBDIR during package cross builds

2016-01-04 Thread Tollef Fog Heen
]] Helmut Grohne 

Hi Helmut,

thanks for the prod on IRC.

> What's wrong with the wrapper? Consider I installed pkg-config:amd64 and
> am now running i586-linux-gnu-pkg-config in a package build. The wrapper
> correctly determines the shell variable
> basename=i586-linux-gnu-pkg-config and derives multiarch=i386-linux-gnu.
> It now determines the architecture of pkg-config using the following
> (erroneous) line:
> 
> native_multiarch="`dpkg-architecture -qDEB_HOST_MULTIARCH 2>/dev/null`"
> 
> Since we are in a package build, dpkg has set up all those
> DEB_{BUILD,HOST,TARGET}_* variables and since we are building for i386,
> native_multiarch will wrongly end up as i386-linux-gnu as well. Thus the
> cross wrapper skips the setup of PKG_CONFIG_LIBDIR entirely and passes
> on to pkg-config with no environment modification.

Right, this is the same as doko reported in #807946.  Do you have any
concerns about his patch there or can I use that instead of your much
more comprehensive one?

-- 
Tollef Fog Heen
UNIX is user friendly, it's just picky about who its friends are



Bug#807289: pkg-config-crosswrapper does not set up PKG_CONFIG_LIBDIR during package cross builds

2016-01-04 Thread Helmut Grohne
Hi Tollef,

Thanks for looking into this.

On Mon, Jan 04, 2016 at 10:13:22PM +0100, Tollef Fog Heen wrote:
> Right, this is the same as doko reported in #807946.  Do you have any
> concerns about his patch there or can I use that instead of your much
> more comprehensive one?

I confirm that doko filed a duplicate. His patch will practically solve
the issue in most cases, but is technically wrong. pkg-config's
architecture is not necessarily the same as DEB_BUILD_ARCH (but it will
be in many cases). Nothing prevents you from installing pkg-config:i386
on an amd64 system and then x86_64-linux-gnu-pkg-config will be broken.

You also notice that the complexity of my patch stems precisely from
doing this correctly. Rather than assuming that pkg-config's
architecture is the same as DEB_BUILD_ARCH, the architecture is written
into the script. This in turn makes it architecture dependent and
mandates a move to /usr/lib causing compatibility code in the dpkg-hook.

So if you dislike this approach, we can try to go Simon McVittie's
route: Simply don't point the symlink for pkg-config's architecture at
the cross wrapper, but at pkg-config directly. There are two ways to
implement this. One is to write pkg-config's architecture into the
dpkg-hook (essentially moving the complexity). The other is to ship it
as a file in the package (knowing that the dpkg-hook won't touch
symlinks that don't point to the cross wrapper). I believe the latter to
be fragile, because
 1) Other packages (crossbuild-essential-*) mess with this symlink as
pkg-config failed to do so for a long time. Think: file conflicts.
 2) preinst needs to remove any cross wrapper symlinks for pkg-config's
architecture adding the complexity there.
 3) There is a chance that the dpkg hook is run in between preinst and
unpack.

So given all of this, I believe that my patch has the least complexity
for doing the right thing.

Helmut



Bug#807289: pkg-config-crosswrapper does not set up PKG_CONFIG_LIBDIR during package cross builds

2015-12-06 Thread Helmut Grohne
Package: pkg-config
Version: 0.29-2
Tags: patch
User: helm...@debian.org
Usertags: rebootstrap

Hi Tollef,

I am sorry to tell you that the cross building story goes on. I confirm
that with your recent uploads the per-arch wrapper symlinks are
correctly created in all cases observed by me. At the same time, I
conclude that they do not improve package builds at all, because the
wrapper is broken. I have no clue how this could have slipped so long
except by nobody ever using the wrapper. I didn't use it.

What's wrong with the wrapper? Consider I installed pkg-config:amd64 and
am now running i586-linux-gnu-pkg-config in a package build. The wrapper
correctly determines the shell variable
basename=i586-linux-gnu-pkg-config and derives multiarch=i386-linux-gnu.
It now determines the architecture of pkg-config using the following
(erroneous) line:

native_multiarch="`dpkg-architecture -qDEB_HOST_MULTIARCH 2>/dev/null`"

Since we are in a package build, dpkg has set up all those
DEB_{BUILD,HOST,TARGET}_* variables and since we are building for i386,
native_multiarch will wrongly end up as i386-linux-gnu as well. Thus the
cross wrapper skips the setup of PKG_CONFIG_LIBDIR entirely and passes
on to pkg-config with no environment modification.

Of course this issue only applies when you are building under dpkg or
when you install pkg-config for a foreign architecture.

>From my pov, the only correct solution is to move the computation of
native_multiarch to the build time of pkg-config as it is only known
there. This is what the attached patch does. What do you think about it?

Helmut
diff -u pkg-config-0.29/debian/changelog pkg-config-0.29/debian/changelog
--- pkg-config-0.29/debian/changelog
+++ pkg-config-0.29/debian/changelog
@@ -1,3 +1,12 @@
+pkg-config (0.29-2.1) UNRELEASED; urgency=medium
+
+  * Non-maintainer upload.
+  * Fix package cross builds using pkg-config. Closes: #-1
++ Detect native architecture at build time.
++ Move pkg-config-crosswrapper to /usr/lib as it is now arch dependent.
+
+ -- Helmut Grohne   Sun, 06 Dec 2015 00:06:06 +0100
+
 pkg-config (0.29-2) unstable; urgency=medium
 
   * Fix typo in crosswrapper.
reverted:
--- pkg-config-0.29/debian/pkg-config-crosswrapper
+++ pkg-config-0.29.orig/debian/pkg-config-crosswrapper
@@ -1,44 +0,0 @@
-#! /bin/sh
-# pkg-config wrapper for cross-building
-# Sets pkg-config search path to search multiarch and historical 
cross-compiling paths.
-
-# If the user has already set PKG_CONFIG_LIBDIR, believe it (even if empty): 
-# it's documented to be an override
-if [ x"${PKG_CONFIG_LIBDIR+set}" = x ]; then
-  # GNU triplet for the compiler, e.g. i486-linux-gnu for Debian i386,
-  # i686-linux-gnu for Ubuntu i386
-  basename="`basename "$0"`"
-  triplet="${basename%-pkg-config}"
-  # Normalized multiarch path if any, e.g. i386-linux-gnu for i386
-  multiarch="`dpkg-architecture -t"${triplet}" -qDEB_HOST_MULTIARCH 
2>/dev/null`"
-  # Native multiarch path
-  native_multiarch="`dpkg-architecture -qDEB_HOST_MULTIARCH 2>/dev/null`"
-
-  # This can be used for native builds as well, in that case, just exec 
pkg-config "$@" directly.
-  if [ "$native_multiarch" = "$multiarch" ]; then
- exec pkg-config "$@"
-  fi
-
-  PKG_CONFIG_LIBDIR="/usr/local/${triplet}/lib/pkgconfig"
-  # For a native build we would also want to append /usr/local/lib/pkgconfig
-  # at this point; but this is a cross-building script, so don't
-  PKG_CONFIG_LIBDIR="$PKG_CONFIG_LIBDIR:/usr/local/share/pkgconfig"
-
-  if [ -n "$multiarch" ]; then
-
PKG_CONFIG_LIBDIR="/usr/local/lib/${multiarch}/pkgconfig:$PKG_CONFIG_LIBDIR"
-PKG_CONFIG_LIBDIR="$PKG_CONFIG_LIBDIR:/usr/lib/${multiarch}/pkgconfig"
-  fi
-
-  PKG_CONFIG_LIBDIR="$PKG_CONFIG_LIBDIR:/usr/${triplet}/lib/pkgconfig"
-  # For a native build we would also want to append /usr/lib/pkgconfig
-  # at this point; but this is a cross-building script, so don't
-  # If you want to allow use of un-multiarched -dev packages for crossing 
-  # (at the risk of finding build-arch stuff you didn't want, if not in a 
clean chroot)
-  # Uncomment the next line:
-  # PKG_CONFIG_LIBDIR="$PKG_CONFIG_LIBDIR:/usr/lib/pkgconfig" 
-  PKG_CONFIG_LIBDIR="$PKG_CONFIG_LIBDIR:/usr/share/pkgconfig"
-
-  export PKG_CONFIG_LIBDIR
-fi
-
-exec pkg-config "$@"
diff -u pkg-config-0.29/debian/pkg-config-dpkghook 
pkg-config-0.29/debian/pkg-config-dpkghook
--- pkg-config-0.29/debian/pkg-config-dpkghook
+++ pkg-config-0.29/debian/pkg-config-dpkghook
@@ -14,7 +14,8 @@
 use Dpkg::Arch qw(debarch_to_gnutriplet);
 use Dpkg::ErrorHandling qw(error);
 
-my $crosswrapper = "/usr/share/pkg-config-crosswrapper";
+my $oldcrosswrapper = "/usr/share/pkg-config-crosswrapper";
+my $crosswrapper = "/usr/lib/pkg-config-crosswrapper";
 
 my $action = $ARGV[0];
 error("parameter must be 'remove' or 'update'")
@@ -33,9 +34,13 @@
 
 foreach my $symlink (keys %symlinks) {
   $symlink =~ m,^/usr/bin/([^-]+-[^-]+-[^-]+)-pkg-config, or next;
-  next if