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

libcurl's package config file unconditionally references mbedtls in Requires.private but it does not exist #15469

Closed
haampie opened this issue Nov 1, 2024 · 10 comments
Labels
build not-a-curl-bug This is not a bug in curl

Comments

@haampie
Copy link

haampie commented Nov 1, 2024

I did this

Install mbedtls 3.6.0 (or any older version) with their makefile build system

It does not produce a package config file.

Now install curl with this mbedtls, and it will produce a libcurl.pc like this:

...
Requires.private: zlib,mbedtls,libssh2,libnghttp2
...

which then breaks builds of dependents of curl such as elfutils, because:

configure:13512: checking for libcurl >= 7.29.0
configure:13519: $PKG_CONFIG --exists --print-errors "libcurl >= 7.29.0"
configure:13522: $? = 0
configure:13536: $PKG_CONFIG --exists --print-errors "libcurl >= 7.29.0"
configure:13539: $? = 0
configure:13553: result: no
Package 'mbedtls', required by 'libcurl', not found

I expected the following

I would expect curl not to put mbedtls in the Requires.private section of its libcurl.pc file, because it never tested whether this package config file exists.

I would also expect curl to continue to allow users to build mbedtls using their makefile build system, given that cmake itself depends on curl, so requiring cmake as a build system for mbedtls causes circular dependencies across these packages.

Lastly, I would expect curl to produce a sensible pc file when using old mbedtls, which did not produce a pc file to start with.

curl/libcurl version

curl 8.9.0 and up

operating system

irrelevant

@haampie
Copy link
Author

haampie commented Nov 1, 2024

ping @vszakats

@vszakats
Copy link
Member

vszakats commented Nov 1, 2024

I don't see a local solution to this besides not referencing mbedtls.pc at all, for the next unknown number of years till the mbedtls Makefile build gets fixed to produce these files, and enough systems provide the mbedtls versions shipping with them.

The same problem might apply to all other dependencies that may miss their pc file, e.g. probably rustls. It also means curl cannot use msh3.pc for the foreseeable future, where the pc file is broken (pending a PR).

Do you have a suggestion on how to make libcurl.pc "sensible"?

@vszakats vszakats added the build label Nov 1, 2024
@haampie
Copy link
Author

haampie commented Nov 1, 2024

It's an annoying issue. I'd love to see Require.private work, have pkg-config recursively resolve dependencies, and have static linking work almost as easy as dynamic linking.

But Requires.private is not without problems:

  1. If a dependency was not detected with pkg-config during configure of curl, there should probably not be a reference to it under Requires*.
  2. Even if all dependencies were detected with pkg-config during configure of curl, it may not necessarily be the case that these .pc files exist on the target system. Notably zlib is unlikely to have a .pc file on an arbitrary system.

If every package always generated pc files, Requires is great. The reality is that this is not the case. Typically some autotools / cmake packages generate .pc files (by choice of the maintainer), but often there are multiple build systems (mbedtls has Makefile or CMake, zlib has its hand-written configure script or CMake, ...) which results in .pc files not being ubiquitous even for the same package.

So, I would say that requiring .pc files is just too strict, and maybe it should be opt-in.

(I'm digressing, but IMO static libs should contain an extra metadata file that is equivalent to the dynamic section of shared libraries in ELF files, so that the linker can resolve dependencies of static libraries, and pkg-config becomes obsolete -- but unfortunately pkg-config remains needed for compiler flags like -I and -D.)

@vszakats
Copy link
Member

vszakats commented Nov 1, 2024

Decisions made at libcurl.pc generation time will inevitably backfire in some cases when using it in a different env. For this reason, I wonder if this is better solved in the target envs, e.g. by ignoring such issues, allow relying on shared dependencies if static ones are broken, or opting out from pkg-config if broken.

Curl cmake builds try to solve this by providing an option to disable pkg-config detection globally. There is also a built-in cmake way to disable it per target, and it's also possible to manually configure dependencies, in which case pkg-config is skipped. autotools builds don't have dedicated options like this, leaving pkg-config envs to control it.

We haven't heard of issues like this for other dependencies, and mbedtls's pkg-config support is fairly new, perhaps another place to fix this is mbedtls's Makefile builds? It's unexpected that the presence of a public file depends on the build tool used.

Dropping mbedtls from the .pc file will certainly break builds for other users. Would this be a clear improvement?

@bagder
Copy link
Member

bagder commented Nov 1, 2024

Static builds will always be a pain.

@haampie
Copy link
Author

haampie commented Nov 2, 2024

I would leave mbedtls out, given that

  1. its .pc file is new and curl can be built with older mbedtls
  2. the .pc file is only generated from their cmake build system, but cmake itself depends on curl, so from a bootstrapping perspective the makefile is the natural build system for mbedtls.
  3. mbedtls has no dependencies, and you already include mbedtls's libraries in the libs section, meaning Require.private does not add missing information

@vszakats
Copy link
Member

vszakats commented Nov 4, 2024

mbedTLS added .pc support to their LTS release in March. Also back-ported
to the 2.x line. It suggests they are willing to support this and it's safe to start
relying on it for dependent projects.

mbedtls.pc being part of a lib's public interface, it seems fair to expect it
regardless of which tool was used to build it.

For these reasons I feel everyone would be better served if mbedTLS
produced this file also from their Makefile builds.

@vszakats vszakats added the not-a-curl-bug This is not a bug in curl label Nov 5, 2024
@haampie
Copy link
Author

haampie commented Nov 5, 2024

Since I don't agree, can you socialize that with the mbedtls folks then.

@vszakats
Copy link
Member

vszakats commented Nov 8, 2024

I don't think I'd like to do that, esp. because I don't use mbedTLS and never tried its Makefile build.

@bagder
Copy link
Member

bagder commented Nov 9, 2024

I am with @vszakats here. This looks like an oversight by mbedTLS and should be taken to them.

pkg-config is a fragile system in that we need to add data that then depends on someone else to to their job correctly. Since we can't know that for sure, we need to do a best effort. I think we do that here.

@bagder bagder closed this as completed Nov 9, 2024
vszakats added a commit that referenced this issue Nov 14, 2024
The idea of linking dependencies found to `libcurl.pc` turns out not
to work in practice in some cases.

Specifically: gss, ldap, mbedtls, libmsh3, rustls

A `.pc` may not work or be missing for a couple of reasons:
- not all build methods generate it: mbedTLS, Rustls
- generated file is broken: msh3
  Ref: nibanks/msh3#225
- installed package flavour isn't shipping with one:
  FreeBSD GSS, OmniOS LDAP, macOS LDAP

The effect of such issues shall be subtle in theory, because
`libcurl.pc` normally lists these dependencies in the `Requires.private`
section meant for static linking. But, e.g. `pkg-config --exists`
requires these to be present, and builds sometimes use this check
regardless of build type. This bug is not present in `pkgconf`; it only
checks for them when `--static` is also passed.

Fix these by adding affected `.pc` references to `libcurl.pc` only when
we detected the dependency via `pkg-config`.

There are a few side-effects of this solution:
- references are never added for dependencies where curl doesn't
  implement `pkg-config` detection. These are:
  - autotools: ldap, mbedtls, msh3
  - cmake: ldap (pending #15273)
- generated `libcurl.pc` depends on the build-time environment.
- generated `libcurl.pc` depends on curl build tool (cmake, autotools).
- generated `libcurl.pc` depends on curl build implementation details.

Make an exception for GNU GSS, where I blindly guess that `gss.pc` is
always available, as no issues were reported.

Other, not mentioned, dependencies continue to be added regardless
of the detection method.

Reported-by: Harmen Stoppels, Thomas, Daniel Engberg, Andy Fiddaman
Fixes #15469
Fixes #15507
Fixes #15535
Fixes #15163 (comment)
Closes #15573
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build not-a-curl-bug This is not a bug in curl
Development

Successfully merging a pull request may close this issue.

3 participants