Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

generate libpng.pc, zlib.pc if needed #28906

Open
dimpase opened this issue Dec 22, 2019 · 27 comments
Open

generate libpng.pc, zlib.pc if needed #28906

dimpase opened this issue Dec 22, 2019 · 27 comments

Comments

@dimpase
Copy link
Member

dimpase commented Dec 22, 2019

libpng.pc, gdlib.pc, zlib.pcetc
are needed in the calls of pkgconfig.parse in src/module_list.py

some perfectly fine implementations of zlib etc. don't install zlib.pc etc. (e.g. MacOS does not).

Since #26286 this problem was sleeping, until we updated pkgconfig on #28883, which made it throw an error on absense of zlib.pc.

This ticket provides generation of zlib.pc and libpng.pc, among with some fly-by refactoring.

CC: @kiwifb @embray @isuruf @mkoeppe

Component: build: configure

Author: Dima Pasechnik

Branch/Commit: u/dimpase/packages/missingpcs @ ba7d963

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

@dimpase dimpase added this to the sage-9.0 milestone Dec 22, 2019
@dimpase

This comment has been minimized.

@dimpase dimpase changed the title generate zlib.pc if needed, or avoid using it. generate libpng.pc, gdlib.pc, zlib.pc if needed Dec 23, 2019
@dimpase

This comment has been minimized.

@dimpase dimpase changed the title generate libpng.pc, gdlib.pc, zlib.pc if needed generate libpng.pc, zlib.pc if needed Dec 23, 2019
@dimpase
Copy link
Member Author

dimpase commented Dec 23, 2019

comment:3

The biggest question I here is how to find the full path to libz located by AC_CHECK_LIB or other similar macro.
This path has to be written into zlib.pc.

I don't quite know: I can link an executable with libz and analyse it with
readelf or otool or a similar thing, but it's not platform independent,
and I could not find a ready-made macro (I could change AC_CHECK_LIB, sure...)

@dimpase
Copy link
Member Author

dimpase commented Dec 23, 2019

Commit: 73d0619

@dimpase
Copy link
Member Author

dimpase commented Dec 23, 2019

Author: Dima Pasechnik

@dimpase
Copy link
Member Author

dimpase commented Dec 23, 2019

Branch: u/dimpase/packages/missingpcs

@dimpase
Copy link
Member Author

dimpase commented Dec 23, 2019

New commits:

ad511a1simplify some spkg-configures (use new macro)
73d0619refactor zlib's spkg-configure, start on a macro for zlib.pc

@isuruf
Copy link
Member

isuruf commented Dec 23, 2019

comment:5

You can use a minimal pkgconfig file like below which doesn't require the full path to the library.

Name: Foo
Description: description for foo
Version: 1.0.0
Libs: -lfoo

@dimpase
Copy link
Member Author

dimpase commented Dec 24, 2019

comment:6

well, a minimal pkgconfig like this would be just a bug in waiting to bite.

I can think of one platform-independent way (well, it needs dladdr, which is non-POSIX, but BSD, something that is available on most Unices, certainly on Linux and MacOS)

Write a C function that calls dlopen() on the library in question, and then
calls dladdr() on a symbol from the library; one of parts of the returned data is the full path to the shared object where the symbol is located.
That's a hell of a macro call to AC_RUN_IFELSE though :-)

@dimpase
Copy link
Member Author

dimpase commented Dec 25, 2019

comment:8

Could someone please try if the following works on MacOS, and if yes, what's the output

/* save into zt.c and run as follows:
  $ cc -o zt zt.c -ldl && ./zt

  On my Debian Linux box it outputs
  /usr/lib/x86_64-linux-gnu/libz.so
*/
#include <stdio.h>
#define __USE_GNU
#include <dlfcn.h>
#ifdef __APPLE__
#define EXT "dylib"
#else
#define EXT "so"
#endif
int main() {
void *z;
Dl_info  info;
int res;
void *ha;
z = dlopen("libz."EXT,  RTLD_LAZY); /* load zlib */
ha = dlsym(z, "inflateEnd");       /* get address of inflateEnd in libz */
res = dladdr(ha, &info);           /* get info for the function */
printf("%s\n", info.dli_fname);
return res;
}

@kiwifb
Copy link
Member

kiwifb commented Dec 25, 2019

comment:9

Replying to @dimpase:

Could someone please try if the following works on MacOS, and if yes, what's the output

fbissey@itsv-frb15 GitHub % uname -a
Darwin itsv-frb15.lan 19.2.0 Darwin Kernel Version 19.2.0: Sat Nov  9 03:47:04 PST 2019; root:xnu-6153.61.1~20/RELEASE_X86_64 x86_64
fbissey@itsv-frb15 GitHub % cc -o zt zt.c -ldl && ./zt
/usr/lib/libz.1.dylib

@isuruf
Copy link
Member

isuruf commented Dec 25, 2019

comment:10

I suggest linking the executable zt to libz so that the correct shared library is loaded.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 25, 2019

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

6bb85c4implemented generating input data for zlib.pc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 25, 2019

Changed commit from 73d0619 to 6bb85c4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 26, 2019

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

80a23acimplemented generating zlib.pc

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 26, 2019

Changed commit from 6bb85c4 to 80a23ac

@dimpase
Copy link
Member Author

dimpase commented Dec 26, 2019

comment:13

This branch is so far only for zlib.pc

Any obvious things I missed?

Naturally, I'd appreciate it tested on MacOS.

For libpng.pc I'll probably refactor that, to make computing abstolute path to libpng/libz/libWhatever a macro.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 27, 2019

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

2d4a1cdrefactor finding absolute path to dylib into macro
ba7d963do not try to recongnise libpng without pkg-config

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Dec 27, 2019

Changed commit from 80a23ac to ba7d963

@dimpase
Copy link
Member Author

dimpase commented Dec 27, 2019

comment:15

this is a result - ugly as hell. I am not sure whether this should get in.

@dimpase dimpase modified the milestones: sage-9.0, sage-9.1 Dec 27, 2019
@embray
Copy link
Contributor

embray commented Dec 30, 2019

comment:17

Replying to @dimpase:

well, a minimal pkgconfig like this would be just a bug in waiting to bite.

Would it though? Why exactly do we require pkgconfig here at all? It's a convenience sure, but on systems that don't install .pc files for some libraries we can still get the required information another way...

I mean, if autoconf could detect a system zlib in the first place, then I don't see why would need the absolute path to the SO file itself. All that should be needed to link against that library are whatever compiler/linker flags (e.g. -lz) that autoconf used to detect usability of the library in the first place.

@dimpase
Copy link
Member Author

dimpase commented Dec 30, 2019

comment:18

Replying to @embray:

Replying to @dimpase:

well, a minimal pkgconfig like this would be just a bug in waiting to bite.

Would it though? Why exactly do we require pkgconfig here at all? It's a convenience sure, but on systems that don't install .pc files for some libraries we can still get the required information another way...

I am thinking of a library trying to detect zlib by calling pkg-config rather than
using autoconf.

IMHO it's better not to install a pc file at all rather than install a rather bogus one. That's why I prefer how it's done on #28883.

I mean, if autoconf could detect a system zlib in the first place, then I don't see why would need the absolute path to the SO file itself. All that should be needed to link against that library are whatever compiler/linker flags (e.g. -lz) that autoconf used to detect usability of the library in the first place.

@embray
Copy link
Contributor

embray commented Dec 30, 2019

comment:19

It's not necessarily "bogus". But I agree--I think what you did in #28883 is more in line with what I was trying to say here.

@isuruf
Copy link
Member

isuruf commented Dec 31, 2019

comment:20

I don't see the difference in #28883 and a minimal .pc file as in #28906 comment:5 . They do the same thing.

@dimpase
Copy link
Member Author

dimpase commented Dec 31, 2019

comment:21

Replying to @isuruf:

I don't see the difference in #28883 and a minimal .pc file as in #28906 comment:5 . They do the same thing.

yes, but they do so by different means, and a pc file is much more likely to trip up someone.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 24, 2020

comment:22

I don't know if this ticket is still needed; but if it is, then it should be revised along the lines of #29082 -- do not write into SAGE_LOCAL at configure time; instead generate a rule in build/make/Makefile that generates the pc file there.

@dimpase
Copy link
Member Author

dimpase commented Mar 24, 2020

comment:23

I don't recall right now all the details, why I thought it's needed. Anyhow, perhaps as a concept of how to generate *.pc files, e.g. for openblas.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants