Bug 94729 - Use INCLUDE to detect system include directories when using --msvc-syntax
Summary: Use INCLUDE to detect system include directories when using --msvc-syntax
Status: RESOLVED FIXED
Alias: None
Product: pkg-config
Classification: Unclassified
Component: src (show other bugs)
Version: unspecified
Hardware: Other Windows (All)
: medium normal
Assignee: Dan Nicholson
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-28 09:13 UTC by Nirbheek Chauhan
Modified: 2017-03-20 16:35 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
pkg-config --debug --cflags flac (16.51 KB, text/plain)
2016-04-10 23:17 UTC, Nirbheek Chauhan
Details

Description Nirbheek Chauhan 2016-03-28 09:13:55 UTC
Two of pkg-config's features conflict with each other:

1) --define-prefix (which is the default now) which auto-detects the prefix so that pkg-config files keep working when the prefix changes

2) Stripping of include paths that are ${prefix}/include (disabled by setting PKG_CONFIG_ALLOW_SYSTEM_CFLAGS)

When the prefix is not /usr, we *want* -I${prefix}/include to be passed in with cflags otherwise the compiler can't find the necessary headers.

To prevent this, I think pkg-config should not strip "system" include paths when it redefines the prefix.
Comment 1 Dan Nicholson 2016-04-09 16:16:18 UTC
Good catch, that makes sense. I'll take a look at that.
Comment 2 Dan Nicholson 2016-04-09 18:19:15 UTC
Sorry, I'm actually a bit confused about the issue here. --define-prefix is only the default on Windows. On all other platforms, it's false and the behavior with respect to prefix redefinition _should_ be unchanged.

Regardless, I don't think the system CFLAGS determination is affected by this. It's constructed at compile time or affected by environment variables.

Could you provide an example of where this is failing and run pkg-config with --debug? Thanks.
Comment 3 Nirbheek Chauhan 2016-04-10 23:17:55 UTC
Created attachment 122852 [details]
pkg-config --debug --cflags flac

(In reply to Dan Nicholson from comment #2)
> Sorry, I'm actually a bit confused about the issue here. --define-prefix is
> only the default on Windows. On all other platforms, it's false and the
> behavior with respect to prefix redefinition _should_ be unchanged.
> 

Sorry for not specifying earlier; I'm facing this issue on Windows. The removal of "system includedirs" from cflags makes sense when they're already included in the default include paths used by the compiler.

With gcc and clang, extra paths are specified in the `C_INCLUDE_PATH` env var and the full list can be found with `cpp -Wp,-v`. With MSVC the full list is in `INCLUDE`. Unless the path is in the relevant variable , the path is needed to find the headers.

I don't know if pkg-config should parse those paths and only then remove the include paths or just not remove include paths when it redefines the prefix. The latter is simpler.

> Regardless, I don't think the system CFLAGS determination is affected by
> this. It's constructed at compile time or affected by environment variables.
> 
> Could you provide an example of where this is failing and run pkg-config
> with --debug? Thanks.

`--debug` output + looking at the source code is how I found this problem. See attached log output. The environment has `PKG_CONFIG_LIDBIR="C:/.../lib/pkg-config"` set, which is how the `flac.pc` file is found.

The relevant portions are:

Parsing package file 'C:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/lib/pkgconfig\flac.pc'
  line>prefix=C:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86
 Variable declaration, 'prefix' overridden with 'C:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86'

[...]

  line>Cflags: -I${includedir}

[...]

Package flac has -IC:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/include in Cflags
Removing -IC:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/include from cflags for flac
Comment 4 Dan Nicholson 2016-04-11 21:54:42 UTC
(In reply to Nirbheek Chauhan from comment #3)
> Created attachment 122852 [details]
> pkg-config --debug --cflags flac
> 
> (In reply to Dan Nicholson from comment #2)
> > Sorry, I'm actually a bit confused about the issue here. --define-prefix is
> > only the default on Windows. On all other platforms, it's false and the
> > behavior with respect to prefix redefinition _should_ be unchanged.
> > 
> 
> Sorry for not specifying earlier; I'm facing this issue on Windows. The
> removal of "system includedirs" from cflags makes sense when they're already
> included in the default include paths used by the compiler.
> 
> With gcc and clang, extra paths are specified in the `C_INCLUDE_PATH` env
> var and the full list can be found with `cpp -Wp,-v`. With MSVC the full
> list is in `INCLUDE`. Unless the path is in the relevant variable , the path
> is needed to find the headers.

I'm still not following how you got the setting that your directory is being detected as a system path. Are you saying the MSVC sets the environment variable C_INCLUDE_PATH and that's how pkg-config is finding it? Or that pkg-config was built with that path as a system path?

> I don't know if pkg-config should parse those paths and only then remove the
> include paths or just not remove include paths when it redefines the prefix.
> The latter is simpler.

I don't think the 2 features actually have much to do with each other. Whether you want pkg-config to magically redefine the prefix in the pc file or not doesn't have anything to do with what it considers your compiler's default paths.
Comment 5 Nirbheek Chauhan 2016-04-11 22:24:39 UTC
It looks like I tried to say too much and everything became confusing. Please just ignore what I said earlier and look at this:

When I type `pkg-config --cflags flac`, I get no output at all. The output should be "-IC:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/include". Here are relevant snippets from the logs:

---
Parsing package file 'C:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/lib/pkgconfig\flac.pc'
  line>prefix=C:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86
 Variable declaration, 'prefix' overridden with 'C:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86'

[...]

  line>Cflags: -I${includedir}

[...]

Package flac has -IC:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/include in Cflags
Removing -IC:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/include from cflags for flac
---

This is the bug. The relevant things I have in the environment are:

PKG_CONFIG_LIBDIR=C:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/lib/pkgconfig
PKG_CONFIG_PATH=C:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/share/pkgconfig
Comment 6 Dan Nicholson 2016-04-11 22:48:57 UTC
(In reply to Nirbheek Chauhan from comment #5)
> 
> Package flac has
> -IC:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/include in Cflags
> Removing -IC:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/include
> from cflags for flac
> ---
> 
> This is the bug. The relevant things I have in the environment are:
> 
> PKG_CONFIG_LIBDIR=C:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/
> lib/pkgconfig
> PKG_CONFIG_PATH=C:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/
> share/pkgconfig

Thanks, I understand what you're seeing now. Can you help determine why pkg-config considers that to be a system path? You might have to add some prints to the code in pkg.c or something. I'd like to understand that before making any changes in this behavior as it's been this way in pkg-config for as long as I can remember.
Comment 7 Nirbheek Chauhan 2016-04-11 23:22:41 UTC
(In reply to Dan Nicholson from comment #6)
> (In reply to Nirbheek Chauhan from comment #5)
> > 
> > Package flac has
> > -IC:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/include in Cflags
> > Removing -IC:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/include
> > from cflags for flac
> > ---
> > 
> > This is the bug. The relevant things I have in the environment are:
> > 
> > PKG_CONFIG_LIBDIR=C:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/
> > lib/pkgconfig
> > PKG_CONFIG_PATH=C:/MinGW/msys/1.0/home/Nirbheek/cerbero/dist/windows_x86/
> > share/pkgconfig
> 
> Thanks, I understand what you're seeing now. Can you help determine why
> pkg-config considers that to be a system path? You might have to add some
> prints to the code in pkg.c or something. I'd like to understand that before
> making any changes in this behavior as it's been this way in pkg-config for
> as long as I can remember.

Hmm, looks like pkg-config considers that to be a system path because the variables C_INCLUDE_PATH and CPLUS_INCLUDE_PATH contain that (because of the build system I'm building inside of). So this behaviour is expected when you're using gcc or clang (the default case) and I was wrong earlier. Apologies for all the confusion and time wasted!

However, pkg-config should still output the include paths when using msvc syntax since MSVC's compiler doesn't consider paths inside those variables as system include directores. `pkg-config --cflags --msvc-syntax flac` should output the full include path. That's how I originally came across this issue. :)

I think that when using msvc syntax, pkg-config should look at INCLUDE instead. See section "Remarks" at: https://msdn.microsoft.com/en-us/library/73f9s62w.aspx

I'll rename the bug.
Comment 8 Dan Nicholson 2016-04-12 00:08:57 UTC
OK, that makes sense now. I think the change you suggest sounds sane. Those environment variables should really be documented. Sorry about that. I've been meaning to do that and will get that on for the next release.
Comment 9 Dan Nicholson 2017-03-20 16:35:26 UTC
Fixed in commit df6e4b6. I also added some documentation in
pkg-config(1) for how the system paths are handled.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.