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

Initialize UniFFI demo and scripts #896

Merged
merged 6 commits into from
Aug 25, 2023

Conversation

swaptr
Copy link
Contributor

@swaptr swaptr commented Jul 10, 2023

  • Initializes the base for building and testing uniffi demo application.
  • Added convenience scripts for setting up and compiling uniffi_aries_vcx project for android.

@swaptr swaptr requested a review from gmulhearn July 10, 2023 02:23
@swaptr swaptr self-assigned this Jul 10, 2023
@swaptr
Copy link
Contributor Author

swaptr commented Jul 10, 2023

@gmulhearn While building the bindings using the uniffi_aries_vcx/scripts/android.build.sh, on generating the kotlin bindings, i run into the following error:

error: linking with `cc` failed: exit status: 1

...

error: could not compile `ursa` (lib) due to previous error
warning: build failed, waiting for other jobs to finish...

For reference, I have set up the toolchain using scripts inside the uniffi_aries_vcx/scripts directory.

@codecov-commenter
Copy link

codecov-commenter commented Jul 10, 2023

Codecov Report

Merging #896 (b84c65d) into main (72744e1) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #896   +/-   ##
=======================================
  Coverage   44.44%   44.45%           
=======================================
  Files         417      417           
  Lines       28975    28975           
  Branches     6176     6176           
=======================================
+ Hits        12879    12880    +1     
+ Misses      12297    12296    -1     
  Partials     3799     3799           
Flag Coverage Δ
unittests-aries-vcx 44.45% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

@gmulhearn
Copy link
Contributor

As discussed, the solution may be:

  1. unset some of the compile env variables OPENSSL_DIR etc. as they may be interfering with the cargo run of uniffi-bindgen, or
  2. move the uniffi-bindgen step to somewhere BEFORE the setting of environment variables and building for android architectures

@swaptr swaptr force-pushed the uniffi-dev-scripts branch 2 times, most recently from e7213d6 to 2d40e6c Compare July 17, 2023 15:40
@swaptr
Copy link
Contributor Author

swaptr commented Jul 17, 2023

1. `unset` some of the compile env variables `OPENSSL_DIR` etc. as they may be interfering with the `cargo run` of uniffi-bindgen, or

You are absolutely correct @gmulhearn. This fixed it right away. Thanks.

@swaptr swaptr marked this pull request as ready for review July 17, 2023 15:44
@swaptr swaptr force-pushed the uniffi-dev-scripts branch 2 times, most recently from db8a403 to 784acc0 Compare July 18, 2023 15:12
@swaptr swaptr force-pushed the uniffi-dev-scripts branch 3 times, most recently from 7607f91 to 52495b4 Compare August 16, 2023 22:14
This was referenced Aug 18, 2023
@swaptr swaptr mentioned this pull request Aug 24, 2023
gmulhearn
gmulhearn previously approved these changes Aug 24, 2023
Copy link
Contributor

@gmulhearn gmulhearn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! nice work. As we know, this is all in a WIP state, but it's a good base for the next PRs to pull everything together.

nice work. no need to address the comment i left in this PR, we will handle it in the next PR

Signed-off-by: Swapnil Tripathi <swapnil06.st@gmail.com>
Signed-off-by: Swapnil Tripathi <swapnil06.st@gmail.com>
Signed-off-by: Swapnil Tripathi <swapnil06.st@gmail.com>
Signed-off-by: Swapnil Tripathi <swapnil06.st@gmail.com>
Signed-off-by: Swapnil Tripathi <swapnil06.st@gmail.com>
Patrik-Stas
Patrik-Stas previously approved these changes Aug 24, 2023
Copy link
Contributor

@Patrik-Stas Patrik-Stas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

Just one thing, can you add brief instructions for the build steps and running in android? For starter doesn't need to be super detailed, but to give me idea how to proceed if I want to try it out.
Also mention the current state of support - what architectures are supporter, or whether the builds work emulator / physical device.

@gmulhearn feel free to merge at your discretion

@swaptr swaptr dismissed stale reviews from Patrik-Stas and gmulhearn via 5bd34cc August 25, 2023 03:49
Signed-off-by: Swapnil Tripathi <swapnil06.st@gmail.com>
@gmulhearn gmulhearn merged commit cfea9f7 into hyperledger:main Aug 25, 2023
31 of 32 checks passed
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.

4 participants