Cargo: configure cargo-c to use vendored .h

[cpu]

Presently we pre-generate the rustls.h header file using cbindgen, commit the result to src/rustls.h, and check that the generated result matches the checked-in result in CI. However, the new experimental cargo-c build regenerates its own header file using cbindgen by default unless told to do otherwise. We'd prefer it didn't do this because we're using a cbindgen feature that requires nightly rust.

This commit updates the Cargo.toml capi metadata to tell cargo-c to skip generation of its own header file. We then configure the pre-generated checked-in header file as an asset to be copied into the install include directory.

This better matches how the Makefile build allowed building the static lib without needing nightly rust or cbindgen and resolves #397.

[cpu]

@Kangie I'd be interested to hear how this branch works for you. It seems to do the right thing in my local testing 🤞

[cpu]

@kpcyrd If you wouldn't mind testing I'm hoping this branch lets you drop using RUSTC_BOOTSTRAP in your Arch package.

[Kangie]

I will try and test with this in the near future, but will be busy over this weekend :(

Pinging @thesamesam as the Gentoo rustls-ffi maintainer who might get a chance to look first!

[cpu]

rustls-ffi / Clippy nightly (optional) (pull_request) Failing after 18s

#399

[thesamesam]

Thanks! This PR works for us in Gentoo. I'll pull it in.

[thesamesam]

Thanks! This PR works for us in Gentoo. I'll pull it in.

I lied! It looks like rustls.h may not be installed when this is applied?

[kpcyrd]

It also builds without RUSTC_BOOTSTRAP=1 for me but the header file is not in the package anymore:

==> Running checkpkg
  -> Checking packages
usr/include/						      <
usr/include/rustls.h					      <
==> No soname differences for librustls.

Maybe @lu-zero has thoughts on this too? :)

[cpu]

Ah, thank you both for reporting back. I did my initial testing with config like the following, and observed the header being included:

[package.metadata.capi.install.include]
asset = [{from = "src/rustls.h", to = "" }]

Right before pushing this branch I thought the to = "" was strange and tried an alternative that I pushed:

[package.metadata.capi.install]
asset = [{from = "src/rustls.h", to = "include" }]

I think when I re-tested this change I must have made a mistake because I see the same results you folks reported: the header file was missing. (It's always the "one last tweak before bed" that gets you...)

I've reverted that change and now see the correct behaviour again:

[daniel@blanc:~/Code/Rust/rustls-ffi]$ git rev-parse HEAD
dd0a1131b50a92479bdb9e18e1a56dd28ad09de1

[daniel@blanc:~/Code/Rust/rustls-ffi]$ cargo cinstall --release --prefix=/tmp/out
   Compiling rustls-ffi v0.12.1 (/home/daniel/Code/Rust/rustls-ffi)
    Finished release [optimized] target(s) in 0.99s
    Building pkg-config files
  Populating uninstalled header directory
  Installing pkg-config file
  Installing header file
  Installing static library
  Installing shared library

[daniel@blanc:~/Code/Rust/rustls-ffi]$ tree /tmp/out
/tmp/out
├── include
│   └── rustls.h
└── lib
    ├── librustls.a
    ├── librustls.so -> librustls.so.0.12.1
    ├── librustls.so.0 -> librustls.so.0.12.1
    ├── librustls.so.0.12.1
    └── pkgconfig
        └── rustls.pc

4 directories, 6 files

Could you give this branch another spin and see if it resolves the issue for you folks as well?

[kpcyrd]

I tried again with the new patch and it seems fine for me 👍

(Looking at your output, you may want to update your cargo-c binary) :)

[cpu]

I tried again with the new patch and it seems fine for me 👍

Excellent, thank you!

(Looking at your output, you may want to update your cargo-c binary) :)

I'm using cargo-c 0.9.24+cargo-0.73.0 but it looks like nixpkgs unstable updated to 0.9.29 since I last rebuilt. I don't think anyone has started packaging .30 or .31 yet. I'll follow up on that (Edit: NixOS/nixpkgs#299789).

[jsha]

Looks good, thanks!

[cpu]

I'm going to merge this so I can include it in the 0.13 release. If it turns out there's still an issue with Gentoo builds we can tweak futher. I'll also backport the fix to a 0.12.2 point release (edit: #401).

[cpu]

This was included in both 0.12.2 and 0.13

[lu-zero]

Maybe @lu-zero has thoughts on this too? :)

I think the default to may be changed since to="" probably is more common. I'll think more about it and in case change it and document the default to. Thank you for pointing out to="" is a bit annoying.

[lu-zero]

Maybe @lu-zero has thoughts on this too? :)

I think the default to may be changed since to="" probably is more common. I'll think more about it and in case change it and document the default to. Thank you for pointing out to="" is a bit annoying.

I just checked and behaves as intended. I would suggest to keep the include subdir since tends to be more future proof.

 [package.metadata.capi.install.include]
-asset = [{from = "src/rustls.h", to = "" }]
+asset = [{from = "src/rustls.h" }]

[cpu]

@lu-zero Ah! Interesting to know we could omit the to. I think I agree with you that we might as well keep it as-is for now. It's working as intended and I doubt we'll have reason to revisit soon.