Correct default paths on W32

[Dan Nicholson]

On Sat, Apr 6, 2013 at 12:24 PM, LRN <lrn1986 at gmail.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 06.04.2013 21:20, Dan Nicholson wrote:
>> 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. [...]
>>> + 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.
> As far as pc_path goes, i don't really care. As i have said, it is
> determined at runtime, thus anything you hardcode at configure-time has
> absolutely no effect when compiling W32 version of pkg-config.
> This change is there just for the sake of consistency.

Right, I was also applying that comment to the include include/library
paths where I don't think resolving them entirely at runtime would be
OK as a default. As far as pc_path goes, I agree it's meaningless on
Windows right now. I would actually like to make it so that you could
use the hard-coded path if you like. Something like --builtin-path. My
thinking here is that if you have a fixed development environment on
Windows, you might prefer to have a little more predictable behavior
from pkg-config.

>>> 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.
>
> OK, i've tried your 8 patches (fixed as suggested by Tollef) +
> expand-paths patch
>
> Here's what i get in config.h:
>
> /* Default pkg-config path */
> #define PKG_CONFIG_PC_PATH "/mingw/lib/pkgconfig:/mingw/share/pkgconfig"
>
> /* Default system include path */
> #define PKG_CONFIG_SYSTEM_INCLUDE_PATH "/usr/include"
>
> /* Default system library path */
> #define PKG_CONFIG_SYSTEM_LIBRARY_PATH "/usr/lib:/lib"
>
> With my ":" -> ":/;" and "usr" -> "${defprefix}" patch (attached,
> 0011-platform-dependent-separator-and-adjustable-prefix.mingw32.patch),
> i get this instead (when i configure with
> - --with-default-prefix-for-default-paths=/mingw):
>
> /* Default pkg-config path */
> #define PKG_CONFIG_PC_PATH "/mingw/lib/pkgconfig;/mingw/share/pkgconfig"
>
> /* Default system include path */
> #define PKG_CONFIG_SYSTEM_INCLUDE_PATH "/mingw/include"
>
> /* Default system library path */
> #define PKG_CONFIG_SYSTEM_LIBRARY_PATH "/mingw/lib;/lib"
>
> The --with-default-prefix-for-default-paths option is, IMO, better than
> just hard-coding host-dependent prefixes (/usr or /mingw) into
> configure.ac.
> Name seems a bit long though...

Right, so you'd be passing --with-default-prefix...=/mingw on Windows?
I think it would be cleaner to just make /mingw the default prefix on
native_win32 like you were doing before. The knobs to adjust
SYSTEM_INCLUDE_PATH and SYSTEM_LIBRARY_PATH already exist, and I'd
prefer not to add another option into that mix. I think it's pretty
well established that the standard native win32 is MINGW. Cygwin
follows an entirely different path (normal Unix), and if someone
wanted to build with MSVC, they'd almost certainly be outside the
autotools environment, anyway. Might as well just make that assumption
in the default.

> I've also attached my local prefix patch
> (0099-no-prefix-on.mingw32.patch), which hacks pkg.m4 to add
> - --dont-define-prefix to all pkg-config invocations that produce paths.
>
> This forces pkg-config to use the prefix from .pc files (which, if
> packages are built correctly, will be prefix=/mingw), thus resolving
> things like -I${prefix}/include to -I/mingw/include, not
> - -Ic:/foo/bar/mymingw/include. That works well with fixed default
> directories, as pkg-config will correctly detect that -I/mingw/include
> is the default include path, and will strip it from the output. Thus for
> most packages i get empty --cflags and only "-lfoo" --libs (without
> - -L/mingw/lib).
>
> I'm not sure this second patch is appropriate for committing as-is,
> since it might have unintended effect on other platforms.
> On W32 it should be fine, since using configure scripts (where code from
> this m4 file goes) means MSys (or Cygwin...? Didn't check how this works
> on Cygwin), and /mingw is the conventional location for the toolchain
> and user-installed stuff when MSys is used. If pkg-config is used from
> makefiles or by non-autotools buildsystems, --dont-define-prefix won't
> be there, and it will produce DOS-style paths, as expected.

I don't think adjusting pkg.m4 is the way to go for controlling
default behavior. Not only is it not the only way pkg-config is run in
the wild, but it's distributed in packages and you're likely to have
old versions of pkg.m4 used typically. If the user wasn't expecting to
rebuild configure, they'd easily run into a situation where pkg-config
seemed to behave differently for different packages. Instead, I think
the thing to do is to have a configure option that controls the
default setting of dont_define_prefix. Then I think you also need a
pkg-config option that enables the prefix defining at runtime. This
way, you control your pkg-config to behave the way you want by
default, and you're not making presumptions about how it's called.

Not sure about the names for these options. If --dont-define-prefix
didn't already exist, I'd suggest:

configure: --with/without-prefix-override (default yes Windows, no elsewhere)
pkg-config: --[no-]prefix-override (default following configure's option)

Since --dont-define-prefix exists, we'd probably want to use names
like that, but could also just make it an alias for any new names.

This discussion also brings up something I hadn't noticed before - you
seem to want to run pkg-config in a fixed path scenario like on
non-Windows. Is that right? In that case, I think there are good
reasons to make sure that the Windows features can be defaulted to off
and that you have a sane builtin pc_path. Then you could make it
behave exactly the same way if you wanted to.

> The alternative that i've mentioned is to detect default include and lib
> paths at runtime (the same way pc_path is detected), in which case
> pkg-config will be able to detect -Ic:/foo/bar/mymingw/include as the
> default path and strip it, so --dont-define-prefix will not be needed.
> Using ${prefix} in these default paths might do the trick (if pkg-config
> expands default paths at runtime).

That seems like it might be a good idea to have, too.

--
Dan