include system pkgconfig path

[vbraun]

Here we actually use the system-wide pkg-config if present to query both Sage's as well as the system's package-config (*.pc) files.

If only Sage's pkgconf is available, .pc files (and hence the packages they belong to) installed outside of Sage still won't be seen by Sage's pkg-config wrapper script.

It is therefore still recommended to have a system-wide pkg-config, despite Sage now having its own, e.g. to let R use graphics packages Sage doesn't ship.

Component: build

Keywords: R cairo pango X11 graphics pkg-config

Author: Volker Braun

Branch: e9b4a60

Reviewer: Leif Leonhardy

Issue created by migration from https://trac.sagemath.org/ticket/16274

[vbraun]

Branch: u/vbraun/include_system_pkgconfig_path

[vbraun]

Commit: 5e17a6e

[vbraun]

Author: Volker Braun

[vbraun]

New commits:

5e17a6e

append system pgk-config search path

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 5e17a6e to afcbfaf

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

afcbfaf

reorder so we catch the right error

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:4

Well, as mentioned on sage-devel,

pkg-config --variable pc_path pkg-config

doesn't work with all versions.

I'd also try to grep the output of pkg-config --debug for Scanning ....

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:5

$ pkg-config --debug 2>&1 | awk '/^Scanning /{ path[i++]=substr($3,2,length($3)-2) } END{ for(j=0;j<i;j++) printf("%s%s",path[j],(j==i-1)?"\n":":") }'
/usr/local/lib/pkgconfig:/usr/lib/pkgconfig:/usr/share/pkgconfig

:-)

But awk may be gawk or whatever...

[kiwifb]

comment:6

I think at this stage we should check if it works with other implementations of pkg-config. So far we are relying on it being a particular implementation if not version. As soon as my new build on power7 finishes I'll test the one currently shipping with sage.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:7

More portable awk version:

pkg-config --debug 2>&1 | awk '/^Scanning /{ path[i++]=substr($3,2,length($3)-2) } END{ for(j=0;j<i;j++) if(j==i-1) printf("%s\n",path[j]); else printf("%s:",path[j]) }'

[kiwifb]

comment:8

Version we ship in sage doesn't have "--debug" so our best effort so far relies on any pkg-config out there being the freedesktop version which to fair is not a bad assumption. Do we know any distro not defaulting to that?

[kiwifb]

comment:9

Of course after saying that I realised that in Gentoo I can switch to the BSD one if I want (but freedesktop is the default). I would imagine debian would be similar, freedesktop is the default but you have an alternative.

Then again anyone savvy enough to use an alternative should be able to deal with the fall out.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:10

One issue still is that we have to call the system-wide pkg-config (if present) in spkg-install; I'd suggest to modify sage-env to save the original PATH in SAGE_ORIG_PATH, say.

(Otherwise we'd presumably at least accumulate folders upon reinstallation of Sage's pkgconf.)

Or we could first try to delete Sage's symlink from pkg-config to pkgconf.

[kiwifb]

comment:11

Replying to @nexttime:

One issue still is that we have to call the system-wide pkg-config (if present) in spkg-install; I'd suggest to modify sage-env to save the original PATH in SAGE_ORIG_PATH, say.

(Otherwise we'd presumably at least accumulate folders upon reinstallation of Sage's pkgconf.)

Or we could first try to delete Sage's symlink from pkg-config to pkgconf.

Have you read Volker's patch? One of the first thing he does is removing any pkg-config already installed in sage. Since we are interested in hardcoded path in the application doing

PKG_CONFIG_PATH="" pkg-config .....

would work.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:12

Replying to @kiwifb:

Replying to @nexttime:

One issue still is that we have to call the system-wide pkg-config (if present) in spkg-install; I'd suggest to modify sage-env to save the original PATH in SAGE_ORIG_PATH, say.

(Otherwise we'd presumably at least accumulate folders upon reinstallation of Sage's pkgconf.)

Or we could first try to delete Sage's symlink from pkg-config to pkgconf.

Have you read Volker's patch?

Yes, a while ago though... m)

One of the first thing he does is removing any pkg-config already installed in sage.

Well, "the" pkg-config if present.

Since we are interested in hardcoded path in the application doing

PKG_CONFIG_PATH="" pkg-config .....

would work.

Yep. Or go with (unset PKG_CONFIG_PATH; ...).

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:13

FWIW, here's the --debug version with just sed (one line per folder):

$ pkg-config --debug 2>&1 | sed -n '/^Scanning /s/.*'\''\([^'\'']*\)'\''/\1/p'
/usr/local/lib/pkgconfig
/usr/lib/pkgconfig
/usr/share/pkgconfig

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:14

One small problem:

While older versions of pkg-config (such as 0.15.0) exit with a non-zero status when called with

pkg-config --variable pc_path pkg-config

some "newer" versions (still not supporting pc_path, such as 0.22) return 0 (and print a blank line).

So we cannot just check the exit code.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from afcbfaf to ef9b2fb

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

ef9b2fb

fallback to system pkg-config if necessary

[vbraun]

comment:16

Ok different approach. Instead of just creating a symlink "pkg-config", use a shell script that calls the system pkg-config if there is one.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:17

Hmmm, that way broken pkg-config installations (seen on Darwin) aren't (or can't be) cured.

Anyway, why /bin/sh?

s/"pkg-config" binary/"pkg-config" script/

You'd have to change the error message (in case m4 failed) accordingly.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:18

Also, PKG_CONFIG_PATH gets overwritten. (We already treat it correctly in sage-env, prepending Sage's folders in case it's already set/non-empty.)

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:19

It's also unclear to me whether that solves problems with R (?) on systems where there isn't a system-wide pkg-config.

(I guess if R sees there's a pkg-config, it relies on it, and only otherwise tries to find libraries by itself. But that's just a guess, and it's probably up to the Darwinist reviewers to check that...)

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:20

Replying to @nexttime:

It's also unclear to me whether that solves problems with R (?) on systems where there isn't a system-wide pkg-config.

(I guess if R sees there's a pkg-config, it relies on it, and only otherwise tries to find libraries by itself. But that's just a guess, and it's probably up to the Darwinist reviewers to check that...)

Hmmm, the only difference I see on a MacOS X system without a system-wide pkg-config is:

92c92
< checking for pkg-config... no
---
> checking for pkg-config... /Users/leif/Sage/sage-6.2.beta7/local/bin/pkg-config
497c497,498
< configure: not checking for cairo as pkg-config is not present
---
> checking whether pkg-config knows about cairo and pango... no
> checking whether pkg-config knows about cairo... no

So apparently R does rely on pkg-config, and doesn't try to find libraries / packages by other means if there's no pkg-config at all.

Still, it would be best to tell pkgconf also which system folders to search (in case there's no system-wide pkg-config), but that's even more kind of reinventing the wheel; the by far easiest (and IMHO most straight-forward) solution is to just make pkg-config a Sage prerequisite. (IIRC there are pre-built binaries available for MacOS X as well; Linux and SunOS aren't a problem either.)

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

46451c7

fix permisions, reviewer patches

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from ef9b2fb to 46451c7

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

Reviewer: Leif Leonhardy

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:22

Replying to @sagetrac-git:

Branch pushed to git repo; I updated commit sha1. New commits:

46451c7

fix permisions, reviewer patches

In the generated script, we have to export the env vars.

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:23

I still wonder whether we should install our pkg-config at all if a system-wide one is already present (as William suggested, i.e., don't install it in that case), unless perhaps SAGE_INSTALL_PKGCONFIG=yes. (I know you do like such environment variables very much.)

In the latter (hopefully rarely used or hardly necessary) case, we might even run something like

pc_folders_to_search=`find / -name \*.pc -exec dirname {} \; 2>/dev/null | sort -u`

(probably limiting the root folder(s) to the usual suspectives, depending on the OS, and excluding [unrelated] Sage folders).

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:24

Replying to @nexttime:

In the latter (hopefully rarely used or hardly necessary) case, we might even run something like

pc_folders_to_search=`find / -name \*.pc -exec dirname {} \; 2>/dev/null | sort -u`

(probably limiting the root folder(s) to the usual suspectives, depending on the OS, and excluding [unrelated] Sage folders).

Once, upon installation, of course; putting the result somewhere into local/var/lib/....

[vbraun]

comment:25

IMHO its better to have a definitive behavior when calling $SAGE_LOCAL/bin/pkg-config than hoping that the environment variables have been correctly set up.

Just searching all directories is wrong, this would find cross-compile toolchains etc.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

afe001f

export variables

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 46451c7 to afe001f

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:27

Replying to @vbraun:

IMHO its better to have a definitive behavior when calling $SAGE_LOCAL/bin/pkg-config than hoping that the environment variables have been correctly set up.

Just searching all directories is wrong, this would find cross-compile toolchains etc.

Sure, but we should recommend to install a system-wide version, and probably document PKG_CONFIG_PATH even for those who don't have one. (But that IMHO doesn't have to happen here and now.)

(I think unfortunately nobody reads the output of Sage's configure unless it bails out, so we'd have to document that elsewhere, or use sleep... ;-) )

[kiwifb]

comment:28

I like the fact that you ended up moving a bit of sage-env in the script. As for the configure script, it should bail out if a hard requirement is not met that's part of its function.

I agree with Volker that scanning for .pc is a bad idea, apart from his reasons it can take ages, especially if you have nfs share mounted. I certainly would not want to try that on my power7 cluster which has 170TB of space on a remote file system.

I thought we could have gone gcc (spkg that is) style on this one, William's idea really. But I am fine with the proposed solution.

[vbraun]

comment:29

Hmm PKG_CONFIG_LIBDIR also has the effect of not searching the compiled-in default path any more, removed.

I've added a spkg-check that will try to find the system pc files if we can guess their location.

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from afe001f to 0afd5c5

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

0afd5c5

fix search path, add tests

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:31

Hmmm, for whatever reason (shell caching I presume), I end up in an infinite loop.

Although $SAGE_LOCAL/bin/pkg-config gets deleted right before m4 is called, command -v pkg-config apparently still gives $SAGE_LOCAL/bin/pkg-config, such that the script finally calls itself...

FWIW,

$ bash --version
GNU bash, version 4.1.5(1)-release (x86_64-pc-linux-gnu)
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>

(I've run into similar a couple of times; the shell doesn't "see" freshly installed programs immediately.)

So it'd probably be better to call PATH="$SAGE_ORIG_PATH" command -v pkg-config (modifying sage-env accordingly, i.e., saving the original PATH).

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:32

Ah, yes:

       If  the name is neither a shell function nor a builtin, and contains no
       slashes, bash searches each element of the PATH for  a  directory  con‐
       taining  an  executable  file  by that name.  Bash uses a hash table to
       remember the full pathnames of executable files (see hash  under  SHELL
       BUILTIN  COMMANDS  below).  A full search of the directories in PATH is
       performed only if the command is not found in the hash table. [...]

(And subshells inherit the cache.)

diff --git a/build/pkgs/pkgconf/spkg-install b/build/pkgs/pkgconf/spkg-install
index e31009b..166008e 100755
--- a/build/pkgs/pkgconf/spkg-install
+++ b/build/pkgs/pkgconf/spkg-install
@@ -38,8 +38,17 @@ if [ $? -ne 0 ]; then
 fi

 # pkgconf is an alternative to the "official" pkg-config, and does not
-# automatically install a "pkg-config" binary.
-rm -f "$SAGE_LOCAL/bin/pkg-config"   # delete old version
+# automatically install a "pkg-config" binary, so we used to install
+# a symbolic link (now replaced by a wrapper script).
+rm -f "$SAGE_LOCAL/bin/pkg-config"   # delete old version (script or symlink)
+
+# Let bash forget where it previously saw 'pkg-config', otherwise we might
+# end up in an infinite loop if 'command -v pkg-config' gives Sage's script,
+# such that the below generated script finally calls itself:
+builtin hash -d pkg-config 2>/dev/null
+
+# Generate a wrapper script which either calls Sage's 'pkgconf' or a system-
+# wide 'pkg-config' if present (hardcoding its location at creation time):
 m4 ../patches/pkg-config.in > "$SAGE_LOCAL/bin/pkg-config"
 if [ $? -ne 0 ]; then
     echo >&2 "Error creating the pkg-config script."

doesn't yet work for me...

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:33

Ah, no, it's even weirder... Take a close look at

m4 ../patches/pkg-config.in > "$SAGE_LOCAL/bin/pkg-config"

Guess what happens...

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:34

... although it shouldn't be executable yet (but that also depends on the umask).

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:35

Hmmm, now works for me with

diff --git a/build/pkgs/pkgconf/patches/pkg-config.in b/build/pkgs/pkgconf/patches/pkg-config.in
index ccda223..4a00741 100644
--- a/build/pkgs/pkgconf/patches/pkg-config.in
+++ b/build/pkgs/pkgconf/patches/pkg-config.in
@@ -14,5 +14,5 @@ fi
 dnl Launch system pkg-config or our own pkgconf
 define(NEWLINE,`
 ')
-define(PKG_CONFIG_COMMAND, translit(esyscmd(`command -v pkg-config'), NEWLINE))
+define(PKG_CONFIG_COMMAND, translit(esyscmd(`bash -c "command -v pkg-config"'), NEWLINE))
 exec ifelse(sysval, `0', PKG_CONFIG_COMMAND, `"$SAGE_LOCAL/bin/pkgconf"') "$@"

(and the "forget" as before).

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

4067e95

avoid potential race when creating pkg-config

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 0afd5c5 to 4067e95

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:37

I have no idea what's going on here; with

# pkgconf is an alternative to the "official" pkg-config, and does not
# automatically install a "pkg-config" binary, so we used to install
# a symbolic link (now replaced by a wrapper script).
rm -f "$SAGE_LOCAL/bin/pkg-config"   # delete old version (script or symlink)

echo M4, shell\:
m4 <<"EOF"
translit(esyscmd(`command -v pkg-config'),`')
EOF
echo M4, bash\:
m4 <<"EOF"
translit(esyscmd(`bash -c "command -v pkg-config"'),`')
EOF

# Generate a wrapper script which either calls Sage's 'pkgconf' or a system-
# wide 'pkg-config' if present (hardcoding its location at creation time):
m4 ../patches/pkg-config.in > "$SAGE_LOCAL/bin/pkg-config"
if [ $? -ne 0 ]; then
    echo >&2 "Error creating the pkg-config script."
    exit 1
fi

echo Generated script:
echo =================
cat "$SAGE_LOCAL/bin/pkg-config"
echo =================

chmod 755 "$SAGE_LOCAL/bin/pkg-config"

(and without bash -c ... in pkg-config.in), I'm always getting:

...
/usr/bin/install -c -m 644 ./pkgconf.1 /foo/local/share/man/man1/pkgconf.1
M4, shell:
/usr/bin/pkg-config

M4, bash:
/usr/bin/pkg-config

Generated script:
=================
#!/usr/bin/env bash

if [ -z "$SAGE_LOCAL" ]; then
    echo >&2 "This script requires that SAGE_LOCAL is set."
    exit 1
fi  

if [ -z "$PKG_CONFIG_PATH" ]; then
    export PKG_CONFIG_PATH="$SAGE_LOCAL/lib/pkgconfig:$SAGE_LOCAL/share/pkgconfig"
else
    export PKG_CONFIG_PATH="$SAGE_LOCAL/lib/pkgconfig:$SAGE_LOCAL/share/pkgconfig:$PKG_CONFIG_PATH"
fi

exec /foo/local/bin/pkg-config "$@"
=================

real    0m14.221s
user    0m1.716s
sys 0m0.572s
Successfully installed pkgconf-0.9.4
Running the test suite for pkgconf-0.9.4...
+-- /usr/lib/pkgconfig found, using it for the pkg-config test.
    +-- fontconfig found, using it for the pkg-config test.
/foo/local/bin/pkg-config: line 16: /foo/local/bin/pkg-config: Argument list too long
/foo/local/bin/pkg-config: line 16: /foo/local/bin/pkg-config: Success
Our pkg-config script failed to pick up fontconfig

real    0m8.012s
user    0m3.568s
sys 0m1.916s
************************************************************************
Error testing package pkgconf-0.9.4
************************************************************************
...

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:38

Replying to @sagetrac-git:

Branch pushed to git repo; I updated commit sha1. New commits:

4067e95

avoid potential race when creating pkg-config

Works for me:

...
M4, shell:
/usr/bin/pkg-config

M4, bash:
/usr/bin/pkg-config

Generated script:
=================
#!/usr/bin/env bash

if [ -z "$SAGE_LOCAL" ]; then
    echo >&2 "This script requires that SAGE_LOCAL is set."
    exit 1
fi  

if [ -z "$PKG_CONFIG_PATH" ]; then
    export PKG_CONFIG_PATH="$SAGE_LOCAL/lib/pkgconfig:$SAGE_LOCAL/share/pkgconfig"
else
    export PKG_CONFIG_PATH="$SAGE_LOCAL/lib/pkgconfig:$SAGE_LOCAL/share/pkgconfig:$PKG_CONFIG_PATH"
fi

exec /usr/bin/pkg-config "$@"
=================

real    0m3.969s
user    0m1.660s
sys 0m0.572s
Successfully installed pkgconf-0.9.4
Running the test suite for pkgconf-0.9.4...
+-- /usr/lib/pkgconfig found, using it for the pkg-config test.
    +-- fontconfig found, using it for the pkg-config test.
   +-- gtk+-3.0 not found, ignoring.
   +-- harfbuzz not found, ignoring.
    +-- x11 found, using it for the pkg-config test.
+-- /usr/lib64/pkgconfig found, using it for the pkg-config test.
    +-- fontconfig found, using it for the pkg-config test.
   +-- gtk+-3.0 not found, ignoring.
   +-- harfbuzz not found, ignoring.
    +-- x11 found, using it for the pkg-config test.
+-- /usr/share/pkgconfig found, using it for the pkg-config test.
   +-- fontconfig not found, ignoring.
   +-- gtk+-3.0 not found, ignoring.
   +-- harfbuzz not found, ignoring.
   +-- x11 not found, ignoring.

real    0m0.018s
user    0m0.000s
sys 0m0.000s

I still don't get what goes wrong or who's to blame otherwise.

Varying indentation of found vs. ignored .pc files is presumably not intentional... ;-)

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Branch pushed to git repo; I updated commit sha1. New commits:

1d3284e

fix indentation

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 4067e95 to 1d3284e

[83660e46-0051-498b-a8c1-f7a7bd232b5a]

comment:40

[still testing...]

Is it intentional that the test suite fails if no system-wide pkg-config is installed?

(I mean, of course it should "fail", but shouldn't we skip the test in that case?)

Imagine having set SAGE_CHECK=yes... (while having sage -f -c pkgconf fail would probably be ok)