Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#28883 closed defect (fixed)

update pkgconfig to version 1.5.1

Reported by: Dima Pasechnik Owned by:
Priority: major Milestone: sage-9.1
Component: packages: standard Keywords:
Cc: François Bissey, Erik Bray, Tobias Hansen, Ximin Luo, Isuru Fernando, Matthias Köppe Merged in:
Authors: Dima Pasechnik Reviewers: Isuru Fernando, Erik Bray, Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: fb8f4e5 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Dima Pasechnik)

the current version is buggy, as we found out on #27870

also, pkgconfig switched to https://python-poetry.org/ from setuptools, so we either get poetry installed too (which is a big task), or go Debian way and provide setup.py. That's what they do in their (not really "orig") tarball on

http://deb.debian.org/debian/pool/main/p/python-pkgconfig/

Cf. https://salsa.debian.org/python-team/modules/python-pkgconfig/tree/master

The pypi tarball has setup.py, so we can just use it. tarball: https://files.pythonhosted.org/packages/6e/a9/ff67ef67217dfdf2aca847685fe789f82b931a6957a3deac861297585db6/pkgconfig-1.5.1.tar.gz

Change History (58)

comment:1 Changed 3 years ago by Dima Pasechnik

Description: modified (diff)

comment:2 Changed 3 years ago by Dima Pasechnik

Cc: François Bissey Erik Bray Tobias Hansen Ximin Luo added
Description: modified (diff)

I found adding setup.py into the "orig" tarball rather than into debian/patches rather unusual.

comment:3 Changed 3 years ago by Dima Pasechnik

Authors: Dima Pasechnik
Branch: u/dimpase/packages/pkgconfig151
Commit: e5c91dbb4d313d43f05cf3d33fc1758ac089613d
Description: modified (diff)

comment:4 Changed 3 years ago by François Bissey

That poetry stuff is really a replacement for pip. And from where I see it, it can only fully replace superficially simple setup.py files. And it probably can generate such simple setup.py file, although I cannot see that in the Readme. And to be able to do what it does, it must be able to process complicated setup.py just fine, since it will install stuff that was never prepared for poetry if I am not mistaken.

Anyway that's outside of the scope of the ticket. But one more of this things to potentially deal with :(

comment:5 Changed 3 years ago by Dima Pasechnik

Status: newneeds_review

I guess poetry generates setup.py files as a part of its pypi deployment feature.

comment:6 Changed 3 years ago by Emmanuel Charpentier

[ A stupidity. Sorry for the noise ... ]

Last edited 3 years ago by Emmanuel Charpentier (previous) (diff)

comment:7 in reply to:  6 Changed 3 years ago by Emmanuel Charpentier

Replying to charpent:

[ A stupidity. Sorry for the noise ... ]

[ After re-checking the setup... ] This doesn't seem to be effective. I still get the systemwide pkg-config:

charpent@zen-book-flip:/usr/local/sage-9$ git status
Sur la branche t/28883/packages/pkgconfig151
Votre branche est à jour avec 'trac/u/dimpase/packages/pkgconfig151'.

rien à valider, la copie de travail est propre
charpent@zen-book-flip:/usr/local/sage-9$ ./sage -sh

Starting subshell with Sage environment variables set.  Don't forget
to exit when you are done.  Beware:
 * Do not do anything with other copies of Sage on your system.
 * Do not use this for installing Sage packages using "sage -i" or for
   running "make" at Sage's root directory.  These should be done
   outside the Sage shell.

Bypassing shell configuration files...

Note: SAGE_ROOT=/usr/local/sage-9
(sage-sh) charpent@zen-book-flip:sage-9$ command -v pkg-config
/usr/bin/pkg-config
(sage-sh) charpent@zen-book-flip:sage-9$ pkg-config --version
0.29
(sage-sh) charpent@zen-book-flip:sage-9$ exit
exit
Exited Sage subshell.

comment:8 Changed 3 years ago by Dima Pasechnik

this is a Python interface to pkg-config. So you need to import pkgconfig in Sage's Python to see it in action.

comment:9 Changed 3 years ago by Dima Pasechnik

Cc: Isuru Fernando Matthias Köppe added

comment:10 Changed 3 years ago by Isuru Fernando

Reviewers: Isuru Fernando
Status: needs_reviewpositive_review

comment:11 Changed 3 years ago by Dima Pasechnik

Milestone: sage-9.1sage-9.0

comment:12 Changed 3 years ago by Volker Braun

Branch: u/dimpase/packages/pkgconfig151e5c91dbb4d313d43f05cf3d33fc1758ac089613d
Resolution: fixed
Status: positive_reviewclosed

comment:13 Changed 3 years ago by Volker Braun

Commit: e5c91dbb4d313d43f05cf3d33fc1758ac089613d
Resolution: fixed
Status: closednew

This breaks on OSX:

[sagelib-9.0.beta9] Traceback (most recent call last):
[sagelib-9.0.beta9]   File "setup.py", line 72, in <module>
[sagelib-9.0.beta9]     from module_list import ext_modules, library_order
[sagelib-9.0.beta9]   File "/Users/buildbot-sage/slave/sage_git/build/src/module_list.py", line 42, in <module>
[sagelib-9.0.beta9]     zlib_pc = pkgconfig.parse('zlib')
[sagelib-9.0.beta9]   File "/Users/buildbot-sage/slave/sage_git/build/local/lib/python3.7/site-packages/pkgconfig/pkgconfig.py", line 248, in parse
[sagelib-9.0.beta9]     _raise_if_not_exists(package)
[sagelib-9.0.beta9]   File "/Users/buildbot-sage/slave/sage_git/build/local/lib/python3.7/site-packages/pkgconfig/pkgconfig.py", line 103, in _raise_if_not_exists
[sagelib-9.0.beta9]     raise PackageNotFoundError(package)
[sagelib-9.0.beta9] pkgconfig.pkgconfig.PackageNotFoundError: zlib not found
[sagelib-9.0.beta9] ************************************************************************
[sagelib-9.0.beta9] Error building the Sage library
[sagelib-9.0.beta9] ************************************************************************

comment:14 Changed 3 years ago by Dima Pasechnik

probably buildbot had a workaround for the buggy package?

comment:15 Changed 3 years ago by Dima Pasechnik

isn't the Sage/Python? environment of that buildbot broken? One should have

sage: import pkgconfig
sage: pkgconfig.parse('zlib')
defaultdict(<class 'list'>, {'libraries': ['z']})
Version 0, edited 3 years ago by Dima Pasechnik (next)

comment:16 Changed 3 years ago by John Palmieri

It works for me on OS X 10.15.2.

comment:17 Changed 3 years ago by Volker Braun

Status: newneeds_review

Turns out #27870 is the culprit...

comment:18 Changed 3 years ago by Volker Braun

Status: needs_reviewpositive_review

comment:19 Changed 3 years ago by Volker Braun

Status: positive_reviewneeds_work

Oh just noticed that this ticket is merged into #27870

comment:20 Changed 3 years ago by Dima Pasechnik

To me, it looks like a Python with broken zlib module. Nothing to do with pkgconfig.

Yes, it could also be that local/lib/pkgconfig/ contents are somehow broken and thus break Python's pkgconfig.

Was it a build from scratch? If not, stale things in local/lib/pkgconfig/ may break stuff.

comment:21 Changed 3 years ago by Volker Braun

On Catalina, there is no zlib in /usr/lib/pkgconfig and we don't build our own zlib with #27870.

buildbot-sage@osx build % ./sage -sh -c 'pkgconf --cflags zlib'

Package zlib was not found in the pkg-config search path.
Perhaps you should add the directory containing `zlib.pc'
to the PKG_CONFIG_PATH environment variable
Package 'zlib', required by 'world', not found

All builds from scratch.

comment:22 Changed 3 years ago by Volker Braun

Status: needs_workpositive_review

The version bump here should be ok, its #27870 that is borked

comment:23 Changed 3 years ago by Dima Pasechnik

No, #27870 is fine.

what's "borked" is that the new pkgconfig v1.5.1 throws an error if there is no .pc file. E.g. on this branch one sees

sage: import pkgconfig
sage: pkgconfig.parse("uhohoh")
---------------------------------------------------------------------------
PackageNotFoundError                      Traceback (most recent call last)
<ipython-input-2-d927c49e8798> in <module>()
----> 1 pkgconfig.parse("uhohoh")

/home/dimpase/sage/local/lib/python3.7/site-packages/pkgconfig/pkgconfig.py in parse(packages, static)
    246     """
    247     for package in packages.split():
--> 248         _raise_if_not_exists(package)
    249 
    250     out = _query(packages, *_build_options('--cflags --libs', static=static))

/home/dimpase/sage/local/lib/python3.7/site-packages/pkgconfig/pkgconfig.py in _raise_if_not_exists(package)
    101 def _raise_if_not_exists(package):
    102     if not exists(package):
--> 103         raise PackageNotFoundError(package)
    104 
    105 

PackageNotFoundError: uhohoh not found
sage: 

but the previous version merrily returns some stuff:

sage: pkgconfig.parse("uhohoh")
defaultdict(<type 'list'>, {'define_macros': []})

All the systems this was tested apparently had zlib.pc somewhere, and so it worked. Your box does not have it, and so an error is thrown.

OK, would it be easier if this is fixed (trivially) on a followup ticket rather than here?

comment:24 Changed 3 years ago by Volker Braun

Status: positive_reviewneeds_work

Feel free to fix it here since there are already a number of tickets that depend on this one that can't be merged until the issue is resolved

comment:25 Changed 3 years ago by Volker Braun

imho the zlib spkg-configure.m4 should just ensure that a zlib.pc file can be found (i.e. use PKG_CHECK_MODULES)

comment:26 Changed 3 years ago by Dima Pasechnik

The whole thing about assuming that zlib.pc exists was iffy since #26286 - that's basically where the bug has slipped in.

It "worked" as the absense of zlib.pc was ignored by old Python's pkgconfig.

There is no actual need to insist on zlib.pc present, as Apple's implementation of zlib is good enough to be used.

comment:27 Changed 3 years ago by Volker Braun

Indeed its never actually necessary to have .pc files you can always kludge it by hand and fudge cflags/libs on all supported platforms. Its just not a sane/maintainable/scalable system of working with shared libraries.

If you want to save OSX users from having to build zlib then we should just provide an appropriate zlib.pc on that particular snowflake of a platform.

comment:28 Changed 3 years ago by Dima Pasechnik

I'll provide workaround here, and properly deal with generation on zlib.pc on #28906.

For the problem at hand it'd suffice just to avoid an expection, and fake the output of pkgconfig.parse() if need be.

comment:29 Changed 3 years ago by Dima Pasechnik

Status: needs_workneeds_review

This should fix the OSX issue, IMHO (can't test on OSX, though)

comment:30 Changed 3 years ago by Dima Pasechnik

Branch: e5c91dbb4d313d43f05cf3d33fc1758ac089613d

Oh blast, need to rename this branch...

comment:31 Changed 3 years ago by Dima Pasechnik

Branch: u/dimpase/packages/pkgconfig151
Commit: 9d33b7160485f78af7a0b8d99b413a948c2a77e8

New commits:

e5c91dbupdate pkgconfig to v1.5.1. See trac #28883
9d33b71allow absent zlib.pc

comment:32 Changed 3 years ago by Dima Pasechnik

potentially, there could also be a problem with libpng.pc, for which ./configure does not check for/does not insist on its presense. But it comes before zlib.pc is tested, so that's apparently not blocking. I'll change #28906 to deal with these too.

Last edited 3 years ago by Dima Pasechnik (previous) (diff)

comment:33 Changed 3 years ago by Erik Bray

Status: needs_reviewneeds_work

That should be pkgconfig.PackageNotFoundError, otherwise you'll just get a NameError raised during exception handling. Please fix that and rebase to squash the fix.

comment:34 Changed 3 years ago by John Palmieri

I'm a little confused here. On OS X, when I run the command in comment:21 (./sage -sh -c 'pkgconf --cflags zlib'), I get the same output as given there, but the build succeeds for me. Why does it fail on some OS X systems and succeed on others? (It worked for me with the current branch but also the earlier one.)

comment:35 in reply to:  34 Changed 3 years ago by Dima Pasechnik

Replying to jhpalmieri:

I'm a little confused here. On OS X, when I run the command in comment:21 (./sage -sh -c 'pkgconf --cflags zlib'), I get the same output as given there, but the build succeeds for me. Why does it fail on some OS X systems and succeed on others? (It worked for me with the current branch but also the earlier one.)

How about the commands in comment:15 ?

sage: import pkgconfig
sage: pkgconfig.parse('zlib')

comment:36 Changed 3 years ago by John Palmieri

With the current branch:

sage:  import pkgconfig
sage: pkgconfig.parse('zlib')
defaultdict(<class 'list'>, {'libraries': ['z']})

comment:37 Changed 3 years ago by Dima Pasechnik

From reading https://github.com/matze/pkgconfig/blob/master/pkgconfig/pkgconfig.py it could happen that you have environment variable PKG_CONFIG defined, and its value will be used instead of pkg-config in your PATH.

https://github.com/matze/pkgconfig/blob/a7108a65625a529dc1cf1b4779d23e5701f202dc/pkgconfig/pkgconfig.py#L108

comment:38 Changed 3 years ago by John Palmieri

When I run Sage, it defines the environment variable

PKG_CONFIG_PATH=.../path/to/SAGE_ROOT/local/lib/pkgconfig

but I don't have anything similar set myself. export | grep PKG returns nothing.

comment:39 Changed 3 years ago by Dima Pasechnik

Is the output of

sage: import os
sage: os.system("which pkg-config")

the same as of which pkg-config at ./sage -sh prompt?

As well, please compare the value of env. var. PKG_CONFIG_PATH at the latter prompt with the output of

sage: os.environ.get('PKG_CONFIG_PATH', None)

comment:40 Changed 3 years ago by John Palmieri

which pkg-config points to the homebrew installation, /usr/local/bin/pkg-config in both invocations.

sage: os.environ.get('PKG_CONFIG_PATH', None)
'/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/pkgconfig'
sage: sage.env.var('PKG_CONFIG_PATH')
sage:

and after running sage --sh:

$ echo $PKG_CONFIG_PATH
/Users/palmieri/Desktop/Sage_stuff/git/sage/local/lib/pkgconfig

('/Users/palmieri/Desktop/Sage_stuff/git/sage/` is SAGE_ROOT.)

Last edited 3 years ago by John Palmieri (previous) (diff)

comment:41 Changed 3 years ago by John Palmieri

So maybe it's the presence of the homebrew pkg-config that makes this system behave differently from Volker's buildbot.

comment:42 Changed 3 years ago by Isuru Fernando

You probably have zlib installed from homebrew.

What do you get with pkg-config zlib --variable pcfiledir ?

comment:43 Changed 3 years ago by Dima Pasechnik

Sage's Python has to dig up zlib.pc to read by pkgconfig.parse(), from somewhere.

By the way, I don't understand pkgconf in ./sage -sh -c 'pkgconf --cflags zlib ???

it should be pkg-config

I guess pkgconf is some "unofficial" name for pkg-config, used by Sage's pkgconf package, which installs pkg-config.

Last edited 3 years ago by Dima Pasechnik (previous) (diff)

comment:44 Changed 3 years ago by Isuru Fernando

pkgconf and pkg-config are two different programs both of which does the same thing. pkgconf has a pkg-config symlink as well. (Think of pkgconf as MPIR built with GMP compat and pkg-config as GMP.)

comment:45 Changed 3 years ago by Dima Pasechnik

So you were confused by Volker's pkgconf invocation --- sorry, I should have noticed it earlier. I bet ./sage -sh -c 'pkg-config --libs zlib has meaningful output with -lz in it.

comment:46 in reply to:  42 ; Changed 3 years ago by John Palmieri

Replying to isuruf:

You probably have zlib installed from homebrew.

What do you get with pkg-config zlib --variable pcfiledir ?

/usr/local/Homebrew/Library/Homebrew/os/mac/pkgconfig/10.15. Also

$ ./sage -sh -c 'pkg-config --libs zlib'
-lz

comment:47 Changed 3 years ago by Isuru Fernando

What does pkg-config zlib --variable libdir give you?

comment:48 in reply to:  46 Changed 3 years ago by Dima Pasechnik

Replying to jhpalmieri:

Replying to isuruf:

You probably have zlib installed from homebrew.

What do you get with pkg-config zlib --variable pcfiledir ?

/usr/local/Homebrew/Library/Homebrew/os/mac/pkgconfig/10.15. Also

$ ./sage -sh -c 'pkg-config --libs zlib'
-lz

so it appears that you use zlib provided by Homebrew, which does supply zlib.pc, so everything works as it should. (I don't know whether they install a real zlib, or just provide zlib.pc pointing to the MacOS native one, probably the former)

comment:49 in reply to:  47 Changed 3 years ago by John Palmieri

Replying to isuruf:

What does pkg-config zlib --variable libdir give you?

/usr/lib

comment:50 Changed 3 years ago by François Bissey

For information on PEP517 and the whole poetry business of packaging https://blogs.gentoo.org/mgorny/2019/12/24/handling-pep-517-pyproject-toml-packages-in-gentoo/

comment:51 Changed 3 years ago by git

Commit: 9d33b7160485f78af7a0b8d99b413a948c2a77e8809abd91a81beddb3f89a514ddccc904cc1713ea

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

809abd9added missing module name

comment:52 Changed 3 years ago by git

Commit: 809abd91a81beddb3f89a514ddccc904cc1713eafb8f4e51d258ce6cfa8e4b221ff67c94cf80467e

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

f74f554update pkgconfig to v1.5.1. See trac #28883
fb8f4e5allow absent zlib.pc

comment:53 Changed 3 years ago by Dima Pasechnik

Reviewers: Isuru FernandoIsuru Fernando, Erik Bray
Status: needs_workneeds_review

I've tried #28906 but it is ugly as hell, so not sure whether it should be getting in. So this is the minimal (rebased) fix.

comment:54 Changed 3 years ago by Matthias Köppe

Reviewers: Isuru Fernando, Erik BrayIsuru Fernando, Erik Bray, Matthias Koeppe
Status: needs_reviewpositive_review

Looks good and works well on macOS Catalina - after fixing #28676.

comment:55 Changed 3 years ago by Dima Pasechnik

Milestone: sage-9.0sage-9.1

comment:56 Changed 3 years ago by Volker Braun

Branch: u/dimpase/packages/pkgconfig151fb8f4e51d258ce6cfa8e4b221ff67c94cf80467e
Resolution: fixed
Status: positive_reviewclosed

comment:57 Changed 2 years ago by Matthias Köppe

Commit: fb8f4e51d258ce6cfa8e4b221ff67c94cf80467e

Follow-up on the poetry issue at #29810

comment:58 Changed 2 years ago by Dima Pasechnik

"...and other forms of boredom advertised as poetry."

Note: See TracTickets for help on using tickets.