-
Notifications
You must be signed in to change notification settings - Fork 849
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
[Ada] Initial library support #7303
Conversation
Can one of the admins verify this patch? |
Hi @Irvise , Thank for the pull request. I will look it over and I also pinged Joakim. I don't see a contrubutor agreement on file. Would you mind submitting a ticket to our support at wolfssl dot com alias referencing this PR? Thanks, |
@Irvise has been approved as a wolfSSL contributor |
@joakim-strandberg any feedback on this? Seems like a nice Ada feature having wolfSSL as a library. |
Hi all! I saw this in my e-mail just now. @Irvise , great of you to push improvements of the Ada binding forward. I saw the recorded video you organized @Irvise a couple of days ago and was very happy to see that the Ada binding to WolfSSL was the first topic of the video chat meetup. The progress I made 7 months ago with the intent of including it in Alire can be found here: https://github.com/joakim-strandberg/wolfssl/tree/ada_windows_support/wrapper/Ada , maybe something can be reused for the Alire support you are working on @Irvise ? |
Hi @Irvise , what is the current state for this PR? Could you please rebase to latest master. I think that may help resolve some of the CI issues reported. Are you planning to add more to this PR? |
Hi @dgarske, I still could not find time to properly finish it. Sorry for the delay. I will nonetheless rebase it and try to get it finished in the next couple of weeks. I may restructure the Ada bindings as in Joakim's branch too. But that shouldn't affect the use/consumption of the binding. Btw @joakim-strandberg, did you receive my reply? Sometimes my personal email address gets marked as spam :/ |
I totally didn't see this pull request. Great idea! I actually started doing the same thing ... whoops. I should probably look next time. I updated the Ada example to use DTLS here: #7397. I was going to make another pull request to make it an Alire project but you already started. I'm happy to help! |
Some initial thoughts:
|
Thanks for your feedback Daly and your work on the support for DTLS. On Saturday April the 6:th of April 15:00 Central European Time (https://forum.ada-lang.io/t/ada-monthly-meeting/384/41) at the Ada monthly meeting the wolfSSL Ada binding will be discussed. It would be great if you would have possibility of joining too. |
Ahh I'm away this weekend so I can't attend unfortunately. Happy to help otherwise. |
Hi @dalybrown
Will do :)
I was also thinking about that, but I am not sure how ergonomic this will be. But I have to give it a try nonetheless!
Yes... I am aware of the linker issues... I agree with your approach to limit the options to only the major features. I will hack a bit on this and see what I can come up with :) Cheers, |
An update from my side. I managed to create a test submission of WolfSSL to the Alire index, see alire-project/alire-index#1023. This demonstrates that Alire does work correctly even with the current project structure. I will wait for #7397 to be finished before continuing :) |
wrapper/Ada/alire.toml
Outdated
@@ -0,0 +1,15 @@ | |||
name = "wolfssl_ada" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is being nitpicky, but, is the _ada
really necessary here? By virtue of being an Alire crate, the ada
part is implied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I will take out the _ada
surname :)
I will get back to this PR then :) But I will not be available for the next week, so it will take a bit longer than I had wished for. I will also try and see if I can catch the Windows warning reported in #7397 |
I'm happy to help. (Although, the next few weeks are also busy for me ... so i'm not sure how much time I can contribute right away.) |
Well... It is better late than ever I guess... I finally took some time to take a final review at the PR. I have documented Alire in the README for the Ada wrapper and I have manually verified that SPARK still passes all checks with the latest version (14.1.1 currently). I have been thinking about how to make DTLS optional... For the time being, I would just simply let it be built by default, when someone needs to turn it off, maybe then the work could be done :) What do you think? Nonetheless, here is the approach I would follow in order to truly make it optional. I would use the I think that the AdaCore community toolchain section could be fully deleted from the README, do you agree @joakim-strandberg? Edit: turn DTLS on -> turn DTLS off |
Thank you @Irvise |
Thanks for reviving this pull request @Irvise .
|
Thanks for your feedback @joakim-strandberg. I have updated the README to reflect the changes. |
Okay to test. Contributor agreement on file. Thank you @Irvise |
Description
The current Ada wrapper and its associated build scripts create a client/server setup/example. However, this is not enough (from my understanding) in order to easily consume WolfSSL from an Ada project. WolfSSL should be built as a library using the files already present in the wrapper. This commit adds an initial implementation of the library build script that should be usable for any Ada project. It is based on the current server build script.
However, some issues/limitations have to be mentioned. I am unsure about how the required Linker flags should be added. For
library project
in GPRBuild, theLinker
package has no effect, as that is for the final executable. So I suppose the consumer of WolfSSL's library will have to add the specified linker flags.Secondly, and in my opinion, most importantly, is how to properly handle the different WolfSSL configuration options within the GPRBuild script... I know that there is
user_settings.h
and that is what currently configures the build process and all the details, but I would prefer if that was not fixed to the provided file... Of course GPRBuild supports user defined options, but I am unsure on how to properly handle the many options that WolfSSL has. I am also including in this aspect the correct handling of the different architectures! For example, I would like to try and use WolfSSL with my new ESP32-C6 (RISC-V) chip :)I also created an
alire.toml
file for WolfSSL that builds the different project files. It can be found below. Alire, for those that don't know what it is, is equivalent to Rust's Cargo or C++ Conan, a build/project manager. Once the library build option is finished I will publish the Alire recipe to the main repository for everybody's ease of use.Documentation will be filled once the scope is fixed and the library works as expected.
I am opening this PR to start gathering feedback and comments.
I am pinging @dgarske as the committer of the bulk of the bindings. I cannot find Joakim's Github user (if there is any).
P.S: this pull requests comes after a conversation that I had with Martin (from the UK division).
Testing
The current GPR file builds the
.a
library, no further checks have been performed.Checklist