Strange linking order for omalloc due to pkgconfig bug

[jpflori]

This is a follow up to #17254 which upgraded to singular 4:

The linking order for Singular libraries is determined by pkgconfig which gives

-lomalloc -lSingular -lpolys -lfactory -lresources

This does not look that right.

This is because pkgconfig uses sets that don't preserve the order of libraries (and include directories or macros etc...).

A solution is to migrate pkgconfig from sets to list.

matze/pkgconfig#10

has been opened upstream to explain the rational, along with a pull request to implement it at

matze/pkgconfig#11

This has now been accepted upstream and is part of a new release with a few other fixes.

Tarball: https://pypi.python.org/packages/9d/ba/80910bbed2b4e646a6adab4474d2e506744c260c7002a0e6b41ef8750d8d/pkgconfig-1.2.2.tar.gz

Upstream: Fixed upstream, in a later stable release.

Component: packages: standard

Author: François Bissey

Branch: bd61fc4

Reviewer: Jeroen Demeyer

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

[jdemeyer]

Replying to @jpflori:

This is a follow up to #17254 which upgraded to singular 4:

• strange linking order (omalloc comes before singular!)

You mean during the build of Singular itself or Sage modules using Singular?

[jpflori]

comment:2

See #17254 comment:510 :
"""
During linking some Sage files I see -lomalloc -lSingular -lpolys -lfactory -lresources which does not look that right.
"""

[kiwifb]

comment:4

That's an annoying defect that I have noticed before in the python interface to pkg-config. The libraries are put in a set and of course it is not preserving the order. It was already evident when using ATLAS for blas/lapack.

$ pkg-config --libs Singular
-lSingular -ldl -lpolys -lflint -lmpfr -lntl -lgmp -ldl -lfactory -lflint -lmpfr -lntl -lgmp -lomalloc -lresources 

but

$ python
Python 2.7.10 (default, Jun 12 2016, 19:30:46) 
[GCC 5.3.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import pkgconfig
>>> singular_pc = pkgconfig.parse('Singular')
>>> list(singular_pc['libraries'])
[u'omalloc', u'dl', u'Singular', u'polys', u'factory', u'flint', u'ntl', u'gmp', u'resources', u'mpfr']
>>> singular_pc['libraries']
set([u'omalloc', u'dl', u'Singular', u'polys', u'factory', u'flint', u'ntl', u'gmp', u'resources', u'mpfr'])

which is what you are seeing. If you still have a sage with ATLAS somewhere look at what you get for lapack. This needs to be fixed upstream so that something preserving the order is used instead of a set.

[jpflori]

comment:5

Is this upstream:

• https://github.com/matze/pkgconfig
  ?

It does not look very active, last release was three years ago.

[infinity0]

comment:6

Debian also thinks that is the homepage, yes.

[kiwifb]

comment:7

Well pkg-config is relatively stable. The nice thing about the set is of course that you don't have repeated elements in it. Is there an "ordered set" type in python?

[kiwifb]

comment:8

After a bit of googling I think upstream should either depend from an "ordered set" package or switch to a dictionary of list instead of a dictionary of set and then remove duplication.

[kiwifb]

comment:9

I have been working on making pkgconfig use lists instead of sets and then doing some deduplication.

There are two issues with deduplication - which was originally achieved by using sets:

• Is it really necessary, that makes longer lines but pkg-config doesn't do it in the first place. Which brings us to second issue
• Currently the first instance of everything is kept. Not a problem for macros, may be for include directories and library directories. Definitely a potential problem for libraries.

I don't think deduplication - potentially from the end of the list rather than the beginning of it, is worth it.

Opinion?

[jdemeyer]

comment:10

I agree with François: we shouldn't be smarter than necessary. There is nothing wrong with longer command lines (as long as it's not excessive).

[kiwifb]

comment:11

OK, I definitely think it is not worth the bother so I'll probably go with this branch for my upstream pull request https://github.com/kiwifb/pkgconfig/tree/listify. If anyone wants to comment, now is the time.

[kiwifb]

comment:12

Upstream accepted my change, hopefully that will come with a new release soon. I did some experiments, while we will be able to simplify sage's code to populate the various variables *_{libs,library_dirs,include_dir} it should work out of the box without modification (haven't checked cflags actually).

[kiwifb]

Upstream: Fixed upstream, but not in a stable release.

[kiwifb]

comment:13

Not sure when upstream will cut a release, should we patch in the meantime?

[jpflori]

comment:14

I say we patch.

[kiwifb]

comment:15

Working on this. Some adjustments are needed in modules_list.py.

[kiwifb]

comment:16

Now ready for review.

---

New commits:

9d4de4a	Add https://github.com/matze/pkgconfig/pull/11
65a1d2a	migrating numpy helper script to use list
0accd59	migrate module_list.py to use list from pkgconfig

[kiwifb]

Commit: 0accd59

[kiwifb]

Author: François Bissey

[kiwifb]

Branch: u/fbissey/pkgconfig

[jdemeyer]

comment:17

For reference, please add a link to the upstream ticket in the patch file and in the ticket description.

[kiwifb]

comment:34

Was thinking about that. I am not satisfied with the branch as it is anyway. I will hammer some more on pkgconfig side with another pull upstream as soon as I can. I'll probably revert my last commit but even so it is probably best if numpy is bumped as well.

[jdemeyer]

comment:35

If you cannot immediately fix the real issue, it's OK for me to merge this hack as-is.

I'm setting it back to needs_work because you still want to work on it.

[kiwifb]

comment:36

I have pushed a few more things upstream and requested a release. The changes I sent upstream makes my last commit unnecessary, so at the very least I'll retract it. If upstream makes a new release, I'll update the package instead of adding more patches.

[sagetrac-git]

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

971c9d4	upgrade pkgconfig to 1.2.2
b03524b	migrate to list as used by the new pkgconfig 1.2.2.

[sagetrac-git]

Changed commit from 9dd50b6 to b03524b

[kiwifb]

comment:38

OK, new release from upstream with all fixes included. Now the output is list, which keep the order, and is also strictly mirroring the output of the equivalent pkg-config command. The extra 2 minor releases are for various fixes to the tests that we don't use in sage but are important for me in Gentoo since I do run nose tests.

[kiwifb]

Changed upstream from Fixed upstream, but not in a stable release. to Fixed upstream, in a later stable release.

[jdemeyer]

comment:39

[comment:33]

[kiwifb]

comment:40

All right, I'll do that ASAP.

[sagetrac-git]

Changed commit from b03524b to bd61fc4

[sagetrac-git]

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

bd61fc4

bump numpy's version to make sure it rebuilds

[kiwifb]

comment:42

Done.

[vbraun]

Changed branch from u/fbissey/pkgconfig to bd61fc4

[jdemeyer]

Changed commit from bd61fc4 to none

[jdemeyer]

comment:46

Breakage: #22047