Nix: provide stacks-common

[malikwirin]

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

• serves clarinet#1515

Additional info (benefits, drawbacks, caveats)

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

[CLAassistant]

All committers have signed the CLA.

[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.

[malikwirin]

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: stacks-network/clarity-wasm#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

[aldur]

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):

stacks-core/.cargo/config.toml

Line 17 in fa0a21c

clarity = { path = "./clarity" }

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]

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;
         };

[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

[aldur]

If we wait for 627 to merge, then we can get rid of this.

[aldur]

Can you add a comment to remember us why we need this? Effectively, this is disabling pre-building any cargo dependency. I think it's an unfortunate byproduct of how the workspace is structured right now, so not much we can do until we fix that.

[malikwirin]

Would this still be needed when stacks-network/clarity-wasm#627 is merged?

[aldur]

Unfortunately yes, because of how dependencies refer to each other (with WASM depending on stacks-common).

[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.

[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.

Can or should we decrease the checking?

[aldur]

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.