pkg-config: add initial pkg-config files

[billatarm]

Description

Add three package config files for mbedtls, mbedcrypto and mbedx509. Also update various project variables so the generated PC files have the required data needed without hardcoding it everywhere.

This will help distros package the project following existing conventsions between a normal and -devel package that includes the headers and .pc files for pkg-config aware consumers.

Fixes: #228

PR checklist

Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")

• changelog provided, or not required
• backport Mbedtls 2.28 add pc files #8851
• tests Will be added as a separate task

[Tachi107]

Hi, thanks for your patch! Distros will be really happy :)

I've left a few suggestions that I think should be addressed before merging this. I'm not a project maintainer, I'm just a guy that played too much with CMake that happens to maintain the mbedtls Debian package.

[billatarm]

Hi, thanks for your patch! Distros will be really happy :)

I've left a few suggestions that I think should be addressed before merging this. I'm not a project maintainer, I'm just a guy that played too much with CMake that happens to maintain the mbedtls Debian package.

Perfect thanks, i've never touched CMake and I know enough about pkg-config to maintain my projects at tpm2-software and to injure others.

[daverodgman]

@billatarm is this still draft - or ready for review? thanks

[billatarm]

@billatarm is this still draft - or ready for review? thanks

Yes thanks

[billatarm]

@jtojnar could you comment if we could relicense this Cmake snippet under the dual Apache 2.0/GPL license scheme?

[jtojnar]

Sure, feel free to relicense it. The intent is that anyone can use it in any software project.

[tom-daubney-arm]

@billatarm We have some CI failure on this PR. Would you be able to check them out please?

[billatarm]

@billatarm We have some CI failure on this PR. Would you be able to check them out please?

The problem seems to be the useages of HOMEPAGE_URL and DESCRIPTION in the project declaration. Those need 3.12 and 3.9 respectively and those CI builds are on cmake version 3.5.1. Which is also at odds with your own readme requiring 3.10.2 here, but even correcting that would only fix one of the variable usages.

I'll just have to define the variables for now and use those.

[billatarm]

@billatarm We have some CI failure on this PR. Would you be able to check them out please?

The problem seems to be the useages of HOMEPAGE_URL and DESCRIPTION in the project declaration. Those need 3.12 and 3.9 respectively and those CI builds are on cmake version 3.5.1. Which is also at odds with your own readme requiring 3.10.2 here, but even correcting that would only fix one of the variable usages.

I'll just have to define the variables for now and use those.

Ahh it's documented as why the README and CIs are disparate:

mbedtls/CMakeLists.txt

Lines 19 to 21 in 32c28ce

	# We specify a minimum requirement of 3.10.2, but for now use 3.5.1 here
	# until our infrastructure catches up.
	cmake_minimum_required(VERSION 3.5.1)

[tom-daubney-arm]

@billatarm Apologies for the delay on this. I was on leave last week. Thanks for addressing the CI failures. I have kicked off a new CI run just now so we should know if it was successful in a few hours.

[daverodgman]

I tweaked the changelog to fix trivial syntax errors, as I think this is the only thing blocking CI - hope that's OK @billatarm

[williamcroberts]

I tweaked the changelog to fix trivial syntax errors, as I think this is the only thing blocking CI - hope that's OK @billatarm

Thanks, appreciate it, yeah - *, I wonder why my editor added that, usually retext is pretty good.

[tom-daubney-arm]

I think this all looks sensible. I have been playing around with it and it seems to work as I would expect, and the CMake stuff all looks sensible. Thanks @billatarm. LGTM.

[yanesca]

LGTM

[Biswa96]

The version field is empty in all pkgconfig files. Is that expected?

[billatarm]

The version field is empty in all pkgconfig files. Is that expected?

No and I can reproduce, let me see whats up

[billatarm]

The version field is empty in all pkgconfig files. Is that expected?

No and I can reproduce, let me see whats up

Ok, so in the final version of the patch when I removed the HOMEPAGE_URL and DESCRIPTION, and let that go back to original form, it broke, I dunno why, and github doesn't show my the version delats AFAIK. But this patch fixes it:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 5585c78fa..3d6f14f4b 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -33,10 +33,17 @@ cmake_policy(SET CMP0011 NEW)
 # is deprecated and will be removed in future versions.
 cmake_policy(SET CMP0012 NEW)
 
+
 if(TEST_CPP)
-    project("Mbed TLS" LANGUAGES C CXX)
+    project("Mbed TLS"
+           LANGUAGES C CXX
+           VERSION 3.5.2
+    )
 else()
-    project("Mbed TLS" LANGUAGES C)
+    project("Mbed TLS"
+           LANGUAGES C
+           VERSION 3.5.2
+    )
 endif()
 
 include(GNUInstallDirs)

I'll submit a PR

[billatarm]

Follow up patch to fix empty version field: #8850

[gilles-peskine-arm]

We're announcing a new feature, but this isn't tested. Are you planning a follow-up with a test (either expanding an existing cmake-related component or a new component in tests/scripts/all.sh)?

[billatarm]

We're announcing a new feature, but this isn't tested. Are you planning a follow-up with a test (either expanding an existing cmake-related component or a new component in tests/scripts/all.sh)?

Yeah the goal is to add this to automated tests doing a make install and then running pkg-config, I have to look at how the test suite is constructed to see where the best place is.

[billatarm]

See PR for tests: #8988

[Tachi107]

Late feedback: shouldn't this be Requires instead of Requires.private? The CMake code, for instance, has a PUBLIC dependency on mbedx509.

mbedtls/library/CMakeLists.txt

Lines 314 to 320 in 30978ec

	add_library(${mbedx509_target} SHARED ${src_x509})
	set_target_properties(${mbedx509_target} PROPERTIES VERSION 3.6.0 SOVERSION 7)
	target_link_libraries(${mbedx509_target} PUBLIC ${libs} ${mbedcrypto_target})
	add_library(${mbedtls_target} SHARED ${src_tls})
	set_target_properties(${mbedtls_target} PROPERTIES VERSION 3.6.0 SOVERSION 21)
	target_link_libraries(${mbedtls_target} PUBLIC ${libs} ${mbedx509_target})

[Biswa96]

That depends on the following question. If a program links with mbedtls only dynamically, does it need to link with mbedcrypto and mbedx509 directly?

[billatarm]

Requires.private is correct here. Those libraries are linked to the shared object not the requestor.

I think this explains it better than me:

• https://stackoverflow.com/questions/45441250/how-does-the-linking-process-differ-when-using-requires-vs-requires-private-in-p

[Tachi107]

That depends on the following question. If a program links with mbedtls only dynamically, does it need to link with mbedcrypto and mbedx509 directly?

I was under the impression that this was the case, but no, Requires.private seems appropriate. mbedtls/ssl.h includes mbedtls/x509_crt.h, which is part of mbedx509, so the user ends up needing the appropriate compiler flags for mbedx509 too, but unless it uses x509 functions it does not need its linking flags. Due to multiple reasons, pkg-config's Requires.private propagates the Cflags when doing shared builds, but not the Libs, so this is exactly what we need.

In short: the pkg-config files are ok, but mebdtls' PUBLIC dependency on mbedx509 in CMake is probably wrong (or at least not precise enough).