Skip to content

Commit

Permalink
Fix rpm-sequoia.pc generation
Browse files Browse the repository at this point in the history 
  - Don't fix the dependency to `nettle`.  The required libraries
    depend on the configured backend.  Substitute the appropriate
    dependencies based on the configured backend at build time.

  - Fixes #36.
  • Loading branch information
@nwalfield
nwalfield committed Apr 13, 2023
1 parent f8ef673 commit 696ff53
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
16 changes: 16 additions & 0 deletions build.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,22 @@ impl PkgConfigTemplate {
("DESCRIPTION".to_string(), env!("CARGO_PKG_DESCRIPTION").to_string()),
("VERSION".to_string(), env!("CARGO_PKG_VERSION").to_string()),
("HOMEPAGE".to_string(), env!("CARGO_PKG_HOMEPAGE").to_string()),
("REQUIRES".to_string(),
if cfg!(feature = "crypto-botan") {
"botan-2"
} else if cfg!(feature = "crypto-nettle") {
"nettle"
} else if cfg!(feature = "crypto-openssl") {
"libssl"

This comment has been minimized.

Sign in to view

Copy link
@ferdnyc

ferdnyc Apr 14, 2023

Contributor

@nwalfield Is libssl sufficient? I guess it would be, since it lists libcrypto (the bit we really want) as a private dependency.

Fedora is currently replacing nettle with openssl, which will link in both libssl and libcrypto explicitly -- but I'm realizing now that that's probably unnecessary overkill.

Oh well, it'll sort itself out as soon as a release with this commit is packaged and the local patch is dropped. (Of the two options, erring on the side of overlinking is certainly the less bad outcome anyway.)

This comment has been minimized.

Sign in to view

Copy link
@nwalfield

nwalfield Apr 14, 2023

Author Collaborator

Thanks for reviewing. You might be right that just listing libcrypto is enough.

This comment has been minimized.

Sign in to view

Copy link
@ferdnyc

ferdnyc Apr 15, 2023 •
edited
Loading

Contributor

@nwalfield Oh, no, I don't think that at all — I think "libssl" is the way to go. Fedora was listing "openssl", which (now that I look at it) doesn't actually link with either library, it just lists them as dependencies:

$ cat /usr/lib64/pkgconfig/openssl.pc
prefix=/usr
exec_prefix=${prefix}
libdir=${exec_prefix}/lib64
includedir=${prefix}/include

Name: OpenSSL
Description: Secure Sockets Layer and cryptography libraries and tools
Version: 3.0.8
Requires: libssl libcrypto

Although, on second thought, they're public dependencies, so they will still get pulled in to pkg-config --libs openssl. libssl only publicly links its own library, not libcrypto... but that still seems correct to me. The crypto interfaces libssl uses are internal, so (just like libssl is a private dep of sequoia), crypto is a private dep of libssl, at least in a shared-library context.

$ pkg-config --libs libssl
-lssl 
$ pkg-config --libs openssl
-lssl -lcrypto
$ pkg-config --static --libs libssl
-lssl -lcrypto -lz -ldl -pthread

This comment has been minimized.

Sign in to view

Copy link
@nwalfield

nwalfield Apr 15, 2023

Author Collaborator

Can you copy your comment over to this issue please.
} else if cfg!(feature = "crypto-cng") {
""
} else if cfg!(feature = "crypto-rust") {
""
} else {
panic!("No cryptographic backend selected. Try: \
\"cargo build --no-default-features \
--features crypto-openssl\"")
}.to_string()),
]);

Ok(PkgConfigTemplate {
Expand Down
2 changes: 1 addition & 1 deletion rpm-sequoia.pc.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ Name: @NAME@
Description: @DESCRIPTION@
URL: @HOMEPAGE@
Version: @VERSION@
Requires.private: nettle
Requires.private: @REQUIRES@
Cflags:
Libs: -L${libdir} -lrpm_sequoia

0 comments on commit 696ff53

Please sign in to comment.