stacks-core icon indicating copy to clipboard operation
stacks-core copied to clipboard

Nix: provide stacks-common

Open malikwirin opened this issue 7 months ago • 9 comments

Description

This PR aims to provide stacks-common via its flake.nix for it to be used in a nix package definition of clarinet.

Applicable issues

Additional info (benefits, drawbacks, caveats)

The impurity in the build process of clar2wasm issue gets patched in the flake.nix

malikwirin avatar Mar 31 '25 10:03 malikwirin

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Mar 31 '25 10:03 CLAassistant

Thanks @malikwirin! Incidentally I did have a local patch that fixes the clarity-wasm behavior as well.

I see this is marked as WIP. Let me know once it's ready for review and I'll take a look.

aldur avatar Mar 31 '25 14:03 aldur

Thanks @malikwirin! Incidentally I did have a local patch that fixes the clarity-wasm behavior as well.

I see this is marked as WIP. Let me know once it's ready for review and I'll take a look.

I already have a PR that fixes the clarity-wasm immutable source issue: https://github.com/stacks-network/clarity-wasm/pull/627 but maybe you got a more elegant solution

Using the patch causes the build of stacks-common to fail for a new reason which I am not yet really sure about. A short excerpt:

error: builder for '/nix/store/fzqrgjl9fvgj5fm74zcfpf27wkp0im74-stacks-core-deps-3.1.0.0.7.drv' failed with exit code 101;
       last 25 log lines:
       >     |
       > 750 |             ResponseType(types) => ResponseType(Box::new((
       >     |                                    ^^^^^^^^^^^^ not found in this scope
       >
       > error[E0425]: cannot find function, tuple struct or tuple variant `OptionalType` in this scope
       >    --> /nix/store/n42d4y5py8criixbv11z9q3qlrv494vp-vendor-cargo-deps/cbadf0ed6a02207cb98bfb6185e33516466bafbec7c853b68ed04eea9f905ddd/clar2wasm-0.1.0/src/wasm_generator.rs:754:39
       >     |
       > 754 |             OptionalType(value_ty) => OptionalType(Box::new(self.type_for_serialization(value_ty))),
       >     |                                       ^^^^^^^^^^^^ not found in this scope
       >
       > error[E0425]: cannot find function, tuple struct or tuple variant `SequenceType` in this scope
       >    --> /nix/store/n42d4y5py8criixbv11z9q3qlrv494vp-vendor-cargo-deps/cbadf0ed6a02207cb98bfb6185e33516466bafbec7c853b68ed04eea9f905ddd/clar2wasm-0.1.0/src/wasm_generator.rs:756:17
       >     |
       > 756 |                 SequenceType(SequenceSubtype::ListType(
       >     |                 ^^^^^^^^^^^^ not found in this scope
       >
       > error[E0425]: cannot find function, tuple struct or tuple variant `TupleType` in this scope
       >    --> /nix/store/n42d4y5py8criixbv11z9q3qlrv494vp-vendor-cargo-deps/cbadf0ed6a02207cb98bfb6185e33516466bafbec7c853b68ed04eea9f905ddd/clar2wasm-0.1.0/src/wasm_generator.rs:764:36
       >     |
       > 764 |             TupleType(tuple_ty) => TupleType(
       >     |                                    ^^^^^^^^^ not found in this scope
       >
       > Some errors have detailed explanations: E0408, E0412, E0422, E0425, E0432, E0433, E0531.
       > For more information about an error, try `rustc --explain E0408`.
       > error: could not compile `clar2wasm` (lib) due to 260 previous errors

malikwirin avatar Mar 31 '25 14:03 malikwirin

I did a bit of digging and I think the problem is as follows:

clarity-wasm depends on clarity and stacks-common. When building from the clarity-wasm-develop branch, there's a config.toml file which overrides those dependencies with the ones in the workspace (instead of git): https://github.com/stacks-network/stacks-core/blob/fa0a21cb5a72a314f5f5fd389e6fc746dbb91738/.cargo/config.toml#L17

It seems crane is not correctly picking that up. It should be possible to either "teach" crane or patch the dependency specification in clarity-wasm to fix the issue.

aldur avatar Apr 01 '25 14:04 aldur

Update, I think I made quite some progress. So the reason we had that failure is that when crane builds dependencies it uses "dummy" Rust files -- because it only cares about the contents of the Cargo.toml. But in our case, we actually need the contents of clarity and stacks-common, plus everything else! (e.g. versions.toml, the Clarity contracts, etc).

So the fix was to add dummySrc = commonArgs.src; to craneLib.buildDepsOnly (I had to look at crane's source code to understand this).

For completeness, here's the full flake.nix I am using:

diff --git i/contrib/nix/flake.nix w/contrib/nix/flake.nix
index afd88e8a61..74cc7c7636 100644
--- i/contrib/nix/flake.nix
+++ w/contrib/nix/flake.nix
@@ -48,7 +48,7 @@
         version = versions.stacks_node_version;
 
         # Common arguments can be set here to avoid repeating them later
-        commonArgs = {
+        baseArgs = {
           strictDeps = true;
 
           buildInputs =
@@ -59,6 +59,46 @@
               # Darwin specific inputs
               pkgs.darwin.apple_sdk.frameworks.SystemConfiguration
             ];
+
+          src = fileSetForCrate ../..;
+          inherit version;
+        };
+
+        isClarityWASM = p: lib.hasPrefix "git+https://github.com/stacks-network/clarity-wasm.git" p.source;
+
+        cargoVendorDir = craneLib.vendorCargoDeps (
+          baseArgs
+          // {
+            # Use this function to override crates coming from git dependencies
+            overrideVendorGitCheckout =
+              ps: drv:
+              # For example, patch a specific repository and tag, in this case num_cpus-1.13.1
+              if lib.any (p: isClarityWASM p) ps then
+                drv.overrideAttrs (_old: {
+                  patches = [
+                    (builtins.fetchurl {
+                      url = "https://github.com/stacks-network/clarity-wasm/pull/627.patch";
+                      sha256 = "sha256:161mx1m21770lrsc2lfqlwzyydhy8f9bc7pmqb26rcki7s2ar31r";
+                    })
+                  ];
+                })
+              else
+                # Nothing to change, leave the derivations as is
+                drv;
+
+            # Use this function to override crates coming from any registry checkout
+            overrideVendorCargoPackage = p: drv: drv;
+          }
+        );
+
+        commonArgs = baseArgs // {
+          inherit cargoVendorDir;
         };
 
         # Build *just* the cargo dependencies, so we can reuse
@@ -66,9 +106,8 @@
         cargoArtifacts = craneLib.buildDepsOnly (
           commonArgs
           // {
-            inherit version;
             pname = name;
-            src = fileSetForCrate ../..;
+            dummySrc = commonArgs.src;
           }
         );
 
@@ -85,8 +124,8 @@
           lib.fileset.toSource {
             root = ../..;
             fileset = lib.fileset.unions [
-              ../../Cargo.toml
-              ../../Cargo.lock
+              (craneLib.fileset.commonCargoSources ../..)
+              (craneLib.fileset.configToml ../..)
               #
               ../../versions.toml
               #
@@ -126,6 +165,16 @@
           }
         );
 
+        stacks-common = craneLib.buildPackage (
+          individualCrateArgs
+          // rec {
+            pname = "stacks-common";
+            cargoFeatures = "--features slog_json";
+            cargoExtraArgs = "${cargoFeatures} -p ${pname}";
+            src = fileSetForCrate ../../stacks-common;
+          }
+        );
+
         # Build the actual crate itself, reusing the dependency
         # artifacts from above.
         stacks-core = craneLib.buildPackage (
@@ -143,7 +192,7 @@
       with pkgs;
       {
         packages = {
-          inherit stacks-signer;
+          inherit stacks-signer stacks-common cargoArtifacts;
           default = stacks-core;
         };

aldur avatar Apr 01 '25 18:04 aldur

@aldur I believe its ready for review. The build of stacks-common doesn't currently terminate on the computer I have currently available because it only has 16gb of ram

malikwirin avatar Apr 02 '25 09:04 malikwirin

@aldur I believe its ready for review. The build of stacks-common doesn't currently terminate on the computer I have currently available because it only has 16gb of ram

I have tried building it and it succeeded, despite the build taking 22 minutes.

aldur avatar Apr 02 '25 09:04 aldur

@aldur I believe its ready for review. The build of stacks-common doesn't currently terminate on the computer I have currently available because it only has 16gb of ram

I have tried building it and it succeeded, despite the build taking 22 minutes.

Can or should we decrease the checking?

malikwirin avatar Apr 02 '25 10:04 malikwirin

Can or should we decrease the checking?

We should but I am not sure we can -- I'll compare with a build from develop and see how long that will take so we have a benchmark. If this makes things significantly worse, then it means we are doing duplicate work in this PR that could be removed.

EDIT: I did two builds, one from develop and the other from this branch and they actually completed roughly in the same time (8m). The previous build might have been due to having to download a few things from Nix cache due to the bump in the flake.lock. I'd say we are good.

aldur avatar Apr 02 '25 10:04 aldur

Closing this for now since there's an alternative solution for Clarinet: https://github.com/hirosystems/clarinet/issues/1515#issuecomment-2809682536.

We can revisit this if it becomes relevant again.

aldur avatar Jul 07 '25 08:07 aldur

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

github-actions[bot] avatar Jul 15 '25 00:07 github-actions[bot]