Opened 23 months ago

Closed 22 months ago

Last modified 17 months ago

#28883 closed defect (fixed)

update pkgconfig to version 1.5.1

Reported by: dimpase Owned by:
Priority: major Milestone: sage-9.1
Component: packages: standard Keywords:
Cc: fbissey, embray, thansen, infinity0, isuruf, mkoeppe 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 dimpase)

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 23 months ago by dimpase

  • Description modified (diff)

comment:2 Changed 23 months ago by dimpase

  • Cc fbissey embray thansen infinity0 added
  • Description modified (diff)

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

comment:3 Changed 23 months ago by dimpase

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

comment:4 Changed 23 months ago by fbissey

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 23 months ago by dimpase

  • Status changed from new to needs_review

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

comment:6 follow-up: Changed 23 months ago by charpent

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

Last edited 23 months ago by charpent (previous) (diff)

comment:7 in reply to: ↑ 6 Changed 23 months ago by charpent

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 23 months ago by dimpase

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 23 months ago by dimpase

  • Cc isuruf mkoeppe added

comment:10 Changed 23 months ago by isuruf

  • Reviewers set to Isuru Fernando
  • Status changed from needs_review to positive_review

comment:11 Changed 23 months ago by dimpase

  • Milestone changed from sage-9.1 to sage-9.0

comment:12 Changed 22 months ago by vbraun

  • Branch changed from u/dimpase/packages/pkgconfig151 to e5c91dbb4d313d43f05cf3d33fc1758ac089613d
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:13 Changed 22 months ago by vbraun

  • Commit e5c91dbb4d313d43f05cf3d33fc1758ac089613d deleted
  • Resolution fixed deleted
  • Status changed from closed to new

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 22 months ago by dimpase

probably buildbot had a workaround for the buggy package?

comment:15 Changed 22 months ago by dimpase

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']})
Last edited 22 months ago by dimpase (previous) (diff)

comment:16 Changed 22 months ago by jhpalmieri

It works for me on OS X 10.15.2.

comment:17 Changed 22 months ago by vbraun

  • Status changed from new to needs_review

Turns out #27870 is the culprit...

comment:18 Changed 22 months ago by vbraun

  • Status changed from needs_review to positive_review

comment:19 Changed 22 months ago by vbraun

  • Status changed from positive_review to needs_work

Oh just noticed that this ticket is merged into #27870

comment:20 Changed 22 months ago by dimpase

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 22 months ago by vbraun

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 22 months ago by vbraun

  • Status changed from needs_work to positive_review

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

comment:23 Changed 22 months ago by dimpase

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 22 months ago by vbraun

  • Status changed from positive_review to needs_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 22 months ago by vbraun

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 22 months ago by dimpase

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 22 months ago by vbraun

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 22 months ago by dimpase

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 22 months ago by dimpase

  • Status changed from needs_work to needs_review

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

comment:30 Changed 22 months ago by dimpase

  • Branch e5c91dbb4d313d43f05cf3d33fc1758ac089613d deleted

Oh blast, need to rename this branch...

comment:31 Changed 22 months ago by dimpase

  • Branch set to u/dimpase/packages/pkgconfig151
  • Commit set to 9d33b7160485f78af7a0b8d99b413a948c2a77e8

New commits:

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

comment:32 Changed 22 months ago by dimpase

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 22 months ago by dimpase (previous) (diff)

comment:33 Changed 22 months ago by embray

  • Status changed from needs_review to needs_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 follow-up: Changed 22 months ago by 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.)

comment:35 in reply to: ↑ 34 Changed 22 months ago by dimpase

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 22 months ago by jhpalmieri

With the current branch:

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

comment:37 Changed 22 months ago by dimpase

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 22 months ago by jhpalmieri

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 22 months ago by dimpase

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 22 months ago by jhpalmieri

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 22 months ago by jhpalmieri (previous) (diff)

comment:41 Changed 22 months ago by jhpalmieri

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

comment:42 follow-up: Changed 22 months ago by isuruf

You probably have zlib installed from homebrew.

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

comment:43 Changed 22 months ago by dimpase

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

By the way, I don't understand what 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 install pkg-config.

Version 0, edited 22 months ago by dimpase (next)

comment:44 Changed 22 months ago by isuruf

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 22 months ago by dimpase

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 ; follow-up: Changed 22 months ago by 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

comment:47 follow-up: Changed 22 months ago by isuruf

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

comment:48 in reply to: ↑ 46 Changed 22 months ago by dimpase

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 22 months ago by jhpalmieri

Replying to isuruf:

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

/usr/lib

comment:50 Changed 22 months ago by fbissey

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 22 months ago by git

  • Commit changed from 9d33b7160485f78af7a0b8d99b413a948c2a77e8 to 809abd91a81beddb3f89a514ddccc904cc1713ea

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

809abd9added missing module name

comment:52 Changed 22 months ago by git

  • Commit changed from 809abd91a81beddb3f89a514ddccc904cc1713ea to fb8f4e51d258ce6cfa8e4b221ff67c94cf80467e

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 22 months ago by dimpase

  • Reviewers changed from Isuru Fernando to Isuru Fernando, Erik Bray
  • Status changed from needs_work to needs_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 22 months ago by mkoeppe

  • Reviewers changed from Isuru Fernando, Erik Bray to Isuru Fernando, Erik Bray, Matthias Koeppe
  • Status changed from needs_review to positive_review

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

comment:55 Changed 22 months ago by dimpase

  • Milestone changed from sage-9.0 to sage-9.1

comment:56 Changed 22 months ago by vbraun

  • Branch changed from u/dimpase/packages/pkgconfig151 to fb8f4e51d258ce6cfa8e4b221ff67c94cf80467e
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:57 Changed 17 months ago by mkoeppe

  • Commit fb8f4e51d258ce6cfa8e4b221ff67c94cf80467e deleted

Follow-up on the poetry issue at #29810

comment:58 Changed 17 months ago by dimpase

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

Note: See TracTickets for help on using tickets.