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

feat(ffi): add cargo-c support #3787

Merged
merged 1 commit into from
Nov 22, 2024
Merged

feat(ffi): add cargo-c support #3787

merged 1 commit into from
Nov 22, 2024

Conversation

seanmonstar
Copy link
Member

Probably doesn't fully work yet, but here's a start.

Closes #3786

@kpcyrd
Copy link

kpcyrd commented Nov 19, 2024

Works fine for me:

# Maintainer: kpcyrd <kpcyrd[at]archlinux[dot]org>

pkgname=libhyper
pkgver=1.5.1
pkgrel=1
pkgdesc="Use hyper HTTP library from languages other than Rust"
arch=('x86_64')
license=('MIT')
url='https://github.com/hyperium/hyper'
depends=(
  gcc-libs
  glibc
)
makedepends=(
  cargo-c
  rust
)
provides=('libhyper.so')
source=(https://github.com/hyperium/hyper/archive/v${pkgver}/${pkgname}-${pkgver}.tar.gz
        https://patch-diff.githubusercontent.com/raw/hyperium/hyper/pull/3787.patch
        Cargo.lock)
sha256sums=('fc043aa072c7ed39740324b250047d8ef2cfb30bff3364fabeeb150c28406ec0'
            '2c0ee5a25bf17eb874b553a9ff4b2f63dbd50ae6dcb3d39e22496ed7194f2579'
            '1696298a6307c5cc2a4eae17664cee47841ba8f6eca8b3784a3f52304b67cacf')
b2sums=('2d52a630a7e652742d5ea7df217e1db3ffea51422d5cdfcba4a88d024705bbb8d3c384636a130bdb3a53b5c3c84e8ca6f329445556e4c1c5d2a002996b956d1b'
        'bc115a621da7884e03061ce98a00db9a6b8aad7e95b36f6fa611e0baa14af54a2c26b15f8cba3381b328ba27c5c12f1f495a42664f1e6383f0bc0473868bcce1'
        '7cece9c84da0e17886c547e1008fa46680fea728b72da846529c126c126cf75d33e5f9c20b105dbd48ab88d9a164170899c0e9745cfe196bf99703e999006bd3')

prepare() {
  cd hyper-${pkgver}
  cp ../Cargo.lock .
  patch -Np1 -i ../3787.patch
  cargo fetch --locked --target "$(rustc -vV | sed -n 's/host: //p')"
}

build() {
  cd hyper-${pkgver}
  cargo cbuild --release --frozen --prefix=/usr
}

package() {
  cd hyper-${pkgver}
  cargo cinstall --release --frozen --prefix /usr --destdir "${pkgdir}"
  install -Dm644 -t "${pkgdir}/usr/share/licenses/${pkgname}" LICENSE
}

# vim: ts=2 sw=2 et:

Results in this package:

% tar tvvf libhyper-1.5.1-1-x86_64.pkg.tar.zst
-rw-r--r-- root/root      5367 2024-11-19 23:36 .BUILDINFO
-rw-r--r-- root/root       479 2024-11-19 23:36 .MTREE
-rw-r--r-- root/root       430 2024-11-19 23:36 .PKGINFO
drwxr-xr-x root/root         0 2024-11-19 23:36 usr/
drwxr-xr-x root/root         0 2024-11-19 23:36 usr/lib/
lrwxrwxrwx root/root         0 2024-11-19 23:36 usr/lib/libhyper.so -> libhyper.so.1.5.1
lrwxrwxrwx root/root         0 2024-11-19 23:36 usr/lib/libhyper.so.1 -> libhyper.so.1.5.1
-rwxr-xr-x root/root     14048 2024-11-19 23:36 usr/lib/libhyper.so.1.5.1
drwxr-xr-x root/root         0 2024-11-19 23:36 usr/lib/pkgconfig/
-rw-r--r-- root/root       287 2024-11-19 23:36 usr/lib/pkgconfig/hyper.pc
drwxr-xr-x root/root         0 2024-11-19 23:36 usr/share/
drwxr-xr-x root/root         0 2024-11-19 23:36 usr/share/licenses/
drwxr-xr-x root/root         0 2024-11-19 23:36 usr/share/licenses/libhyper/
-rw-r--r-- root/root      1062 2024-11-19 23:36 usr/share/licenses/libhyper/LICENSE

Possibly something you want to take note of: Since hyper is >= 1.0.0 it's using libhyper.so.1 as SONAME, if you want to make an ABI breaking change to the C bindings you'd either need to bump hyper to 2.0.0 (which you likely won't want to do) or configure an overwrite in cargo-c configuration (which is possible and fully supported), but this only becomes relevant when the time comes anyway.

@seanmonstar
Copy link
Member Author

I don't think I have it correctly copying the header file, and trying to get the C examples to build in CI just to check it works.

As for the ABI compatibility: so far, the C API has been considered experimental, while in development with curl as a user. So I wouldn't want to make a promise yet, until we mark it as stable in both projects.

@kpcyrd
Copy link

kpcyrd commented Nov 19, 2024

You likely want to set this then (which I forgot is an option, as well as setting this to 2):

[package.metadata.capi.library]
version_suffix_components = 3

This sets the SONAME to libhyper.so.1.5.1 instead of libhyper.so.1, so any version change of hyper signals an ABI breakage to consumers.

When setting this to 2, the SONAME would be libhyper.so.1.5 and a 1.6.0 release of hyper would signal ABI breakage, without the need for 2.0.0 or maintaining a version override.

@milibopp
Copy link

Great to see that added. Packaging this branch with Nix using cargo-c also worked mostly fine for me.

One exception: I would have expected the test suite to pass via cargo ctest here as well:

RUSTFLAGS="--cfg hyper_unstable_ffi" cargo ctest --features ffi,http1,http2,client

It does not compile though. Presumably the integration tests are not intended to be run against a dynamic library rather than a Rust crate.

@seanmonstar
Copy link
Member Author

seanmonstar commented Nov 20, 2024

Presumably the integration tests are not intended to be run against a dynamic library rather than a Rust crate.

Ah, that's a mistake in hyper in general, unit tests don't have the right config attributes, so it also needs the server feature enabled to run. I confirmed it works with cargo ctest. Filed #3790.

@seanmonstar
Copy link
Member Author

OK, tried this locally and it's now copying over the correct header file. I removed the steps to test building the examples, I didn't want to make the makefile too complicated (that's not my expertise at all). (They're still checked in the other FFI CI job.)

@kpcyrd
Copy link

kpcyrd commented Nov 21, 2024

Using the latest patch, I currently get this package (the headerfile was indeed missing before):

% tar tvvf libhyper-1.5.1-1-x86_64.pkg.tar.zst
-rw-r--r-- root/root      5367 2024-11-21 01:31 .BUILDINFO
-rw-r--r-- root/root       543 2024-11-21 01:31 .MTREE
-rw-r--r-- root/root       443 2024-11-21 01:31 .PKGINFO
drwxr-xr-x root/root         0 2024-11-21 01:31 usr/
drwxr-xr-x root/root         0 2024-11-21 01:31 usr/include/
-rw-r--r-- root/root     13397 2024-11-21 01:31 usr/include/hyper.h
drwxr-xr-x root/root         0 2024-11-21 01:31 usr/lib/
lrwxrwxrwx root/root         0 2024-11-21 01:31 usr/lib/libhyper.so -> libhyper.so.1.5.1
lrwxrwxrwx root/root         0 2024-11-21 01:31 usr/lib/libhyper.so.1 -> libhyper.so.1.5.1
-rwxr-xr-x root/root     14048 2024-11-21 01:31 usr/lib/libhyper.so.1.5.1
drwxr-xr-x root/root         0 2024-11-21 01:31 usr/lib/pkgconfig/
-rw-r--r-- root/root       281 2024-11-21 01:31 usr/lib/pkgconfig/hyper.pc
drwxr-xr-x root/root         0 2024-11-21 01:31 usr/share/
drwxr-xr-x root/root         0 2024-11-21 01:31 usr/share/licenses/
drwxr-xr-x root/root         0 2024-11-21 01:31 usr/share/licenses/libhyper/
-rw-r--r-- root/root      1062 2024-11-21 01:31 usr/share/licenses/libhyper/LICENSE

The pkg-config file looks like this:

% cat /usr/lib/pkgconfig/hyper.pc
prefix=/usr
exec_prefix=${prefix}
libdir=${prefix}/lib
includedir=${prefix}/include

Name: hyper
Description: A protective and efficient HTTP library for all.
Version: 1.5.1
Libs: -L${libdir} -lhyper
Cflags: -I${includedir}
Libs.private: -lgcc_s -lutil -lrt -lpthread -lm -ldl -lc

I noticed I'm likely passing the wrong flags to cargo-c however (RUSTFLAGS and --features mentioned by @milibopp), I didn't attempt to build curl with this yet.

@@ -85,6 +85,7 @@ server = ["dep:httpdate", "dep:pin-project-lite", "dep:smallvec"]

# C-API support (currently unstable (no semver))
ffi = ["dep:http-body-util", "futures-util?/alloc"]
capi = []
Copy link

Choose a reason for hiding this comment

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

Probably would be nice to add here the required features client,http1,http2,ffi

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, the reason I haven't added them there automatically is because it's possible to choose to only enable http1 or http2, and there's a separate PR here as some people are working on the server portion of the C API.

@seanmonstar seanmonstar merged commit 7f4a682 into master Nov 22, 2024
22 checks passed
@seanmonstar seanmonstar deleted the ffi-cargo-c branch November 22, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo-c integration and dynamic linking
4 participants