Created attachment 36071 [details] [review]
patch to fix PKG_CONFIG_SYSROOT_DIR

The problem is solved if PKG_CONFIG_ALLOW_SYSTEM_LIBS is also set:

# export PKG_CONFIG_LIBDIR="/test/usr/lib/pkgconfig"
# export PKG_CONFIG_SYSROOT_DIR="/test"
# export PKG_CONFIG_ALLOW_SYSTEM_LIBS=yes
# ./pkg-config --libs x11
-L/test/usr/lib -lX11  

This demonstrates that /usr/lib is stripped out before sysroot is applied. My feeling is that this is a bug, because /usr/lib (which is hardcoded into pkg-config) becomes irrelevant if sysroot is applied.

One possible fix is for sysroot to imply PKG_CONFIG_ALLOW_SYSTEM_LIBS (and similarly PKG_CONFIG_ALLOW_SYSTEM_CFLAGS) as well. See attached patch.

Created attachment 36074 [details] [review]
Don't strip /usr/include or /usr/lib

An even better fix would be for pkg-config to not strip /usr/include or /usr/lib at all.

This avoids having to add a new exception (previous fix) to correct an existing exception (strip /usr/include and /usr/lib), thus making pkg-config more predictable.

And it removes tricky code and unnecessary environment variables. It particular PKG_CONFIG_ALLOW_SYSTEM_CFLAGS and PKG_CONFIG_ALLOW_SYSTEM_LIBS, as well as the undocumented C_INCLUDE_PATH and CPLUS_INCLUDE_PATH, can all be removed.

Obviously this is a more radical fix. But it is far preferable in my opinion; adding new hacks to fix existing hacks always seems like the wrong approach.

Not removing /usr/lib and /usr/include is not an option, as that will in some cases lead to the compiler getting confused and then looking for headers in the wrong directories.

Thanks for the patch to not strip /usr/lib and /usr/include when the sysroot dir is set, that sounds like a reasonable solution.

I would actually venture to say that pkg-config is doing the right thing in this case. In a normal root scenario, we strip /usr/include and /usr/lib because the compiler/linker search there by default. If we add -I/usr/include -L/usr/lib in our command lines we could easily override another part of the command that sets the search path more importantly.

In a sysroot scenario (for gcc), the compiler and linker search in $sysroot/usr/include and $sysroot/usr/lib by default when you pass --sysroot to them.

$ sudo mkdir -p /test/usr{,/local}/{include,lib}
$ echo 'void main(void){}' > foo.c
$ gcc -v -o foo foo.c &> gcc-normal.log
$ gcc -v --sysroot=/test -o foo foo.c &> gcc-sysroot.log
$ diff -u gcc-normal.log gcc-sysroot.log 
--- gcc-normal.log	2012-08-21 05:35:47.110719048 -0700
+++ gcc-sysroot.log	2012-08-21 05:36:00.874363937 -0700
@@ -6,7 +6,7 @@
 Thread model: posix
 gcc version 4.6.3 20120306 (Red Hat 4.6.3-2) (GCC) 
 COLLECT_GCC_OPTIONS='-v' '-o' 'foo' '-mtune=generic' '-march=x86-64'
- /usr/libexec/gcc/x86_64-redhat-linux/4.6.3/cc1 -quiet -v foo.c -quiet -dumpbase foo.c -mtune=generic -march=x86-64 -auxbase foo -version -o /tmp/ccONRgkV.s
+ /usr/libexec/gcc/x86_64-redhat-linux/4.6.3/cc1 -quiet -v -isysroot /test foo.c -quiet -dumpbase foo.c -mtune=generic -march=x86-64 -auxbase foo -version -o /tmp/cc5lwbGj.s
 GNU C (GCC) version 4.6.3 20120306 (Red Hat 4.6.3-2) (x86_64-redhat-linux)
 	compiled by GNU C version 4.6.3 20120306 (Red Hat 4.6.3-2), GMP version 4.3.2, MPFR version 3.0.0, MPC version 0.9
 GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
@@ -15,16 +15,18 @@
 #include "..." search starts here:
 #include <...> search starts here:
  /usr/lib/gcc/x86_64-redhat-linux/4.6.3/include
- /usr/local/include
- /usr/include
+ /test/usr/local/include
+ /test/usr/include
 End of search list.
 GNU C (GCC) version 4.6.3 20120306 (Red Hat 4.6.3-2) (x86_64-redhat-linux)
 	compiled by GNU C version 4.6.3 20120306 (Red Hat 4.6.3-2), GMP version 4.3.2, MPFR version 3.0.0, MPC version 0.9
 GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
 Compiler executable checksum: a832aa6a2b1e3d9f3b0f3b81987c045f
 COLLECT_GCC_OPTIONS='-v' '-o' 'foo' '-mtune=generic' '-march=x86-64'
- as --64 -o /tmp/ccjkJwzB.o /tmp/ccONRgkV.s
+ as --64 -o /tmp/cc87qhYv.o /tmp/cc5lwbGj.s
 COMPILER_PATH=/usr/libexec/gcc/x86_64-redhat-linux/4.6.3/:/usr/libexec/gcc/x86_64-redhat-linux/4.6.3/:/usr/libexec/gcc/x86_64-redhat-linux/:/usr/lib/gcc/x86_64-redhat-linux/4.6.3/:/usr/lib/gcc/x86_64-redhat-linux/
-LIBRARY_PATH=/usr/lib/gcc/x86_64-redhat-linux/4.6.3/:/usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../../../lib64/:/lib/../lib64/:/usr/lib/../lib64/:/usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../../:/lib/:/usr/lib/
+LIBRARY_PATH=/usr/lib/gcc/x86_64-redhat-linux/4.6.3/:/usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../../../lib64/:/usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../../:/test/usr/lib/
 COLLECT_GCC_OPTIONS='-v' '-o' 'foo' '-mtune=generic' '-march=x86-64'
- /usr/libexec/gcc/x86_64-redhat-linux/4.6.3/collect2 --build-id --no-add-needed --eh-frame-hdr -m elf_x86_64 --hash-style=gnu -dynamic-linker /lib64/ld-linux-x86-64.so.2 -o foo /usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../../../lib64/crt1.o /usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../../../lib64/crti.o /usr/lib/gcc/x86_64-redhat-linux/4.6.3/crtbegin.o -L/usr/lib/gcc/x86_64-redhat-linux/4.6.3 -L/usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../../../lib64 -L/lib/../lib64 -L/usr/lib/../lib64 -L/usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../.. /tmp/ccjkJwzB.o -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed /usr/lib/gcc/x86_64-redhat-linux/4.6.3/crtend.o /usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../../../lib64/crtn.o
+ /usr/libexec/gcc/x86_64-redhat-linux/4.6.3/collect2 --sysroot=/test --build-id --no-add-needed --eh-frame-hdr -m elf_x86_64 --hash-style=gnu -dynamic-linker /lib64/ld-linux-x86-64.so.2 -o foo /usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../../../lib64/crt1.o /usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../../../lib64/crti.o /usr/lib/gcc/x86_64-redhat-linux/4.6.3/crtbegin.o -L/usr/lib/gcc/x86_64-redhat-linux/4.6.3 -L/usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../../../lib64 -L/usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../.. -L/test/usr/lib /tmp/cc87qhYv.o -lgcc --as-needed -lgcc_s --no-as-needed -lc -lgcc --as-needed -lgcc_s --no-as-needed /usr/lib/gcc/x86_64-redhat-linux/4.6.3/crtend.o /usr/lib/gcc/x86_64-redhat-linux/4.6.3/../../../../lib64/crtn.o
+/usr/bin/ld: this linker was not configured to use sysroots
+collect2: ld returned 1 exit status

It's a little difficult to see with all the noise, but the compiler is looking for headers under /test/usr/include and /test/usr/local/include. It's then telling the linker to look for libraries in /test/usr/lib in addition to the compiler private directories in /usr/lib/gcc. After all, the point of sysroot building is that the sysroot should be transparent to the build as much as possible.

http://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html

So, I think pkg-config is doing the right thing by treating /usr/include & /usr/lib specially even when under the sysroot. Is your usage different? How are you building in the sysroot? That said, the first patch doesn't seem too risky.

I'm going to close this NOTABUG for two reasons.

1. pkg-config seems to be doing the right thing when used with the only sysroot-enabled toolchain I can think of (gcc + GNU ld). By that I mean that it's suppressing the toolchain default paths just as it does in a normal root scenario.

2. The options PKG_CONFIG_ALLOW_SYSTEM_CFLAGS and PKG_CONFIG_ALLOW_SYSTEM_LIBS are there to override this behavior if the user desires.

Please reopen if you feel strongly that pkg-config should not suppress these paths when using a sysroot.