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

pkg-config Requires.private and removal of static libraries #1880

Open
hmaarrfk opened this issue Jan 16, 2023 · 11 comments
Open

pkg-config Requires.private and removal of static libraries #1880

hmaarrfk opened this issue Jan 16, 2023 · 11 comments
Labels

Comments

@hmaarrfk
Copy link
Contributor

hmaarrfk commented Jan 16, 2023

Your question:

There has been a trend at conda-forge to remove static libraries and to split off runtime libraries from build time dependencies in order to save download space.

There has also been a combined trend to add pkgconfig files to enable more packages that were historically built only for linux, be able to get built for windows.

However, the inclusion of the Requires.private flag in the pkgconfig files makes it necessary to add back build time dependencies in order to "help the static builds" (see the last FAQ question in https://people.freedesktop.org/~dbn/pkg-config-guide.html)

I'm really not sure what to do about this. For example, many users have had to add back zlib as a host dependency (when their software doesn't link to it directly).

Second, the glib -> harfbuzz dependency is also a tough one to manage.

Thank you for your advice.

xref: conda-forge/zlib-feedstock#69

@leofang
Copy link
Member

leofang commented Jan 16, 2023

It's unclear to me why/how would pkgconfig files be useful to conda package users. Do we have any guideline that those must be retained? I don't think we have any pkgconfig test infrastructure set up to test their validity?

@hmaarrfk
Copy link
Contributor Author

Taking harfbuzz as an example package Isn't this command useful?

pkg-config --cflags harfbuzz

Generating the output:

-I/home/mark/mambaforge/envs/dev/include/harfbuzz -I/home/mark/mambaforge/envs/dev/include/freetype2 -I/home/mark/mambaforge/envs/dev/include/libpng16 -I/home/mark/mambaforge/envs/dev/include -I/home/mark/mambaforge/envs/dev/include/glib-2.0 -I/home/mark/mambaforge/envs/dev/lib/glib-2.0/include -I/home/mark/mambaforge/envs/dev/include

is inherently useful to me.

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Jan 16, 2023

The problem is that pkgconfig suggests that the file looks like:

prefix=/home/mark/mambaforge/envs/dev
includedir=${prefix}/include
libdir=${prefix}/lib

Name: harfbuzz
Description: HarfBuzz text shaping library
Version: 6.0.0
Requires.private: freetype2, graphite2, glib-2.0
Libs: -L${libdir} -lharfbuzz
Libs.private: -pthread -lm
Cflags: -I${includedir}/harfbuzz

whereas I think that for conda-forge, where we encourage dynamic linking, we should encourage the removal of the line Requires.Private.

Editing the file harfbuzz.pc so that it looks like

prefix=/home/mark/mambaforge/envs/dev
includedir=${prefix}/include
libdir=${prefix}/lib

Name: harfbuzz
Description: HarfBuzz text shaping library
Version: 6.0.0
# Requires.private: freetype2, graphite2, glib-2.0
Libs: -L${libdir} -lharfbuzz
Libs.private: -pthread -lm
Cflags: -I${includedir}/harfbuzz

makes the flags provided by pkg-config much more concise since the end user doesn't need to link to freetype2 themselves:

pkg-config --cflags harfbuzz
-I/home/mark/mambaforge/envs/dev/include/harfbuzz

@carterbox
Copy link
Member

carterbox commented Jan 16, 2023

Removal of static libraries is policy. https://github.com/conda-forge/cfep/blob/main/cfep-18.md

We already patch packages in order to devendor dependencies and ensure that artifacts are installed into the conda-environment correctly and avoid building static libs. Patching pc files to prevent static/unused linkages seems like a routine and reasonable thing.

There is probably an easy call to sed that can be added at the end of a build script to remove any lines beginning with Requires.private Something like:

sed -i '/Requires.private:/d' ./example.pc

@hmaarrfk
Copy link
Contributor Author

3 challenges:

  1. Windows equivalent command? It would be great if this could work without any additional dependencies (I know this is hard).
  2. OSX Equivalent command? sed -i doesn't do the same thing on OSX.
  3. There are some libraries where it was decided to explicitly go against that policy: https://github.com/conda-forge/zlib-feedstock/blob/main/recipe/meta.yaml#L105 Disable static libraries zlib-feedstock#64 (comment) Zlib is only an example, but would be OK to break pkg-config in such situations?

@carterbox
Copy link
Member

1.2. https://github.com/uutils/coreutils is promising, but they haven't implemented sed yet. Otherise, roll your own python script? 🤷🏼 😄

  1. zlib doesn't follow the policy "to the letter", but it does follow the spirit. "to the letter" would be putting the static libs in a libzlib-static package, but in the spirit of the policy, they are separated from the dynamic libraries in zlib. Downstream packages can link to libzlib without getting the static libs by accident.

@isuruf
Copy link
Member

isuruf commented Jan 17, 2023

This is a bug in pkg-config, but pkg-config is not actively maintained anymore AFAIK. We should change to the new actively maintained implementation pkgconf and let that provide pkg-config

See also https://fedoraproject.org/wiki/Changes/pkgconf_as_system_pkg-config_implementation

@hmaarrfk hmaarrfk changed the title pkgconfig Requires.private and removal of static libraries pkg-config Requires.private and removal of static libraries Jan 17, 2023
@jakirkham
Copy link
Member

Just following up on this question

OSX Equivalent command? sed -i doesn't do the same thing on OSX.

Would use sed -i.bak. Basically sed on macOS won't do in-place writing without a backup file being written. sed -i.bak says add a .bak extension/suffix to the file in question and keep that as a backup. Can do this in the source directory (to keep these files from showing up in the package accidentally).

Would also note this behavior works fine on Linux. So can be used on all Unix-y platforms.

Lots of examples of this in conda-forge including the build scripts of python.

@pkgw
Copy link
Contributor

pkgw commented Jan 30, 2023

I'm not sure what exact bug @isuruf is referring to, but I am not sure if removing the Requires.private lines is the right solution here. One area where these lines factors in is static vs. dynamic linking, but they also matter if you want to compile any kind of C/C++ code against a library.

Specifically, the example given above uses the --cflags option, which has nothing to do with linking — that option reports C/C++ compiler flags. And it would not be correct to truncate the output to just be -I$PREFIX/include/harfbuzz. The Harfbuzz headers reference types defined in the Glib/freetype/etc. headers and you simply can't (reliably) compile C/C++ code against Harfbuzz without including those headers as well. This is the same category of issue as you run into with static linking — the C/C++ development files for Harfbuzz have to depend on the C/C++ development files for Glib/freetype/etc, and so on throughout the dependency graph — but it's a different issue, and the fact that we don't encourage/support static linking doesn't make it go away, unfortunately. (And forcing people to write pkg-config --cflags harfbuzz glib freetype2 induces the exact same problem, just in a more error-prone and less scalable way.)

Ultimately, as long as we are compiling C/C++ code, the development files for libraries will have to depend on the development files for their dependents. (And I think that you can argue that this is really the essence of what it means to be a "library" regardless of language/OS/whatever, but that's beyond the scope here ...) The real issue here, in my view, is that there are still many cases where we don't (or don't properly) separate libraries and development files into different subpackages. You can't get rid of the dependency graph, but you can at least do a better job of partitioning it into runtime/dev components. Unfortunately, Conda didn't include this distinction when it was first developed, so all of this separation has to be applied retroactively. (Which is a bit frustrating as Linux distros have been getting this right for 30 years, but there is also a reason that Conda has achieved the level of success that it has ...)

@hmaarrfk
Copy link
Contributor Author

So in other words, resolve this issue correctly, we would need to really create -dev packages (or whatever name we choose) and ensure that the -dev package of the dependencies are included.

but there is also a reason that Conda has achieved the level of success that it has ...

I think may be we should have a place where we can write why we chose to use conda(-forge). It is easy to forget sometimes.

@pkgw
Copy link
Contributor

pkgw commented Feb 2, 2023

That's my belief, yes. But as @isuruf correctly points out, the current implementation of pkg-config does have bugs regarding this stuff, and there may be individual cases where it's correct to tweak the pkg-config file. But if the upstream developers have done things right, I think that in general the dependencies induced by Requires.private should be kept — they are basically developer dependencies that will need to be present to compile any kind of code against a library, not just for static linking purposes.

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

No branches or pull requests

6 participants