Create dynamic libraries with C interfaces for Dart FFI #3098

andrei-toterman · 2023-05-25T14:28:03Z

This PR adds a new shared library called dart_ffi which contains C interfaces of some of our C++ functions which we'd like to call from Dart using FFI.
Some additional refactoring was needed in order decouple the generation of certificates from the creation of the gRPC channel, so that certificate data can be more easily retrieved using a C interface.

codecov · 2023-05-31T13:40:01Z

Codecov Report

❗ No coverage uploaded for pull request base (desktop-gui@d382be2). Click here to learn what that means.
The diff coverage is n/a.

@@              Coverage Diff               @@
##             desktop-gui    #3098   +/-   ##
==============================================
  Coverage               ?   88.48%           
==============================================
  Files                  ?      239           
  Lines                  ?    12107           
  Branches               ?        0           
==============================================
  Hits                   ?    10713           
  Misses                 ?     1394           
  Partials               ?        0

src/client/desktop_gui/ffi/dart_ffi.cpp

luis4a0

Hey @andrei-toterman! Great job here. I see us using extensively this library in the future, so we'd need to be careful in the design. But until now, I don't see any flaws in design nor in code. So please rebase the base branch from main and make sure this will merge fine into it, answer please the few questions and we are good to go!

luis4a0 · 2023-07-03T13:54:29Z

src/client/cli/client.cpp

@@ -74,7 +74,7 @@ auto make_handler_unregisterer(mp::SettingsHandler* handler)
 } // namespace

 mp::Client::Client(ClientConfig& config)
-    : stub{mp::Rpc::NewStub(mp::client::make_channel(config.server_address, config.cert_provider.get()))},
+    : stub{mp::Rpc::NewStub(mp::client::make_channel(config.server_address, *config.cert_provider))},


Isn't this config->cert_provider?

I don't think so. We need to pass a reference to make_channel and cert_provider in this case is a unique_ptr, so we need * to get that reference.

luis4a0 · 2023-07-03T13:56:49Z

src/client/common/client_common.cpp

-                                                        mp::CertProvider* cert_provider)
+                                                        const mp::CertProvider& cert_provider)


I thnk this interface update is a drive-by change. But nevermind, it's a good change so I'm perfectly OK with it.

Yeah, we could've kept the pointer but since after this refactoring we never pass a null pointer to this function, we can use a reference instead for more safety

luis4a0 · 2023-07-03T14:06:20Z

3rd-party/CMakeLists.txt

@@ -86,6 +86,7 @@ option(YAML_CPP_BUILD_CONTRIB OFF)
 option(YAML_CPP_BUILD_TESTS OFF)

 add_subdirectory(yaml-cpp EXCLUDE_FROM_ALL)
+set_target_properties(yaml-cpp PROPERTIES POSITION_INDEPENDENT_CODE ON)


Why do we need PIC here? Anyway, I'm OK with it, especially because it's third party. Just asking out of curiosity.

Without it, I was simply getting a compilation error that said it needs PIC to compile some functions in that library :)

…ation

luis4a0 · 2023-08-14T11:40:10Z

bors r+

3098: Create dynamic libraries with C interfaces for Dart FFI r=luis4a0 a=andrei-toterman This PR adds a new shared library called `dart_ffi` which contains C interfaces of some of our C++ functions which we'd like to call from Dart using FFI. Some additional refactoring was needed in order decouple the generation of certificates from the creation of the gRPC channel, so that certificate data can be more easily retrieved using a C interface. Co-authored-by: Andrei Toterman <[email protected]>

luis4a0 · 2023-08-14T14:18:31Z

bors cancel

bors · 2023-08-14T14:18:33Z

Canceled.

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch 6 times, most recently from 6b2e90f to b1b1eb9 Compare May 31, 2023 13:10

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch 3 times, most recently from a159520 to 16a7fec Compare June 7, 2023 10:09

andrei-toterman changed the base branch from main to desktop-gui June 7, 2023 10:10

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch from 16a7fec to 41a2ba0 Compare June 7, 2023 14:25

andrei-toterman force-pushed the desktop-gui branch from 28b9394 to 97092b2 Compare June 7, 2023 16:07

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch from 41a2ba0 to e2b2664 Compare June 7, 2023 16:07

andrei-toterman force-pushed the desktop-gui branch from 97092b2 to cdac76c Compare June 7, 2023 18:32

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch from e2b2664 to fc95786 Compare June 7, 2023 18:33

andrei-toterman force-pushed the desktop-gui branch from cdac76c to 20c1e44 Compare June 9, 2023 10:22

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch from fc95786 to 6d8b724 Compare June 9, 2023 10:22

andrei-toterman force-pushed the desktop-gui branch from 20c1e44 to 5dfd0bd Compare June 12, 2023 09:49

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch from 6d8b724 to 2bf493c Compare June 12, 2023 09:49

andrei-toterman force-pushed the desktop-gui branch from 5dfd0bd to 2a08190 Compare June 28, 2023 14:07

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch from 2bf493c to a11ec29 Compare June 28, 2023 14:08

ricab reviewed Jun 29, 2023

View reviewed changes

src/client/desktop_gui/ffi/dart_ffi.cpp Outdated Show resolved Hide resolved

luis4a0 approved these changes Jul 3, 2023

View reviewed changes

luis4a0 force-pushed the desktop-gui branch from 2a08190 to 0b6bc3e Compare July 4, 2023 13:02

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch 3 times, most recently from 255f615 to 3b57f26 Compare July 6, 2023 19:32

andrei-toterman force-pushed the desktop-gui branch from 0b6bc3e to 65a3e68 Compare July 18, 2023 07:49

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch from 3b57f26 to a3372f2 Compare July 18, 2023 07:50

andrei-toterman force-pushed the desktop-gui branch from 65a3e68 to 3c0494b Compare July 24, 2023 11:03

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch from a3372f2 to d05952f Compare July 24, 2023 11:05

andrei-toterman force-pushed the desktop-gui branch from 3c0494b to 78ac095 Compare August 2, 2023 09:56

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch 3 times, most recently from 820a85c to 642accf Compare August 2, 2023 11:15

luis4a0 force-pushed the desktop-gui branch 2 times, most recently from a56949c to 2220a61 Compare August 4, 2023 12:10

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch from 642accf to 91bf677 Compare August 4, 2023 12:27

luis4a0 force-pushed the desktop-gui branch from 1762b26 to 6548168 Compare August 11, 2023 14:35

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch from 91bf677 to dfa919c Compare August 11, 2023 16:02

luis4a0 force-pushed the desktop-gui branch from 6548168 to d382be2 Compare August 11, 2023 17:44

andrei-toterman added 2 commits August 14, 2023 10:43

[client/common] decouple certificate generation from grpc channel cre…

38c2e60

…ation

[gui] add shared library with c interfaces for ffi

239d612

andrei-toterman force-pushed the dyn-libs-with-c-interfaces branch from dfa919c to 239d612 Compare August 14, 2023 07:43

luis4a0 merged commit 590e625 into desktop-gui Aug 14, 2023
12 of 14 checks passed

bors bot deleted the dyn-libs-with-c-interfaces branch August 14, 2023 14:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create dynamic libraries with C interfaces for Dart FFI #3098

Create dynamic libraries with C interfaces for Dart FFI #3098

andrei-toterman commented May 25, 2023 •

edited

Loading

codecov bot commented May 31, 2023 •

edited

Loading

luis4a0 left a comment

luis4a0 Jul 3, 2023

andrei-toterman Jul 6, 2023

luis4a0 Jul 3, 2023

andrei-toterman Jul 6, 2023

luis4a0 Jul 3, 2023

andrei-toterman Jul 6, 2023

luis4a0 commented Aug 14, 2023

luis4a0 commented Aug 14, 2023

bors bot commented Aug 14, 2023

		mp::CertProvider* cert_provider)
		const mp::CertProvider& cert_provider)

Create dynamic libraries with C interfaces for Dart FFI #3098

Create dynamic libraries with C interfaces for Dart FFI #3098

Conversation

andrei-toterman commented May 25, 2023 • edited Loading

codecov bot commented May 31, 2023 • edited Loading

Codecov Report

luis4a0 left a comment

Choose a reason for hiding this comment

luis4a0 Jul 3, 2023

Choose a reason for hiding this comment

andrei-toterman Jul 6, 2023

Choose a reason for hiding this comment

luis4a0 Jul 3, 2023

Choose a reason for hiding this comment

andrei-toterman Jul 6, 2023

Choose a reason for hiding this comment

luis4a0 Jul 3, 2023

Choose a reason for hiding this comment

andrei-toterman Jul 6, 2023

Choose a reason for hiding this comment

luis4a0 commented Aug 14, 2023

luis4a0 commented Aug 14, 2023

bors bot commented Aug 14, 2023

andrei-toterman commented May 25, 2023 •

edited

Loading

codecov bot commented May 31, 2023 •

edited

Loading