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

Use tirpc as RPC implementation if available #47

Closed
wants to merge 1 commit into from

Conversation

schopin-pro
Copy link
Contributor

Glib has recently removed its RPC headers, and it is advised to use
tirpc instead. This patch simply switches to tirpc if available on the system.

I plan to ship this patch in Ubuntu and Debian to resolve build issues with their latest glibc updates.

Glib has recently removed its RPC headers, and it is advised to use
tirpc instead. This patch simply switches to tirpc if available on the system.
@calderonth
Copy link
Contributor

Good morning Simon and thanks for the patches.
Same comment as in #46 but testing here is even more important if the underlying RPC libraries has changed.

@rben-dev
Copy link
Contributor

rben-dev commented Aug 23, 2021

Hi Simon,

Thanks again for the patches and for making the compilation work again!

Unfortunately some first tests seem to show, as pointed by @calderonth, that something goes wrong during RPC as the proxy is killed and restarted during the first request. We will have to investigate this a bit more to fix the issue.

Small update: installing tirpc on an ubuntu 18.04 and compiling with the PR works, while there are some issues on debian (at least on my 'unstable' release).

Regards,

@schopin-pro
Copy link
Contributor Author

schopin-pro commented Aug 23, 2021 via email

@rben-dev
Copy link
Contributor

Hi Simon,

Thanks for the help proposal :-)

A quick update after some further investigation: the issue under Debian unstable and recent versions of OCaml seems to lie in the IDL generation layer that provokes a "late segfault" during RPC in the server process.
From what we see, the good news is that your PR are indeed working!

We will try to understand where is the exact hiccup with IDL, hopefully fix it, and then deal with merging your PR.
Anyways, thanks again for your interest and participation to improve CamlCrush :-)

Regards,

@rben-dev
Copy link
Contributor

This should be fixed in branch https://github.com/caml-pkcs11/caml-crush/tree/rbe/fix_release with the two PRs #46 and #47 merged and tested. The compatibility with OCaml < 4.02 should be back with the last commit 549542b.

We will take time to perform some tests with @calderonth and will merge all this if all is OK :-)

Regards,

calderonth added a commit that referenced this pull request Aug 27, 2021
This PR addresses includes the changes provided in #46 and #47 but add the necessary logic to have the Caml Crush code compatible with TIRPC has some implementation details had changed.
In addition, this introduces a way to perform end-to-end testing using docker-compose and should allow for better future-proofing and better backward compatibility testing.

* Port the code to use safe strings only
* build: Remove the unsafe-strings flag
* Use tirpc as RPC implementation if available
* Fix generated IDL patch with recent versions of camlidl as some functions
renaming prevent spatch (coccinelle) from performing its semantic patch on
the pkcs11_stub.c
* Put back compatibility with OCaml < 4.02 where the Bytes module is not present.
This is achieved through preprocessing.
* Refresh IDL and RPC generated headers and files with more recent versions of
camlidl and rpcgen. This can be useful for people who do not want to activate
--with-idlgen and/or --with-rpcgen.
* Initial integration tests using Docker
* Implement tirpc fixes for OpenSSL
* Propagate the binding patches to the standalone mode.
* Add UNIX socket integration tests and more tirpc fixes

Co-authored-by: Simon Chopin <simon.chopin@canonical.com>
Co-authored-by: Ryad Benadjila <ryadbenadjila@gmail.com>
@calderonth calderonth closed this Aug 27, 2021
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.

None yet

3 participants