#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: |
Description (last modified by )
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
- Description modified (diff)
comment:2 Changed 3 years ago by
- Cc fbissey embray thansen infinity0 added
- Description modified (diff)
comment:3 Changed 3 years ago by
- Branch set to u/dimpase/packages/pkgconfig151
- Commit set to e5c91dbb4d313d43f05cf3d33fc1758ac089613d
- Description modified (diff)
comment:4 Changed 3 years ago by
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
- 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: ↓ 7 Changed 3 years ago by
[ A stupidity. Sorry for the noise ... ]
comment:7 in reply to: ↑ 6 Changed 3 years ago by
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
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
- Cc isuruf mkoeppe added
comment:10 Changed 3 years ago by
- Reviewers set to Isuru Fernando
- Status changed from needs_review to positive_review
comment:11 Changed 3 years ago by
- Milestone changed from sage-9.1 to sage-9.0
comment:12 Changed 3 years ago by
- Branch changed from u/dimpase/packages/pkgconfig151 to e5c91dbb4d313d43f05cf3d33fc1758ac089613d
- Resolution set to fixed
- Status changed from positive_review to closed
comment:13 Changed 3 years ago by
- 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 3 years ago by
probably buildbot had a workaround for the buggy package?
comment:15 Changed 3 years ago by
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']})
comment:16 Changed 3 years ago by
It works for me on OS X 10.15.2.
comment:17 Changed 3 years ago by
- Status changed from new to needs_review
Turns out #27870 is the culprit...
comment:18 Changed 3 years ago by
- Status changed from needs_review to positive_review
comment:19 Changed 3 years ago by
- Status changed from positive_review to needs_work
Oh just noticed that this ticket is merged into #27870
comment:20 Changed 3 years ago by
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
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
- Status changed from needs_work to positive_review
The version bump here should be ok, its #27870 that is borked
comment:23 Changed 3 years ago by
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
- 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 3 years ago by
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
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
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
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
- Status changed from needs_work to needs_review
This should fix the OSX issue, IMHO (can't test on OSX, though)
comment:30 Changed 3 years ago by
- Branch e5c91dbb4d313d43f05cf3d33fc1758ac089613d deleted
Oh blast, need to rename this branch...
comment:31 Changed 3 years ago by
- Branch set to u/dimpase/packages/pkgconfig151
- Commit set to 9d33b7160485f78af7a0b8d99b413a948c2a77e8
comment:32 Changed 3 years ago by
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.
comment:33 Changed 3 years ago by
- 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: ↓ 35 Changed 3 years ago by
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
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
With the current branch:
sage: import pkgconfig sage: pkgconfig.parse('zlib') defaultdict(<class 'list'>, {'libraries': ['z']})
comment:37 Changed 3 years ago by
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.
comment:38 Changed 3 years ago by
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
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
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.)
comment:41 Changed 3 years ago by
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: ↓ 46 Changed 3 years ago by
You probably have zlib installed from homebrew.
What do you get with pkg-config zlib --variable pcfiledir
?
comment:43 Changed 3 years ago by
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
.
comment:44 Changed 3 years ago by
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
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: ↓ 48 Changed 3 years ago by
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: ↓ 49 Changed 3 years ago by
What does pkg-config zlib --variable libdir
give you?
comment:48 in reply to: ↑ 46 Changed 3 years ago by
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
comment:50 Changed 3 years ago by
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
- Commit changed from 9d33b7160485f78af7a0b8d99b413a948c2a77e8 to 809abd91a81beddb3f89a514ddccc904cc1713ea
Branch pushed to git repo; I updated commit sha1. New commits:
809abd9 | added missing module name
|
comment:52 Changed 3 years ago by
- Commit changed from 809abd91a81beddb3f89a514ddccc904cc1713ea to fb8f4e51d258ce6cfa8e4b221ff67c94cf80467e
comment:53 Changed 3 years ago by
- 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 3 years ago by
- 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 3 years ago by
- Milestone changed from sage-9.0 to sage-9.1
comment:56 Changed 3 years ago by
- Branch changed from u/dimpase/packages/pkgconfig151 to fb8f4e51d258ce6cfa8e4b221ff67c94cf80467e
- Resolution set to fixed
- Status changed from positive_review to closed
comment:57 Changed 2 years ago by
- Commit fb8f4e51d258ce6cfa8e4b221ff67c94cf80467e deleted
Follow-up on the poetry issue at #29810
comment:58 Changed 2 years ago by
"...and other forms of boredom advertised as poetry."
I found adding
setup.py
into the "orig" tarball rather than into debian/patches rather unusual.