Bug 688171

Summary: i686-pc-mingw32-pkg-config shouldn't unset PKG_CONFIG_PATH
Product: [Fedora] Fedora Reporter: Neil Roberts <bpeeluk>
Component: mingw32-filesystemAssignee: Richard W.M. Jones <rjones>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: drizt72, erik-fedora, fedora-mingw, kalevlember, kevin, marcandre.lureau, rjones, yselkowi
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: mingw32-filesystem-67-1.fc15 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-05-19 21:56:41 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Neil Roberts 2011-03-16 14:23:40 UTC
The MingW32 compiler on Fedora includes a wrapper script for pkg-config called i686-pc-mingw32-pkg-config that sets PKG_CONFIG_LIBDIR. Presumably the idea of this is to prevent pkg-config from picking up the native system .pc files when cross compiling for Windows. However the script also does “unset PKG_CONFIG_PATH”. This makes it very difficult to use pkg-config when using a custom cross-compile environment where all of your dependencies are installed in a non-standard prefix (typically somewhere in your home directory). I can't see why the script would want to unset PKG_CONFIG_PATH as this won't be typically set on a normal system so it's unlikely to pick up any native .pc files. A developer would usually want to explicitly set this variable when building in a custom environment so he or she would know to set it to only include the Win32 environment path.

This was causing problems when cross-compiling Clutter on Fedora systems. Clutter includes a build script for cross-compiling that installs all of Clutter's dependencies into a custom prefix and then tries to set PKG_CONFIG_PATH to point to this prefix. However Fedora's pkg-config wrapper script prevents this variable from being passed to pkg-config. Clutter's build script now explicitly includes its own wrapper for pkg-config to work around Fedora.

I think the solution is to just remove the line that unsets PKG_CONFIG_PATH.

Comment 1 Erik van Pienbroek 2011-03-17 11:27:34 UTC
Unfortunately this will break compilation of packages using RPM:
$ grep -Hr PKG_CONFIG /usr/lib/rpm/*
macros:  PKG_CONFIG_PATH=\"%{_libdir}/pkgconfig:%{_datadir}/pkgconfig\"\
macros:  export PKG_CONFIG_PATH\
pkgconfigdeps.sh:	export PKG_CONFIG_PATH="$DIR:$DIR/../../share/pkgconfig"
pkgconfigdeps.sh:	export PKG_CONFIG_PATH="$DIR:$DIR/../../share/pkgconfig"

The match in the macros file is part of the %___build_pre section (which will always get called in RPM %build sections)

If I change the mingw32-curl package to this:
 %build
+export > /tmp/rpm_export1
 %{_mingw32_configure} \
   --with-ssl --enable-ipv6 \
   --with-ca-bundle=%{_mingw32_sysconfdir}/pki/tls/certs/ca-bundle.crt \
   --with-libidn \
   --enable-static --with-libssh2 \
   --without-random
+export > /tmp/rpm_export2

Then the /tmp/rpm_export1 file will contain the line: export PKG_CONFIG_PATH="/usr/lib64/pkgconfig:/usr/share/pkgconfig" (set by the %___build_pre macro)
The /tmp/rpm_export2 file doesn't have the PKG_CONFIG_PATH export as the %{_mingw32_configure} call (thus %{_mingw32_env}) has replaced it with: export PKG_CONFIG_LIBDIR="/usr/i686-w64-mingw32/sys-root/mingw/lib/pkgconfig" (which is what we want to avoid pulling in native stuff)

A possible solution to your issue would be that a new variable like MINGW32_PKG_CONFIG_PATH gets introduced. When it's set, we won't unset the PKG_CONFIG_PATH, but set the PKG_CONFIG_PATH to the value of that MINGW32_PKG_CONFIG_PATH variable. So with this, you could do something like:

MINGW32_PKG_CONFIG_PATH=/opt/my_custom_prefix/lib/pkgconfig pkg-config --cflags glib-2.0

How do the other Fedora MinGW maintainers think of such a change?

Comment 2 Richard W.M. Jones 2011-03-17 11:58:42 UTC
Appears that MINGW32_PKG_CONFIG_PATH is a good practical
solution.  Does it fit the reporter's need?

Comment 3 Kalev Lember 2011-03-17 12:24:13 UTC
(In reply to comment #1)
> Unfortunately this will break compilation of packages using RPM:

Not necessarily.

The way i686-pc-mingw32-* wrapper scripts are currently done is that they internally evaluate the matching rpm macro. For example, i686-pc-mingw32-pkg-config runs 'rpm --eval %{_mingw32_pkg_config}'.

We need the unset command in the rpm macro, but it's not needed for the wrapper script. In fact, it's not only unneeded but might cause problems when configure scripts call <host-triplet>-pkg-config.

I would say another solution is rewriting i686-pc-mingw32-pkg-config script so it wouldn't call rpm --eval internally. With that we'd be able to get rid of the 'unset' command in the script, but keep it in the rpm macro.

Comment 4 Kevin Kofler 2011-03-19 20:55:05 UTC
What about filtering the contents of PKG_CONFIG_PATH for bad directories instead of unsetting it entirely? It's more code, of course, but it'd work better.

Comment 5 Yaakov Selkowitz 2011-04-05 01:45:23 UTC
(In reply to comment #3)
> We need the unset command in the rpm macro, but it's not needed for the wrapper
> script. In fact, it's not only unneeded but might cause problems when configure
> scripts call <host-triplet>-pkg-config.
> 
> I would say another solution is rewriting i686-pc-mingw32-pkg-config script so
> it wouldn't call rpm --eval internally. With that we'd be able to get rid of
> the 'unset' command in the script, but keep it in the rpm macro.

Why not just remove the "unset PKG_CONFIG_PATH" from %_mingw32_pkg_config but leave it in %_mingw32_env?  That way PKG_CONFIG_PATH will still be honoured when building manually, such as the OP's example, but will be ignored when building RPMs.

Comment 6 Erik van Pienbroek 2011-04-10 14:54:56 UTC
That sounds like a good solution.
Here's a proposed patch:

--- a/macros.mingw32
+++ b/macros.mingw32
@@ -97,9 +97,7 @@ package or when debugging this package.\
   unset x i
 
 %_mingw32_pkg_config \
-  PKG_CONFIG_LIBDIR="%{_mingw32_libdir}/pkgconfig"; export PKG_CONFIG_LIBDIR; \
-  unset PKG_CONFIG_PATH; \
-  pkg-config
+  PKG_CONFIG_LIBDIR="%{_mingw32_libdir}/pkgconfig" pkg-config
 
 %_mingw32_configure %{_mingw32_env} ; \
   __mingw32_topdir=.; if ! test -x configure; then __mingw32_topdir=..; fi; \\\

The %_mingw32_env macro still has the unset PKG_CONFIG_PATH, but it shouldn't be called when one calls the script 'mingw32-pkg-config' or 'i686-pc-mingw32-pkg-config'.

Is the patch good enough to commit?

Comment 7 Marc-Andre Lureau 2011-05-12 13:39:24 UTC
+1, yes please :)

Comment 8 Kalev Lember 2011-05-12 18:13:48 UTC
+1, looks good.

Comment 9 Fedora Update System 2011-05-14 12:14:59 UTC
mingw32-filesystem-67-1.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/mingw32-filesystem-67-1.fc15

Comment 10 Fedora Update System 2011-05-17 05:37:45 UTC
Package mingw32-filesystem-67-1.fc15:
* should fix your issue,
* was pushed to the Fedora 15 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing mingw32-filesystem-67-1.fc15'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/mingw32-filesystem-67-1.fc15
then log in and leave karma (feedback).

Comment 11 Fedora Update System 2011-05-19 21:56:35 UTC
mingw32-filesystem-67-1.fc15 has been pushed to the Fedora 15 stable repository.  If problems still persist, please make note of it in this bug report.