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

docs: Fix and refine protocol setup docs #3

Closed
wants to merge 3 commits into from
Closed

docs: Fix and refine protocol setup docs #3

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
19cd63f
Refined the example build script
danwilliams Jun 20, 2024
0af143c
Adjusted example Cargo.toml
danwilliams Jun 20, 2024
3d002ae
Fixed complete code example
danwilliams Jun 20, 2024
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
48 changes: 37 additions & 11 deletions docs/03-build/01-protocol-sdks/02-protocol-rs-sdk.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ You can now configure your project to use the Calimero SDK by adding it as a dep

```toml title="File: Cargo.toml"
[dependencies]
calimero-sdk = { git = "https://github.com/calimero-network/core" }
calimero-sdk = { git = "https://github.com/calimero-network/core/crates/sdk" }
```

Then, we need to specify a custom build profile for the most compact Wasm output:
Expand All @@ -64,8 +64,9 @@ Then, we need to specify a custom build profile for the most compact Wasm output
inherits = "release"
codegen-units = 1
opt-level = "z"
lto = true
lto = "thin"
debug = false
strip = true
panic = "abort"
overflow-checks = true
```
Expand All @@ -86,16 +87,17 @@ crate-type = ["cdylib"]

# highlight-start
[dependencies]
calimero-sdk = { git = "https://github.com/calimero-network/core" }
calimero-sdk = { git = "https://github.com/calimero-network/core/crates/sdk" }
Copy link
Member

Choose a reason for hiding this comment

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

this was correct actually, it should be

Suggested change
calimero-sdk = { git = "https://github.com/calimero-network/core/crates/sdk" }
calimero-sdk = { git = "https://github.com/calimero-network/core" }

although, I'd expect this to fail now because calimero-network/core is a private repo

Copy link
Author

Choose a reason for hiding this comment

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

I don't think it would work at all, as we use a workspace and the sdk crate is inside it...? When using the repo locally, I needed to specify the path to the crate.

Copy link
Member

Choose a reason for hiding this comment

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

as we use a workspace and the sdk crate is inside it

yeah, with git, cargo's behavior is to clone the repo, and then use it's root manifest to locate the member crate

but with the path field, you need to include the full path

even locally, if you use the git field, it needs to be the root of the repo

calimero-sdk = { git = "https://github.com/calimero-network/core" }
calimero-sdk = { git = "file:///path/to/calimero-network/core" }
calimero-sdk = { path = "/path/to/calimero-network/core/crates/sdk" }

Copy link
Author

Choose a reason for hiding this comment

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

It's probably due to it being private that it didn't work for me, then 🙂

# highlight-end

# highlight-start
[profile.app-release]
inherits = "release"
codegen-units = 1
opt-level = "z"
lto = true
lto = "thin"
debug = false
strip = true
panic = "abort"
overflow-checks = true
# highlight-end
Expand All @@ -111,25 +113,48 @@ set -e

cd "$(dirname $0)"

TARGET="${CARGO_TARGET_DIR:-../../target}"
# Check if rustup is installed
Copy link
Member

Choose a reason for hiding this comment

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

Some environments may not have rustup available, IIRC, termux was one of those platforms which had cargo and rustc in it's package manager, but not rustup.

Copy link
Member

Choose a reason for hiding this comment

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

Checking for the presence of the target's libdir is one solution to decoupling from rustup

test -d "$(rustc --target wasm32-unknown-unknown --print target-libdir)"

Copy link
Author

Choose a reason for hiding this comment

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

Perhaps if there's no rustup I should fall back to trying to use rustc directly? It's preferable to use rustup ideally.

Copy link
Member

Choose a reason for hiding this comment

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

Indifferent. But personally, I think I'd go with the simpler approach, since the only thing we need is to check for the presence of the target, that it's usable by rustc.. and not much else

Plus, this is in a doc, so I tried to keep it compact.. but at the same time Vuki's working on a template at calimero-network/core-app-template#2 so I guess that's okay.

The final shape is up to you though

if ! command -v rustup &> /dev/null; then
echo "Error: rustup is not installed"
echo "Please install rustup in order to use this build script."
exit 1
fi

rustup target add wasm32-unknown-unknown
# Check if the wasm32-unknown-unknown target is installed
if ! rustup target list --installed | grep -q "wasm32-unknown-unknown"; then
echo "Error: Target wasm32-unknown-unknown is not installed"
echo "Please run 'rustup target add wasm32-unknown-unknown' to install it."
exit 1
fi

# Get the application name from Cargo
APP_ID=$(cargo pkgid)
APP_NAME=${APP_ID#*#}
APP_NAME=${APP_NAME%%:*}
TARGET_DIR="${CARGO_TARGET_DIR:-target}"
WASM_FILE="$TARGET_DIR/wasm32-unknown-unknown/app-release/$APP_NAME.wasm"

# Build the application
cargo build --target wasm32-unknown-unknown --profile app-release
Comment on lines +130 to 138
Copy link
Member

@miraclx miraclx Jun 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

bit tempted to leave this to cargo with --message-format json but that'll require a JSON parsing step

primarily because this parsing logic assumes the folder name won't match the package name

$ cargo metadata --format-version 1 | jq -r '.workspace_members[]'
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/crates/application#[email protected]
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/crates/network#[email protected]
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/crates/primitives#[email protected]
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/crates/identity#[email protected]
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/crates/node#[email protected]
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/crates/runtime#[email protected]
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/crates/server#[email protected]
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/crates/server-primitives#[email protected]
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/crates/store#[email protected]
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/crates/sdk#[email protected]
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/crates/sdk/macros#[email protected]
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/apps/kv-store#0.1.0
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/apps/only-peers#0.1.0
path+file:///Users/miraclx/git/calimero-is-near/cali2.0-experimental/contracts/package-manager#0.1.0

as observed, the package name is only present post-# when its dir already isn't the package name

and secondarily.. it requires the developer to manually modify the build script if they're using it inside a workspace

Copy link
Member

Choose a reason for hiding this comment

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

alternatively, if we choose to rely on JSON serialization:

WASM_FILE=$(
  cargo build \
    --target wasm32-unknown-unknown --profile app-release \
    --message-format json-render-diagnostics \
  | jq -r 'select(.target.name == "kv-store") | .filenames[] | select(endswith(".wasm"))'
)

# if $wasm_file doesn't exist, maybe we didn't build the `cdylib`?  check that `lib.crate-type` is well defined

Copy link
Author

Choose a reason for hiding this comment

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

I specifically tried to avoid JSON and jq, as it introduces another dependency 🙂 Hence using cargo pkgid instead of cargo metadata.

Copy link
Author

Choose a reason for hiding this comment

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

If you feel requiring jq is okay, I can make the change...? Or otherwise, I can adjust for the situation you mentioned?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I'm torn here.. I don't like the required dependency, but at the same time, idk if any simple solution exists to get the correct default target directory for the crate (the secondary concern).

Copy link
Author

Choose a reason for hiding this comment

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

I guess it is an example... so not canon... and we do have the intention of creating a CLI, so this is somewhat temporary. In which case, maybe I am being too fussy about the extra dependency...?

Copy link
Member

Choose a reason for hiding this comment

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

It's a valid concern ngl, can't even guarantee everyone on the team has jq installed 😅

Getting the target folder wrong is also an interesting case, since cargo will implicitly create one.


mkdir -p res
# Check if the WASM file exists
if [[ ! -f "$WASM_FILE" ]]; then
echo "Error: WASM file $WASM_FILE not found"
exit 1
fi

cp $TARGET/wasm32-unknown-unknown/app-release/kv_store.wasm ./res/
```
mkdir -p res

You can optionally choose to install and use [`wasm-opt`](https://github.com/WebAssembly/binaryen), for an additional optimization step in the build script. This step is not required but can help reduce the size of the generated Wasm file:
cp "$WASM_FILE" ./res/

```bash title="File: build.sh"
# Optimize the WASM file
if command -v wasm-opt > /dev/null; then
wasm-opt -Oz ./res/kv_store.wasm -o ./res/kv_store.wasm
fi
```

You can optionally choose to install and use [`wasm-opt`](https://github.com/WebAssembly/binaryen), for an additional optimization step in the build script (the final stage above). This step is not required, but can help reduce the size of the generated Wasm file.

Don't forget to make the `build.sh` script executable:

```bash title="Terminal"
Expand Down Expand Up @@ -190,6 +215,7 @@ use calimero_sdk::{app, env};

#[app::state]
#[derive(Default, BorshSerialize, BorshDeserialize)]
#[borsh(crate = "calimero_sdk::borsh")]
Copy link
Member

Choose a reason for hiding this comment

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

line 364 as well

struct KvStore {
entries: HashMap<String, String>,
}
Expand Down
Loading