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.

Sign up for GitHub

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

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

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/mac-build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ jobs:
- name: Build play-cpp-sdk library
run: ./checkmac.sh && cargo build --package play-cpp-sdk --release --target x86_64-apple-darwin

- name: Test building demo project
working-directory: demo
run: make x86_64_build_test

- name: Build demo project
working-directory: demo
run: make x86_64_build
Expand All @@ -40,6 +44,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
Copy link
Contributor

@damoncro damoncro Jun 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok, i'm checking now

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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)

Copy link
Contributor

@damoncro damoncro Jun 24, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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:

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, i'll modify

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

otool -L ./lib/libplay_cpp_sdk.dylib
tar zcvf ../play_cpp_sdk_${PLATFORM}.tar.gz *
cd ..
shasum -a 256 *.tar.gz > "checksums-$PLATFORM.txt"
Expand Down
9 changes: 2 additions & 7 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,9 @@ The following build events are included in the project file:
git clone https://github.com/crypto-com/play-cpp-sdk.git
```
2. Unzip the archive file into `demo` folder
3. Copy the dynamic library to `/usr/local/lib`
``` sh
cd demo
cp lib/libplay_cpp_sdk.dylib /usr/local/lib
```
4. Under `demo` folder and build the `demo` project
3. Under `demo` folder and build the `demo` project
``` sh
make
make x86_64_build
```

### Linux
Expand Down
41 changes: 39 additions & 2 deletions demo/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,17 @@ dynamic: easywsclient.o
$(FLAGS) \
-L lib \

x86_64_build: prepare_x86_64 easywsclient.o.x86_64
# test whether dylib loading correctly
x86_64_test_dylib:
./demo test

x86_64_build: x86_64_build_static x86_64_build_dynamic

x86_64_build_test:x86_64_build_dynamic x86_64_test_dylib

x86_64_build_static: prepare_x86_64 easywsclient.o.x86_64
arch -x86_64 \
g++ -o demo \
g++ -o demostatic \
easywsclient.o \
main.cc \
chainmain.cc \
Expand All @@ -65,6 +73,35 @@ x86_64_build: prepare_x86_64 easywsclient.o.x86_64
-std=c++14 \
$(FLAGS)

# use -rpath option to add the path to the dynamic library loading path
# -rpath lib in gcc or
# 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
Copy link
Contributor

Choose a reason for hiding this comment

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

g++ has -install_name option.

otool -L ./lib/libplay_cpp_sdk.dylib
arch -x86_64 \
g++ -o demo \
easywsclient.o \
main.cc \
chainmain.cc \
cronos.cc \
extra.cc \
./include/walletconnectcallback.cc \
./include/nft.cc \
./include/pay.cc \
./include/defi-wallet-core-cpp/src/contract.rs.cc \
./include/defi-wallet-core-cpp/src/core.cc \
./include/defi-wallet-core-cpp/src/nft.rs.cc \
./include/defi-wallet-core-cpp/src/uint.rs.cc \
./include/extra-cpp-bindings/src/lib.rs.cc \
lib/libplay_cpp_sdk.dylib \
lib/libcxxbridge1.a \
-std=c++14 \
-rpath lib \
$(FLAGS)


run_static:
. ./.env && ./demostatic

Expand Down
37 changes: 25 additions & 12 deletions demo/main.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,34 @@
#include <thread>

int main(int argc, char *argv[]) {
try {
chainmain_process(); // chain-main
test_chainmain_nft(); // chainmain nft tests
test_login(); // decentralized login
cronos_process(); // cronos
test_cronos_testnet(); // cronos testnet
} catch (const rust::cxxbridge1::Error &e) {
// Use `Assertion failed`, the same as `assert` function
if (argc >= 2) {
if ("test" == std::string(argv[1])) {
// check wheter calling works
try {
test_login();
std::cout<<"OK"<<std::endl;
} catch (const std::exception &e) {
std::cout << "Assertion failed: " << e.what() << std::endl;
}
return 0;
}
}

test_interval();
try {
chainmain_process(); // chain-main
test_chainmain_nft(); // chainmain nft tests
test_login(); // decentralized login
cronos_process(); // cronos
test_cronos_testnet(); // cronos testnet
} catch (const rust::cxxbridge1::Error &e) {
// Use `Assertion failed`, the same as `assert` function
std::cout << "Assertion failed: " << e.what() << std::endl;
}

test_blackscout_cronoscan();
test_wallet_connect();
test_interval();

return 0;
test_blackscout_cronoscan();
test_wallet_connect();

return 0;
}