Problem: absolute path inside dylib (fix #123) #124

leejw51crypto · 2022-06-23T08:27:46Z

Solution: remove absolute path, use relative path instead

damoncro · 2022-06-23T08:59:36Z

Does user still need to copy to /usr/local/lib?

damoncro · 2022-06-23T09:01:32Z

Maybe tag a temporary version and test the binary release.

leejw51crypto · 2022-06-24T02:15:13Z

nope , don't need /usr/local/lib
i'll tag and test binary
test would be like this
otool -L , check whether libplay_cpp_sdk starts with @rpath (previously, it was compiling computer's absolute path)

install_name_tool -id @rpath/libplay_cpp_sdk.dylib ./libplay_cpp_sdk.dylib
otool -L ./libplay_cpp_sdk.dylib

damoncro · 2022-06-24T02:20:19Z

.github/workflows/mac-build.yml

@@ -40,6 +40,8 @@ jobs:
          cp ./LICENSE build
          cp ./CHANGELOG.md build
          cd build
+          install_name_tool -id @rpath/libplay_cpp_sdk.dylib ./lib/libplay_cpp_sdk.dylib


Modifying the path in packing probably has a side effect: the binary was not tested in integration test.

It is better to modify the build.rs or setup .cargo/config and pass the the option to compiler: rust-lang/cargo#5077

ok, i'm checking now

checked with tag, now released with @rpath like this

otool -L libplay_cpp_sdk.dylib libplay_cpp_sdk.dylib: @rpath/libplay_cpp_sdk.dylib (compatibility version 0.0.0, current version 0.0.0)

leejw51crypto · 2022-06-24T03:26:14Z

after using @rpath,
export DYLD_LIBRARY_PATH=$(PWD)/lib && ./demo
user need to run like this on mac, and need to add rpath in user's executive binary.

in packaged app, libplay_cpp_sdk.dylib should be copied in app's default rpath

ref: https://users.rust-lang.org/t/need-help-linking-to-third-party-dynamic-library/47817

it's only mac specific, windows searches dll from the same folder with exe, PATH etc.. , in order

damoncro · 2022-06-24T04:03:26Z

demo/Makefile

@@ -53,9 +53,14 @@ dynamic: easywsclient.o
 		$(FLAGS) \
 		-L lib \

-x86_64_build: prepare_x86_64 easywsclient.o.x86_64
+x86_64_run: x86_64_build
+	export DYLD_LIBRARY_PATH=$(PWD)/lib && ./demo


Could this be tested in CI?

I agree dylib should be copied into @rpath. We build in g++, unfortunately it is not an app bundle. Override DYLD_LIBRARY_PATH seems can not verify rpath works. So how about setup LD_RUNPATH_SEARCH_PATH (not tested) instead?

ok, i'll add to mac-build.yml , checking whether dynamic dylib can be called correctly

Tested LD_RUNPATH_SEARCH_PATH, can not work..

Passing -rpath lib to g++ can fix the issue. No need to export env.

g++ -o demo \ easywsclient.o \ main.cc \ chainmain.cc \ cronos.cc \ extra.cc \ include/*.cc \ include/extra-cpp-bindings/src/*.cc \ include/defi-wallet-core-cpp/src/*.cc \ lib/libcxxbridge1.a \ -lplay_cpp_sdk \ -std=c++14 \ $(FLAGS) \ -rpath lib \ -L lib \

So unreal and g++ can both use the same rpath setting like you mentioned before.

otool -L libplay_cpp_sdk.dylib libplay_cpp_sdk.dylib: @rpath/libplay_cpp_sdk.dylib (compatibility version 0.0.0, current version 0.0.0)

add rlib option and also comment, too.

damoncro · 2022-06-24T05:21:11Z

demo/Makefile

@@ -53,9 +53,14 @@ dynamic: easywsclient.o
 		$(FLAGS) \
 		-L lib \

-x86_64_build: prepare_x86_64 easywsclient.o.x86_64
+x86_64_test_dylib:
+	export DYLD_LIBRARY_PATH=$(PWD)/lib && ./demo test


No need DYLD_LIBRARY_PATH?

ok, i'll remove it

damoncro · 2022-06-24T05:27:09Z

.github/workflows/mac-build.yml

@@ -40,6 +40,8 @@ jobs:
          cp ./LICENSE build
          cp ./CHANGELOG.md build
          cd build
+          install_name_tool -id @rpath/libplay_cpp_sdk.dylib ./lib/libplay_cpp_sdk.dylib


if we pack @rpath in CI for users, the binary will be used in demo (starts from a clean repository clone), so we should keep consistent in the setup:

https://github.com/cronos-labs/play-cpp-sdk/blob/main/README.md?plain=1#L60 is no longer needed.

https://github.com/cronos-labs/play-cpp-sdk/blob/main/README.md?plain=1#L64 make should work.

yes, i'll modify

i think libplay_cpp_sdk.dylib should be delivered with @rpath, because it is not system dylib.
it will be invalid in user's envrironment.

damoncro · 2022-06-24T08:17:30Z

README.md

    ``` sh
    cd demo
-    cp lib/libplay_cpp_sdk.dylib /usr/local/lib
+    cp lib/libplay_cpp_sdk.dylib $HOME/lib


$HOME/lib maybe make things complicated... See https://stackoverflow.com/questions/57318463/macos-whats-the-correct-place-to-install-a-dylib-on-a-users-system#:~:text=%2FLibrary%2FApplication%20Support%20or%20%2FLibrary%2FFrameworks

Of course, it is ok to add a new $HOME/lib and export it with DYLD_LIBRARY_PATH. It is still a workaround... Ideally, most users just simply run the program in terminal (like other programs) without touching the env.

ok, i'll removed it, now just make will work

damoncro · 2022-06-24T09:02:08Z

README.md

-4. Under `demo` folder and build the `demo` project
-    ``` sh
-    make
+2. or Unzip the archive file into `demo` folder


After cloning the repository, demo folder doesn't have the include and lib folder, it is a must to unzip the folders and files into demo folder.

git clone or unzip the archive file

I see, but archive file here refers to the https://github.com/cronos-labs/play-cpp-sdk/blob/main/README.md?plain=1#L25, not the source codes.

Or did I get the wrong thing?

Isn't that git clone first, then unzip for example play_cpp_sdk_Darwin_x86_64.tar.gz?

yes, git clone then unzip or untar,
modified

leejw51crypto · 2022-06-24T10:34:12Z

copy to /usr/local/lib not necessary

leejw51crypto · 2022-06-24T14:04:53Z

separated mac build test, and checking now

leejw51crypto · 2022-06-24T16:40:01Z

fixed bug in Makefile (in flags..) (no \ before $(FALGS) )

damoncro · 2022-06-30T01:54:49Z

dylib linking is incorrect, I am checking. run make or make x86_64_build or make build in demo

include/extra-cpp-bindings/src/lib.rs.cc:1199:58: error: cannot initialize a variable of type 'void (com::crypto::game_sdk::WalletConnectCallback::*)(::std::unique_ptr< ::com::crypto::game_sdk::WalletConnectSessionInfo>) const' with an rvalue of type 'void (com::crypto::game_sdk::WalletConnectCallback::*)(const com::crypto::game_sdk::WalletConnectSessionInfo &) const': type mismatch at 1st parameter ('::std::unique_ptr< ::com::crypto::game_sdk::WalletConnectSessionInfo>' (aka 'unique_ptr<com::crypto::game_sdk::WalletConnectSessionInfo>') vs 'const com::crypto::game_sdk::WalletConnectSessionInfo &')
  void (::com::crypto::game_sdk::WalletConnectCallback::*onConnected$)(::std::unique_ptr<::com::crypto::game_sdk::WalletConnectSessionInfo>) const = &::com::crypto::game_sdk::WalletConnectCallback::onConnected;
                                                         ^                                                                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/extra-cpp-bindings/src/lib.rs.cc:1204:58: error: cannot initialize a variable of type 'void (com::crypto::game_sdk::WalletConnectCallback::*)(::std::unique_ptr< ::com::crypto::game_sdk::WalletConnectSessionInfo>) const' with an rvalue of type 'void (com::crypto::game_sdk::WalletConnectCallback::*)(const com::crypto::game_sdk::WalletConnectSessionInfo &) const': type mismatch at 1st parameter ('::std::unique_ptr< ::com::crypto::game_sdk::WalletConnectSessionInfo>' (aka 'unique_ptr<com::crypto::game_sdk::WalletConnectSessionInfo>') vs 'const com::crypto::game_sdk::WalletConnectSessionInfo &')
  void (::com::crypto::game_sdk::WalletConnectCallback::*onDisconnected$)(::std::unique_ptr<::com::crypto::game_sdk::WalletConnectSessionInfo>) const = &::com::crypto::game_sdk::WalletConnectCallback::onDisconnected;
                                                         ^                                                                                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/extra-cpp-bindings/src/lib.rs.cc:1209:58: error: cannot initialize a variable of type 'void (com::crypto::game_sdk::WalletConnectCallback::*)(::std::unique_ptr< ::com::crypto::game_sdk::WalletConnectSessionInfo>) const' with an rvalue of type 'void (com::crypto::game_sdk::WalletConnectCallback::*)(const com::crypto::game_sdk::WalletConnectSessionInfo &) const': type mismatch at 1st parameter ('::std::unique_ptr< ::com::crypto::game_sdk::WalletConnectSessionInfo>' (aka 'unique_ptr<com::crypto::game_sdk::WalletConnectSessionInfo>') vs 'const com::crypto::game_sdk::WalletConnectSessionInfo &')
  void (::com::crypto::game_sdk::WalletConnectCallback::*onConnecting$)(::std::unique_ptr<::com::crypto::game_sdk::WalletConnectSessionInfo>) const = &::com::crypto::game_sdk::WalletConnectCallback::onConnecting;
                                                         ^                                                                                            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/extra-cpp-bindings/src/lib.rs.cc:1214:58: error: cannot initialize a variable of type 'void (com::crypto::game_sdk::WalletConnectCallback::*)(::std::unique_ptr< ::com::crypto::game_sdk::WalletConnectSessionInfo>) const' with an rvalue of type 'void (com::crypto::game_sdk::WalletConnectCallback::*)(const com::crypto::game_sdk::WalletConnectSessionInfo &) const': type mismatch at 1st parameter ('::std::unique_ptr< ::com::crypto::game_sdk::WalletConnectSessionInfo>' (aka 'unique_ptr<com::crypto::game_sdk::WalletConnectSessionInfo>') vs 'const com::crypto::game_sdk::WalletConnectSessionInfo &')
  void (::com::crypto::game_sdk::WalletConnectCallback::*onUpdated$)(::std::unique_ptr<::com::crypto::game_sdk::WalletConnectSessionInfo>) const = &::com::crypto::game_sdk::WalletConnectCallback::onUpdated;
                                                         ^                                                                                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4 errors generated.
make: *** [dynamic] Error 1
make  15.95s user 2.48s system 86% cpu 21.210 total

damoncro · 2022-06-30T02:40:53Z

.github/workflows/mac-test.yml

+
+      - name: Build demo project
+        working-directory: demo
+        run: make x86_64_build_test


Can this command run in mac-build.yml?

yes, i'll move it

damoncro · 2022-06-30T02:41:56Z

demo/Makefile

+# install_name_tool -add_rpath @executable_path/lib ./demo
+# in mac, to use DYLD_LIBRARY_PATH, it is normalized to use $rpath in dylib
+x86_64_build_dynamic : prepare_x86_64 easywsclient.o.x86_64
+	install_name_tool -id @rpath/libplay_cpp_sdk.dylib ./lib/libplay_cpp_sdk.dylib


g++ has -install_name option.

leejw51crypto · 2022-06-30T08:19:18Z

ok, i'll fix

leejw51crypto · 2022-06-30T08:29:38Z

x86_64_build

checked on apple m1

could you clone again?
run make cppx86_64 ?

i think lib.rs.cc is not copied from target correctly.

damoncro · 2022-06-30T08:31:44Z

In #129, there may be some conflicts:

Integrate x86_64 and amd64 into one Makefile entry
Modify the rpath in Makefile as make build, so that we can pack the library after testing in ci.
Integrate cmake workflow, including dynamic build
Integrate examples into cmake to make use the dynamic library
move the bindings and library into sdk folder for easy packing and have better support in cmake (with add_subdirectory)

add x86_64 dylib build update Makefile test dylib works fix main reformat add rpath option add rpath comment add more comment remove un-necessary change readme make simple add or typo add or for windows restore readme untar restore disable test separate test change name tidyup test fix compile option add comment remove mac-test

leejw51crypto · 2022-06-30T08:34:07Z

rebased

leejw51crypto · 2022-06-30T08:35:01Z

In #129, there may be some conflicts:

Integrate x86_64 and amd64 into one Makefile entry

Modify the rpath in Makefile as make build, so that we can pack the library after testing in ci.

Integrate cmake workflow, including dynamic build

Integrate examples into cmake to make use the dynamic library

move the bindings and library into sdk folder for easy packing and have better support in cmake (with add_subdirectory)

ok, i'll rebase
if #129, uses @rpath, #129 also closes this issue.
this pr not necessary

damoncro · 2022-06-30T08:36:29Z

it still has problems on crypto-com/defi-wallet-core-rs#461, it is better to rebase after both finish...

damoncro · 2022-06-30T08:37:46Z

Ok, I will fix rapth in #129

leejw51crypto · 2022-06-30T08:38:15Z

thanks , after #129 merged, i'll close this one.

tomtau · 2022-07-13T09:32:35Z

#129 was merged, so closing this one