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.

Sign up for GitHub

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

Jump to bottom

Use pkg config #139

Merged
merged 2 commits into from
Feb 2, 2024
Merged

Use pkg config #139

merged 2 commits into from
Feb 2, 2024

Conversation

billatarm
Copy link
Collaborator

No description provided.

@billatarm billatarm force-pushed the use-pkg-config branch 3 times, most recently from 9ef16f5 to 8d0146e Compare January 23, 2024 21:46
billatarm
billatarm commented Jan 23, 2024
View reviewed changes
psa-crypto-sys/build.rs Outdated Show resolved Hide resolved
tgonzalezorlandoarm
tgonzalezorlandoarm requested changes Jan 24, 2024
View reviewed changes
psa-crypto-sys/build.rs Outdated Show resolved Hide resolved
psa-crypto-sys/build.rs Outdated Show resolved Hide resolved
@billatarm billatarm force-pushed the use-pkg-config branch 4 times, most recently from d158d1e to 57bb452 Compare January 24, 2024 17:53
@billatarm
build: support pkg-config
0b5b462 
Support querying pkg-config for mbedtls directories.

When building against mbedtls, use the following search criteria to
locate the mbedtls dependency:
1. Env Variables
2. pkg-config
3. vendor directory

This will allow binary packages from distributions to work without
modification and not require a vendored mbedtls package. The problem
with vendored packages is upsates, and since mbedtls is a critical
cryptographic suite, it's important that the distributions can quickly
update a single mbedtls package that everyone links to.

Related to:
  - Mbed-TLS/mbedtls#8691

Signed-off-by: Bill Roberts <[email protected]>
@billatarm
build: drop configuring external mbedtls
7da41a6 
This also fixes a possible header mismatch, as the interface feature was
wanting an external mbedtls header file to build against, but would
generate the bindings and compile the shim library against the local
vendored mbedtls. On an ABI change, things would have been broken.

To fix this, and not require the vendored package, use the externally
supplied mbedtls found through the env var or pkg-config.

Signed-off-by: Bill Roberts <[email protected]>
tgonzalezorlandoarm
tgonzalezorlandoarm previously approved these changes Jan 26, 2024
View reviewed changes
Copy link
Member

@tgonzalezorlandoarm tgonzalezorlandoarm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, Thanks!

@gowthamsk-arm
Copy link
Contributor

@billatarm how to test the pkg-config feature locally? The CI doesn't do it right now so want to test it manually.
Installing mbedtls with your pkg-config patch and then building this crate, is it sufficient? :)

@tgonzalezorlandoarm tgonzalezorlandoarm dismissed their stale review January 26, 2024 16:11

Awaiting steps for testing locally

@billatarm
Copy link
Collaborator Author

@billatarm how to test the pkg-config feature locally? The CI doesn't do it right now so want to test it manually. Installing mbedtls with your pkg-config patch and then building this crate, is it sufficient? :)

Yes, that is correct. You need to install mbedtls and have the pc files on PKG_CONFIG_PATH, your mileage will vary based your distro's specific defaults for pkg-config. If you install to a non default path, ie using DESTDIR during make install, ensure your values of --prefix and PKG_CONFIG_DIR are set appropriately.

pkg-config can tell you how your system is configured:

# default search path for .pc files:
pkg-config --variable=pc_path pkg-config
/usr/lib64/pkgconfig:/usr/share/pkgconfig

# default include paths
pkg-config --variable=pc_system_includedirs pkg-config
/usr/include

# default libdir
pkg-config --variable=pc_system_libdirs pkg-config
/usr/lib64

@gowthamsk-arm
Copy link
Contributor

Thanks for the steps @billatarm
I just tested the pkg-config changes with the mbedtls PR that you put up and it works all well.

gowthamsk-arm
gowthamsk-arm approved these changes Feb 1, 2024
View reviewed changes
Copy link
Contributor

@gowthamsk-arm gowthamsk-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tgonzalezorlandoarm tgonzalezorlandoarm merged commit 7695a58 into parallaxsecond:main Feb 2, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gowthamsk-arm gowthamsk-arm gowthamsk-arm approved these changes

@tgonzalezorlandoarm tgonzalezorlandoarm tgonzalezorlandoarm approved these changes

@egrimley-arm egrimley-arm Awaiting requested review from egrimley-arm

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@billatarm @gowthamsk-arm @tgonzalezorlandoarm