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

Jump to bottom

fix: iOS linker error workaround #3401

Merged
merged 1 commit into from
Oct 1, 2021
Merged

fix: iOS linker error workaround #3401

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
4 changes: 4 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ jobs:
sudo apt-get update && \
sudo apt-get -y install \
libssl-dev \
openssl \
libsqlite3-dev \
clang-10 \
pkg-config \
git \
Expand Down Expand Up @@ -119,6 +121,8 @@ jobs:
sudo apt-get update && \
sudo apt-get -y install \
libssl-dev \
openssl \
libsqlite3-dev \
pkg-config \
git \
cmake \
Expand Down
64 changes: 9 additions & 55 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion base_layer/wallet/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,6 @@ env_logger = "0.7.1"
prost = "0.8.0"

[features]
default=["bundled_sqlite"]
c_integration = []
Copy link
Contributor

Choose a reason for hiding this comment

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

So instead of just reverting these changes lets try and clean up the feature situation. As far as I can tell this c_integration flag doesn't do anything anymore so lets remove it.

Having Sqlite bundled by default seems correct to me so I would say leave this in and then we make sure that when the wallet_ffi build happens it doesn't use these features.

avx2 = ["tari_crypto/avx2", "tari_core/avx2"]
bundled_sqlite = ["libsqlite3-sys"]
19 changes: 12 additions & 7 deletions base_layer/wallet_ffi/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ version = "0.18.6"
edition = "2018"

[dependencies]
tari_comms = { version = "^0.10", path = "../../comms" }
tari_comms = { version = "^0.10", path = "../../comms", features = ["c_integration"]}
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this feature even get used in the tari_comms crate anymore?

tari_comms_dht = { version = "^0.10", path = "../../comms/dht", default-features = false }
tari_common_types = {path="../common_types"}
tari_crypto = { git = "https://github.com/tari-project/tari-crypto.git", branch = "main" }
tari_key_manager = { version = "^0.10", path = "../key_manager" }
tari_p2p = { version = "^0.10", path = "../p2p" }
tari_wallet = { version = "^0.10", path = "../wallet", default-features = false }
tari_wallet = { version = "^0.10", path = "../wallet", features = ["c_integration"]}
Copy link
Contributor

Choose a reason for hiding this comment

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

Pretty sure that the c_integration flag does nothing in the wallet lib, the default-features = false should mean that this crate does not use the bundled sql lib when building wallet. Not using the bundled Sqlite should be an exception to the default.

tari_shutdown = { version = "^0.10", path = "../../infrastructure/shutdown" }
tari_utilities = "^0.3"

Expand All @@ -26,6 +26,16 @@ rand = "0.8"
thiserror = "1.0.26"
tokio = "1.11"

# <workaround>
# Temporary workaround until crates utilizing openssl have been updated from security-framework 2.4.0
# which is currently broken for iOS
[target.x86_64-apple-ios.dependencies]
security-framework = "2.4.2"

[target.aarch64-apple-ios.dependencies]
security-framework = "2.4.2"
# </workaround>

[dependencies.tari_core]
path = "../../base_layer/core"
version = "^0.10"
Expand All @@ -35,11 +45,6 @@ features = ["transactions"]
[lib]
crate-type = ["staticlib","cdylib"]

[features]
default = ["bundled_sqlite", "bundled_openssl"]
bundled_openssl= [ "tari_comms/bundled_openssl"]
bundled_sqlite = ["tari_wallet/bundled_sqlite"]

[dev-dependencies]
tempfile = "3.1.0"
lazy_static = "1.3.0"
Expand Down
20 changes: 13 additions & 7 deletions base_layer/wallet_ffi/mobile_build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,19 @@ if [ -n "${DEPENDENCIES}" ] && [ -n "${PKG_PATH}" ] && [ "${BUILD_IOS}" -eq 1 ]
export PKG_CONFIG_PATH=${PKG_PATH}
# shellcheck disable=SC2028
echo "\t${CYAN}Building Wallet FFI${NC}"
cargo-lipo lipo --release --no-default-features > "${IOS_LOG_PATH}/cargo.txt" 2>&1
cargo build --lib --release --target=x86_64-apple-ios > "${IOS_LOG_PATH}/cargo_ios_x86_64.txt" 2>&1
cargo build --lib --release --target=aarch64-apple-ios > "${IOS_LOG_PATH}/cargo_ios_aarch64.txt" 2>&1
cd ../..
cd target || exit
# Copy the fat library (which contains symbols for all built iOS architectures) created by the lipo tool
# XCode will select the relevant set of symbols to be included in the mobile application depending on which arch is built
mkdir -p universal/release
cd universal || exit
cd release || exit
# Create the fat library from the thin ones.
cp "../../x86_64-apple-ios/release/libtari_wallet_ffi.a" "${PWD}/libtari_wallet_ffi_x86_64.a"
cp "../../aarch64-apple-ios/release/libtari_wallet_ffi.a" "${PWD}/libtari_wallet_ffi_aarch64.a"
lipo -create libtari_wallet_ffi_x86_64.a libtari_wallet_ffi_aarch64.a -output libtari_wallet_ffi.a
# Copy the fat library (which contains symbols for all built iOS architectures) created by the lipo tool
# XCode will select the relevant set of symbols to be included in the mobile application depending on which arch is built
cp libtari_wallet_ffi.a "${DEPENDENCIES}/MobileWallet/TariLib/"
cd ../../.. || exit
rm -rf target
Expand Down Expand Up @@ -392,9 +398,9 @@ EOF
if [ "${MACHINE}" == "Mac" ]; then
if [ "${MAC_MAIN_VERSION}" -le 10 ]; then
if [ "${MAC_SUB_VERSION}" -ge 15 ]; then
cargo build --lib --release --no-default-features > "${ANDROID_LOG_PATH}/cargo_${PLATFORMABI}_${LEVEL}.txt" 2>&1
cargo build --lib --release > "${ANDROID_LOG_PATH}/cargo_${PLATFORMABI}_${LEVEL}.txt" 2>&1
else
cargo ndk --target ${PLATFORMABI} --android-platform ${LEVEL} -- build --release --no-default-features > "${ANDROID_LOG_PATH}/cargo_${PLATFORMABI}_${LEVEL}.txt" 2>&1
cargo ndk --target ${PLATFORMABI} --android-platform ${LEVEL} -- build --release > "${ANDROID_LOG_PATH}/cargo_${PLATFORMABI}_${LEVEL}.txt" 2>&1
fi
else
# Fix for lmdb-sys compilation for armv7 on Big Sur
Expand All @@ -405,7 +411,7 @@ EOF
# shellcheck disable=SC2028
echo "\t${CYAN}Extraction complete, continuing build ${NC}"
fi
cargo build --lib --release --no-default-features > "${ANDROID_LOG_PATH}/cargo_${PLATFORMABI}_${LEVEL}.txt" 2>&1
cargo build --lib --release > "${ANDROID_LOG_PATH}/cargo_${PLATFORMABI}_${LEVEL}.txt" 2>&1
if [ "${PLATFORMABI}" == "armv7-linux-androideabi" ]; then
BACKTRACK=${PWD}
# shellcheck disable=SC2028
Expand All @@ -416,7 +422,7 @@ EOF
fi
fi
else
cargo ndk --target ${PLATFORMABI} --android-platform ${LEVEL} -- build --release --no-default-features > "${ANDROID_LOG_PATH}/cargo_${PLATFORMABI}_${LEVEL}.txt" 2>&1
cargo ndk --target ${PLATFORMABI} --android-platform ${LEVEL} -- build --release > "${ANDROID_LOG_PATH}/cargo_${PLATFORMABI}_${LEVEL}.txt" 2>&1
fi
cp wallet.h "${DEPENDENCIES}/"
rm -rf .cargo
Expand Down
2 changes: 1 addition & 1 deletion comms/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,6 @@ tempfile = "3.1.0"
tari_common = { version = "^0.10", path = "../common", features = ["build"] }

[features]
bundled_openssl = ["openssl-sys/vendored"]
c_integration = []
Copy link
Contributor

Choose a reason for hiding this comment

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

now is this correct? All the other clients of tari_comms would like to bundle openssl but we are removing that option because just the FFI crate doesn't like it. The FFI crate should just not use this feature.

The problem of course is that Core and the Wallet crates use Comms and they both by default use the bundled features in comms. So we should make a feature in Core and Wallet that excludes this feature from Comms. That way Wallet_FFI can choose to use non-default features to exclude this bundling rather than making the default to exclude it.

avx2 = ["tari_crypto/avx2"]
rpc = ["tower-make"]