github CI workflow for module on linux #4
Conversation
icing
commented
Jan 15, 2025
icing
commented
- Adjust config and server commands as in mod_h2 and mod_md projects.
- Add python install requirements.
Adjust config and server commands as in mod_h2 and mod_md projects. Add python install requirements.
|
@cpu trying to get a CI workflow going, but rustls-ffi wants |
Thank you! :-)
@icing Odd 🤔 I haven't seen that particular build failure before. I'm tied up today/tomorrow but if you leave this with me I'm happy to debug further. To unblock this PR in the meantime you might have luck adding |
| - name: 'build rustls-ffi (cmake)' | ||
| if: matrix.rustls-version == 'main' | ||
| run: | | ||
| cd $HOME/rustls-ffi | ||
| cmake \ | ||
| -DCRYPTO_PROVIDER=${{matrix.crypto}} \ | ||
| -DDYN_LINK=on \ | ||
| -DCMAKE_BUILD_TYPE=Release \ | ||
| -S librustls -B build | ||
| cmake --build build --config "Release" |
There was a problem hiding this comment.
Using cmake here is making your life harder for no reason. It's for building the client.c and server.c examples and only coincidentally builds the library as a prereq.
I would recommend using cargo-c directly like the README describes:
cargo capi install --release --prefix <wherever> --features ${{matrix.features}}.
There was a problem hiding this comment.
Well, how about a Makefile in rustls-ffi that does this and I do not have to worry about how to use cargo capi?
Btw. it took me some time to find out that the generate cmake accepts a -DCMAKE_INSTALL_PREFIX=path, performs a --install without complain, but does not install anything.
There was a problem hiding this comment.
Well, how about a Makefile in rustls-ffi that does this and I do not have to worry about how to use cargo capi?
I mentioned in a DM, but we plan to publish binary artifacts for common platforms for the next release and that will remove the need for cargo c. You can download a tarball with the .so/.a/.pc all ready to go.
We had a Makefile before but it:
- Was brittle and hard to maintain - especially for things like mutually exclusive features
- Didn't produce a
.pcfile for pkg-config - Didn't produce a
.sofor dynamic linking - Didn't support Windows
Cargo-c fixes all of the above and it's being used by a number of similar Rust projects producing native libraries. To me it seems like the best solution short of cargo itself building in support for producing these types of build artifacts (which doesn't seem likely to happen anytime soon unfortunately).
Btw. it took me some time to find out that the generate cmake accepts a -DCMAKE_INSTALL_PREFIX=path, performs a --install without complain, but does not install anything.
Did you find the rustls-ffi README made it seem like this should work? I thought it was fairly clear about the role of cmake but I'm open to suggestions to make it clearer!
There was a problem hiding this comment.
Did you find the rustls-ffi README made it seem like this should work? I thought it was fairly clear about the role of
cmakebut I'm open to suggestions to make it clearer!
Cannot say. Hacker as I am, I just expected a project with a cmake to support the standard features. I could have live with an error. Don't know enough about cmake (and prefer not to), to suggest how to address that. Nothing major.
There was a problem hiding this comment.
Well, how about a Makefile in rustls-ffi that does this and I do not have to worry about how to use cargo capi?
Thinking on this more it does seem reasonable to leave behind a Makefile that does the right cargo-c invocations under the hood.
My objections were about implementing the build/install logic in the Makefile itself and aren't relevant to using the Makefile as a compat shim for invoking cargo-c in a way more familiar to Makefile users.
Thanks for the suggestion, I will handle this in rustls/rustls-ffi#525
There was a problem hiding this comment.
Don't know enough about cmake (and prefer not to), to suggest how to address that.
😆 Tell me about it. I learned cmake recently just for rustls-ffi and it's a mess.
I'll put some thought into how we can emit an error (or just support installing that way if it's not too tricky). (Edit: rustls/rustls-ffi#526)
|
I commented out the It would be nice to test with that crypto provider as well. |
|
I'll merge this PR into master, so when you feel like playing around with it, it will be easier to make a PR with any improvements you might want to add. |
Sounds great, thank you again 🙇 I promise to spend some time looking into the aws-lc-rs build issue very soon. |
|
Merged as 5f65a43 |
