Improved resource handling #48

rot256 · 2025-01-28T19:50:38Z

In relation to issue #44

This PR reworks the way resources such as the stone-prover binary, are handled by the stone-cli. Previously, build.rs would retrieve the resources and using commands create the required directory structure in $HOME/.stone-cli.

This meant that:

Building the stone-cli would change the users environment
It could leave stale/old versions of the various resources in .stone-cli which would not get overwritten.
You could not build the tool on one machine and run it on another.

The new system ensures that:

The stone-cli works even if build on a different machine (e.g. we can distribute binaries now).
Does not modify the users home directory.
Allows uninstalling by using cargo uninstall (simply deleting the stone-cli binary will do).

Overall, the approach taken is:

Retrieve all the resources for the build
Create the desired directory structure in build.rs
Create a tarball with the directory structure
Embed the tarball inside the stone-cli binary
Upon execution of the stone-cli binary extract the tarball to a "temporary" directory, ensuring however that this is only done on the first execution, as oppose to creating a fresh "TempDir" every time.

The negative results of this approach are:

Slightly longer startup when using the stone-cli the first time after reboot
A larger stone-cli binary.

This commit reworks the way resources such a the stone-prover binary, are handled by the stone-cli. The new system ensures that: - The stone-cli works even if build on a different machine and moved. - Does not modify the users home directory - Allows uninstalling by using `cargo uninstall` (simply deleting the stone-cli binary will do). Overall, the approach taken: - Retrieve all the resources for the build - Create the desired directory structure in build.rs - Create a tarball with the directory structure - Embed the tarball inside the stone-cli binary - Upon execution of the stone-cli binary extract the tarball to a "temporary" directory, ensuring however that this is only done on the first execution, as oppose to creating a fresh "TempDir" every time.

Use the OUT_DIR instead of src/

Use that rename is atomic, rather than creating a flag file

rot256 · 2025-01-29T14:29:28Z

This PR still depends on the cairo1-run binary, perhaps we should merge the large PR on dynamic layouts first and I can get rid of the cairo1-run artifact in the subsequent rebase.

mellowcroc · 2025-02-03T14:48:01Z

Code looks really nice, thanks! Left some minor comments.

This PR still depends on the cairo1-run binary, perhaps we should merge the large PR on dynamic layouts first and I can get rid of the cairo1-run artifact in the subsequent rebase.

This sounds good to me.

mellowcroc · 2025-02-03T06:46:55Z

build.rs

+    (RES_STONE_V6_VERIFIER, BIN_STONE_V6_VERIFIER),
+];
+
+fn target_dir() -> Result<PathBuf, anyhow::Error> {


seems like we can guarantee CARGO_TARGET_DIR value by setting it on .cargo/config.toml?

... [build] target-dir = "path/to/target"

mellowcroc · 2025-02-03T13:21:13Z

build.rs

+    let tar = flate2::write::GzEncoder::new(tar_gz, flate2::Compression::default());
+    let mut archive = tar::Builder::new(tar);
+
+    // decompress the corelib tarball and add "cario/corelib" as "corelib" to the archive


nit) cario -> cairo

mellowcroc · 2025-02-03T14:00:56Z

build.rs

+const RES_STONE_V6_PROVER: &str = "bin:v6-prover";
+const RES_STONE_V5_VERIFIER: &str = "bin:v5-verifier";
+const RES_STONE_V6_VERIFIER: &str = "bin:v6-verifier";
+const RES_CAIRO_1RUN: &str = "tar-gz:cairo1-run";


nit) RES_CAIRO_1RUN -> RES_CAIRO1_RUN?

mellowcroc · 2025-02-03T14:01:03Z

build.rs

+const BIN_STONE_V6_PROVER: &str = "cpu_air_prover_v6";
+const BIN_STONE_V5_VERIFIER: &str = "cpu_air_verifier_v5";
+const BIN_STONE_V6_VERIFIER: &str = "cpu_air_verifier_v6";
+const BIN_CAIRO_1RUN: &str = "cairo1-run";


mellowcroc · 2025-02-03T14:16:41Z

src/resources.rs

@@ -0,0 +1,18 @@
+// This file is generated by build.rs"


nit) remove "

mellowcroc · 2025-02-03T14:39:23Z

src/setup.rs

+];
+
+fn copy_resources(uid: u32, mode: u32) -> anyhow::Result<()> {
+    // if the flag file exists, return: setup is already done


flag file -> resource root directory

mellowcroc · 2025-02-03T14:44:37Z

src/setup.rs

+    let dir = resources::resource_dir(meta.uid());
+    for (env_name, filename) in ENV_CONFIGURE.iter() {
+        let full_path = dir.join(filename);
+        debug_assert!(


does this not need to be assert!?

mellowcroc · 2025-02-03T14:46:36Z

src/setup.rs

+            "File not found: {:?}",
+            full_path
+        );
+        if std::env::var(env_name).is_err() {


should we overwrite instead of not writing if it doesn't exist?

also update to use fork of `integrity` since the original repo is not up-to-date with the current version of the `sncast` command

rot256 · 2025-02-08T16:13:22Z

I attempted to merge with the dynamic layout PR (merge with main).

However, I'm getting errors on all the cairo1 related tests. Probably because I setup the corelibs directory wrong when switching to the new resource handling.

I also got rid of the environment variables used to point to the various resources and the need to call "setup" explicitly.

mellowcroc · 2025-02-15T02:41:41Z

Looks good to merge! (The cairo1 failing issue was due to downloading the wrong version of corelibs in build.rs)

rot256 requested a review from mellowcroc January 28, 2025 19:51

rot256 force-pushed the improved-resource-handling branch from c28d739 to 7d56e00 Compare January 28, 2025 19:53

rot256 added 5 commits January 29, 2025 11:00

feat: use OUT_DIR

cb2c2b8

Use the OUT_DIR instead of src/

feat: use atomicity of rename instead of flag

fa229a2

Use that rename is atomic, rather than creating a flag file

chore: fix clippy hints

7cdad45

fix: missing import in tests

2491f3a

chore: simplify build.rs

986f952

mellowcroc reviewed Feb 3, 2025

View reviewed changes

rot256 and others added 3 commits February 3, 2025 16:52

minor: incorporate review feedback

976639e

fix: update tests.sh for compatibility with the integrity verifier

54b094c

also update to use fork of `integrity` since the original repo is not up-to-date with the current version of the `sncast` command

Merge branch 'main' into improved-resource-handling

fd55d5d

mellowcroc added 2 commits February 14, 2025 15:37

fix: fix corelib version

df60030

Merge branch 'main' into improved-resource-handling

52f361d

mellowcroc approved these changes Feb 15, 2025

View reviewed changes

rot256 merged commit f5eacef into main Feb 17, 2025
2 checks passed

rot256 mentioned this pull request Feb 17, 2025

Project modifies HOME directory on open #44

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved resource handling #48

Improved resource handling #48

rot256 commented Jan 28, 2025 •

edited

Loading

rot256 commented Jan 29, 2025

mellowcroc commented Feb 3, 2025

mellowcroc Feb 3, 2025

mellowcroc Feb 3, 2025

mellowcroc Feb 3, 2025

mellowcroc Feb 3, 2025

mellowcroc Feb 3, 2025

mellowcroc Feb 3, 2025

mellowcroc Feb 3, 2025

mellowcroc Feb 3, 2025

rot256 commented Feb 8, 2025

mellowcroc commented Feb 15, 2025

Improved resource handling #48

Improved resource handling #48

Conversation

rot256 commented Jan 28, 2025 • edited Loading

rot256 commented Jan 29, 2025

mellowcroc commented Feb 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rot256 commented Feb 8, 2025

mellowcroc commented Feb 15, 2025

rot256 commented Jan 28, 2025 •

edited

Loading