Correct default paths on W32

Dan Nicholson dbn.lists at gmail.com
Sat Apr 6 10:20:18 PDT 2013


On Sat, Mar 30, 2013 at 10:52:22PM +0400, LRN wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 30.03.2013 21:52, LRN wrote:
> > (2) has some build-time implications. For example, you'll need to 
> > reconfigure pkg-config to build with different paths (with current 
> > system you can just pass different stuff via command line), and 
> > variables (such as ${libdir}) will also expand during configure,
> > not at the moment of compiler invocation.

I do think the default on Windows should stay as it is where by default
the search path is determined at runtime and the system include/library
paths are defined at build time. In both cases there are mechanisms for
overriding the defaults if necessary. I would like to get rid of those
warnings, though.

Longer term, I'd like it if all the Windows specific stuff was available
as an option everywhere and vice versa. For instance, I'd like it if the
runtime search path determination was available on Linux even if it
wasn't the default. Then I could test that behavior without any hassle.

> After tinkering with the code i've realized that ${libdir} should not
> be expanded at all, in fact. It is passed like that all the way to the
> source code, and used as-is.
> 
> Anyway, here's a patch. Tell me what you think of it.

Thanks for looking at this. I think this is pretty good, but I think
there would be a couple issues as is. See below.

> Date: Sat, 30 Mar 2013 22:49:44 +0400
> Subject: [PATCH] Define default paths at configuration time, pass via
>  config.h
> 
> ---
>  Makefile.am  | 13 -------------
>  configure.ac | 41 ++++++++++++++++++++++++-----------------
>  2 files changed, 24 insertions(+), 30 deletions(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 41e48e8..b808aa4 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -9,19 +9,6 @@ endif
>  SUBDIRS = $(GLIB_SUBDIR) . check
>  DIST_SUBDIRS = $(SUBDIRS)
>  
> -# Escape paths properly on windows
> -if NATIVE_WIN32
> -AM_CPPFLAGS = \
> -	-DPKG_CONFIG_PC_PATH="\"$(subst /,\/,$(pc_path))\"" \
> -	-DPKG_CONFIG_SYSTEM_INCLUDE_PATH="\"$(subst /,\/,$(system_include_path))\"" \
> -	-DPKG_CONFIG_SYSTEM_LIBRARY_PATH="\"$(subst /,\/,$(system_library_path))\""
> -else
> -AM_CPPFLAGS = \
> -	-DPKG_CONFIG_PC_PATH="\"$(pc_path)\"" \
> -	-DPKG_CONFIG_SYSTEM_INCLUDE_PATH="\"$(system_include_path)\"" \
> -	-DPKG_CONFIG_SYSTEM_LIBRARY_PATH="\"$(system_library_path)\""
> -endif
> -
>  AM_CFLAGS = \
>  	$(WARN_CFLAGS) \
>  	$(GCOV_CFLAGS) \
> diff --git a/configure.ac b/configure.ac
> index 2b33371..b80fcd6 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -32,6 +32,22 @@ AC_ARG_VAR([TESTS_SHELL], [Path to a POSIX shell to be used for testing])
>  conf_path="$PATH:`getconf PATH 2>/dev/null`"
>  AC_PATH_PROGS([TESTS_SHELL], [bash ksh sh], [$CONFIG_SHELL], [$conf_path])
>  
> +AC_MSG_CHECKING([for Win32])
> +case "$build" in
> +  *-*-mingw*)
> +    native_win32=yes
> +    pathseparator=";"
> +    defaultprefix=/mingw
> +  ;;
> +  *)
> +    native_win32=no
> +    pathseparator=":"
> +    defaultprefix=/usr
> +  ;;
> +esac
> +AC_MSG_RESULT([$native_win32])
> +AM_CONDITIONAL(NATIVE_WIN32, [test "x$native_win32" = xyes])

I like the idea of getting /mingw into the default path. BTW, I changed
$build to $host in one of the patches I just posted.

> +
>  dnl
>  dnl Default pkg-config search path
>  dnl
> @@ -44,12 +60,13 @@ AC_ARG_WITH([pc_path],
>  # This is slightly wrong, but hopefully causes less confusion than
>  # people being unable to find their .pc files in the standard location.
>  if test "${prefix}" = "NONE"; then
> -	pc_path='${libdir}/pkgconfig:${datadir}/pkgconfig:/usr/lib/pkgconfig:/usr/share/pkgconfig'
> +	pc_path="\${libdir}/pkgconfig${pathseparator}\${datadir}/pkgconfig${pathseparator}${defaultprefix}/lib/pkgconfig${pathseparator}${defaultprefix}/share/pkgconfig"
>  else
> -	pc_path='${libdir}/pkgconfig:${datadir}/pkgconfig'
> +	pc_path="\${libdir}/pkgconfig${pathseparator}\${datadir}/pkgconfig"

I don't think this is OK to do for a couple reasons.

1. This is supposed to be a fixed path, and PKG_CONFIG_LIBDIR is there
   if you want to tweak at runtime. This makes the path expansion
   entirely delayed until runtime, and would be subject to someone
   passing --define-variable=libdir=/foo. That would cause not only the
   packages to be affected, but also pkg-config itself. This is kind of
   par for the course on the Windows build, but everywhere else,
   pkg-config is expected to be unchanged.

2. What ${libdir} and ${datadir} would this correspond to? At the time
   that pkg-config expands the path, I can't even think what these would
   be. And if it ever became dependent on ${libdir} in a .pc file, that
   would be bad.

>  fi
>  ])
>  AC_MSG_RESULT([$pc_path])
> +AC_DEFINE_UNQUOTED([PKG_CONFIG_PC_PATH], ["$pc_path"], [Default location of .pc files])
>  AC_SUBST([pc_path])
>  
>  dnl
> @@ -60,8 +77,9 @@ AC_ARG_WITH([system_include_path],
>    [AS_HELP_STRING([--with-system-include-path],
>      [avoid -I flags from the given path])],
>    [system_include_path="$withval"],
> -  [system_include_path="/usr/include"])
> +  [system_include_path="${defaultprefix}/include"])
>  AC_MSG_RESULT([$system_include_path])
> +AC_DEFINE_UNQUOTED([PKG_CONFIG_SYSTEM_INCLUDE_PATH], ["$system_include_path"], [Default location of headers])
>  AC_SUBST([system_include_path])
>  
>  dnl
> @@ -76,14 +94,15 @@ AC_ARG_WITH([system_library_path],
>  pc_lib_sfx=`echo "$libdir" | sed 's:.*/lib::'`
>  case "$pc_lib_sfx" in
>  */*|"")
> -  system_library_path="/usr/lib:/lib"
> +  system_library_path="${defaultprefix}/lib${pathseparator}/lib"
>    ;;
>  *)
> -  system_library_path="/usr/lib$pc_lib_sfx:/lib$pc_lib_sfx:/usr/lib:/lib"
> +  system_library_path="${defaultprefix}/lib$pc_lib_sfx${pathseparator}/lib$pc_lib_sfx${pathseparator}${defaultprefix}/lib${pathseparator}/lib"
>    ;;
>  esac

This part I think is good. I'm attaching a patch that does the full
expansion in configure. Could you try that out? Then maybe you could
split out the /mingw paths part on top of it.

--
Dan
-------------- next part --------------
>From 31c522842364459c2a240ac87de2d9f9ba07aab3 Mon Sep 17 00:00:00 2001
Message-Id: <31c522842364459c2a240ac87de2d9f9ba07aab3.1365267423.git.dbn.lists at gmail.com>
From: Dan Nicholson <dbn.lists at gmail.com>
Date: Thu, 4 Apr 2013 07:39:56 -0700
Subject: [PATCH] Expand paths in configure to avoid MSYS garbage in Makefile

The default pkg-config path and system include/library paths are
currently defined in the Makefile so that make will expand the
variables. Due to MSYS shell mangling paths with forward slashes into
Windows style paths, the path separators all need to be escaped before
passing to the compiler via -D. This causes warnings in both automake
and the compiler about "\/".

Expand the variables in configure and set them in config.h to avoid this
issue.
---
 Makefile.am      | 13 -------------
 check/check-path |  3 +--
 configure.ac     | 28 ++++++++++++++++++++++++++++
 3 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 41e48e8..b808aa4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -9,19 +9,6 @@ endif
 SUBDIRS = $(GLIB_SUBDIR) . check
 DIST_SUBDIRS = $(SUBDIRS)
 
-# Escape paths properly on windows
-if NATIVE_WIN32
-AM_CPPFLAGS = \
-	-DPKG_CONFIG_PC_PATH="\"$(subst /,\/,$(pc_path))\"" \
-	-DPKG_CONFIG_SYSTEM_INCLUDE_PATH="\"$(subst /,\/,$(system_include_path))\"" \
-	-DPKG_CONFIG_SYSTEM_LIBRARY_PATH="\"$(subst /,\/,$(system_library_path))\""
-else
-AM_CPPFLAGS = \
-	-DPKG_CONFIG_PC_PATH="\"$(pc_path)\"" \
-	-DPKG_CONFIG_SYSTEM_INCLUDE_PATH="\"$(system_include_path)\"" \
-	-DPKG_CONFIG_SYSTEM_LIBRARY_PATH="\"$(system_library_path)\""
-endif
-
 AM_CFLAGS = \
 	$(WARN_CFLAGS) \
 	$(GCOV_CFLAGS) \
diff --git a/check/check-path b/check/check-path
index 2a6be75..d50f421 100755
--- a/check/check-path
+++ b/check/check-path
@@ -7,8 +7,7 @@ set -e
 RESULT=""
 PKG_CONFIG_PATH="$srcdir/sub" run_test --exists sub1
 
-# default pkg-config path, making sure to resolve the variables fully
-eval pc_path="$pc_path"
+# check default pkg-config path
 if [ "$native_win32" = yes ]; then
     # This is pretty hacky. On native win32 (MSYS/MINGW), pkg-config
     # builds the default path from the installation directory. It
diff --git a/configure.ac b/configure.ac
index 94666a4..481573b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -37,6 +37,25 @@ dnl from the make command line if necessary.
 AC_SUBST([TESTS_PKG_CONFIG], ['${top_builddir}/pkg-config${EXEEXT}'])
 
 dnl
+dnl Macro to expand variables
+dnl
+AC_DEFUN([PKG_EXPAND_VAR],
+[m4_pushdef([var], [m4_default([$2], [$1])])
+var=[$]$1
+_prefix_save=$prefix
+_exec_prefix_save=$exec_prefix
+test "x$prefix" = xNONE && prefix=$ac_default_prefix
+test "x$exec_prefix" = xNONE && exec_prefix='${prefix}'
+_prev=
+while test "x$var" != "x$_prev"; do
+  _prev=$var
+  eval var=\"$var\"
+done
+prefix=$_prefix_save
+exec_prefix=$_exec_prefix_save
+m4_popdef([var])])
+
+dnl
 dnl Default pkg-config search path
 dnl
 AC_MSG_CHECKING([for default search path for .pc files])
@@ -53,8 +72,11 @@ else
 	pc_path='${libdir}/pkgconfig:${datadir}/pkgconfig'
 fi
 ])
+PKG_EXPAND_VAR([pc_path])
 AC_MSG_RESULT([$pc_path])
 AC_SUBST([pc_path])
+AC_DEFINE_UNQUOTED([PKG_CONFIG_PC_PATH], ["$pc_path"],
+                   [Default pkg-config path])
 
 dnl
 dnl System default -I paths
@@ -65,8 +87,11 @@ AC_ARG_WITH([system_include_path],
     [avoid -I flags from the given path])],
   [system_include_path="$withval"],
   [system_include_path="/usr/include"])
+PKG_EXPAND_VAR([system_include_path])
 AC_MSG_RESULT([$system_include_path])
 AC_SUBST([system_include_path])
+AC_DEFINE_UNQUOTED([PKG_CONFIG_SYSTEM_INCLUDE_PATH], ["$system_include_path"],
+                   [Default system include path])
 
 dnl
 dnl System default -L paths
@@ -87,8 +112,11 @@ case "$pc_lib_sfx" in
   ;;
 esac
 ])
+PKG_EXPAND_VAR([system_library_path])
 AC_MSG_RESULT([$system_library_path])
 AC_SUBST([system_library_path])
+AC_DEFINE_UNQUOTED([PKG_CONFIG_SYSTEM_LIBRARY_PATH], ["$system_library_path"],
+                   [Default system library path])
 
 dnl Code taken from gtk+-2.0's configure.in.
 dnl
-- 
1.8.1.4



More information about the pkg-config mailing list