Update to scaffolding to new holonix

[c12i]

Versions:

• [x] 0.4
• [x] 0.3

Emerging issues:

• [x] Conditionally add package manager nixpkgs (#373)

[guillemcordoba]

Hey @c12i ! Just reviewing the new format of the scaffolded flake.nix files.

Why are we both adding the inputsFrom = [ inputs'.holonix.devShells ] and packages = [<ALL PACKAGES INSIDE THE DEVSHELL FROM HOLONIX>]? Firstly it is redundant to do so as the inputsFrom attribute gathers all the packages from the holonix devShelll and includes them in the caller devShell. But secondly, I just found myself with a problem in shipyard apps, where the shipyard is exporting a devShell containing a special version of rust, but it was being overriden by the fact that rust is present in the packages attribute, which takes preference over the inputsFrom.

I would prefer the holonix packages not be redeclared again, and also why are we adding all possible package managers to the flake? Feels like we are forcing everyone to download all package managers when they only need one.

Am I missing something @ThetaSinner ?

[c12i]

I would prefer the holonix packages not be redeclared again, and also why are we adding all possible package managers to the flake? Feels like we are forcing everyone to download all package managers when they only need one.

Hi Guillem, yes this is a known issue, I am working on a fix for this, PR should be opened soon.

Why are we both adding the inputsFrom = [ inputs'.holonix.devShells ] and packages = []?

Interesting, so the holonix packages don't need to be added separately? Is this something you have tested out already and it works, don't have the capacity to try this out now, but if so, it actually makes sense not to include them then

[c12i]

@guillemcordoba I've investigated this further:

1. Omitting packages from packages [ .. ]:

   • I tried this approach, but it appears that inputsFrom doesn't fully gather all packages from the holonix devShell.
   • As a result, we should keep these packages explicitly defined to ensure all necessary dependencies are included.

2. Exporting a devShell with a special Rust version:

   • I'd be interested in exploring this option further.
   • Could you share the shipyard flake you mentioned? This would help better understand the problem and how to address this.

[guillemcordoba]

Hey @c12i :

Omitting the packages does work if you fix the way that you are importing the devShell from holonix. So this is the diff of what I'm talking about:

{
  description = "Flake for Holochain app development";

  inputs = {
    holonix.url = "github:holochain/holonix?ref=main";

    nixpkgs.follows = "holonix/nixpkgs";
    flake-parts.follows = "holonix/flake-parts";

  };

  outputs = inputs@{ flake-parts, ... }:
    flake-parts.lib.mkFlake { inherit inputs; } {
      systems = builtins.attrNames inputs.holonix.devShells;
      perSystem = { inputs', pkgs, ... }: {
        formatter = pkgs.nixpkgs-fmt;

        devShells.default = pkgs.mkShell {
-          inputsFrom = [ inputs'.holonix.devShells ];
+          inputsFrom = [ inputs'.holonix.devShells.default ];

-          packages = (with inputs'.holonix.packages; [
-            holochain
-            lair-keystore
-            hc-launch
-            hc-scaffold
-            hn-introspect
-            rust # For Rust development, with the WASM target included for zome builds
-          ]) ++ (with pkgs; [
-            nodejs_20
-            nodePackages.pnpm
-            yarn-berry
-            bun
-            binaryen
-
-          ]);
        };
      };
    };
}

Using this, I get all the binaries from the holonix devShell. I think this would be the correct way of handling it. I don't think the scaffolding tool should concern itself with exporting special rust versions, the nix devShells coming from the shipyard should do that.

[c12i]

I see, this makes sense now. I think this change is worth adding then. What do you think @ThetaSinner?

[ThetaSinner]

inputsFrom is only supposed to include build dependencies for the specified shell. That we've specified the shell incorrectly sounds like an issue to address. But it should not include all the packages exported by the flake, unless I'm missing something in the Nix docs.

I can take a look at Holonix tomorrow and check what's misbehaving there. At least with Rust, that is a build input and so we probably are providing that twice, that sounds like a mistake.

It was intended that people would be able to use a different Rust version than the shell provides by default. The docs are still being written for this kind of stuff in the new Holonix. In fact, that probably won't get covered in the first pass. But I'd be happy to help get that figured out.

[ThetaSinner]

@guillemcordoba Thank you for pointing out the mistake there. You are correct that the packages get pulled in twice when you fix the inputsFrom. You are welcome to use inputsFrom if that suits you but I think by default for Holonix we want to drop that. Please see the PR here -> https://github.com/holochain/holonix/pull/52

I've also added a template that demonstrates how we can use a different Rust version. I've tested that by hand and it seems to work as expected for me.

[guillemcordoba]

Yeah okey... So I think more context is needed from my side. One of the cool things that the shipyard provides is the promise of taking your freshly scaffolded app, running the scaffold-tauri-happ command and have it produce a desktop installable executable app.

For that I need to do a bunch of things, but one of the main ones is adding the shipyard as an input to the flake.nix file that the scaffolding tool creates. One of the most important things that the shipyard flake adds is a custom version of rust tailored to tauri apps and to cross compile to mobile targets. So whenever changes are made in holonix and scaffolding specially around the scaffolded flake.nix, I need to follow suit and make sure that the scaffold-tauri-happ command scaffolds the shipyard correctly. Before in the previous version of holonix, the change from the freshly scaffolded flake.nix looked like this:

    devShells.default = {
-        inputsFrom = [ inputs'.holochain.devShells.holonix ];
+        inputsFrom = [ inputs'.p2p-shipyard.devShells.holochainTauriDev inputs'.holochain.devShells.holonix ];
    };
+    devShells.androidDev = {
+        inputsFrom = [ inputs'.p2p-shipyard.devShells.holochainTauriAndroidDev inputs'.holochain.devShells.holonix ];
+    };

Now with the new structure of the proposed changes it would look like this:

    devShells.default = {
+        inputsFrom = [ inputs'.p2p-shipyard.devShells.holochainTauriDev ];

        packages = (with inputs'.holonix.packages; [
          holochain
          lair-keystore
          hc-launch
          hc-scaffold
          hn-introspect
-          rust # For Rust development, with the WASM target included for zome builds
        ]) ++ (with pkgs; [
          nodejs_20 # For UI development
          binaryen # For WASM optimisation
          # Add any other packages you need here
        ]);
    };

+    devShells.androidDev = {
+        inputsFrom = [ inputs'.p2p-shipyard.devShells.holochainTauriAndroidDev  ];
+
+        packages = (with inputs'.holonix.packages; [
+          holochain
+          lair-keystore
+          hc-launch
+          hc-scaffold
+          hn-introspect
+        ]) ++ (with pkgs; [
+          nodejs_20 # For UI development
+          binaryen # For WASM optimisation
+          # Add any other packages you need here
+        ]);
+    };

Point being, I need to actively delete the rust package from the list of packages because it takes precedence over the one that comes from inputsFrom.holochainTauriDev. I already implemented this removal, but I'd prefer it with the inputsFrom way... I think it's cleaner.

[c12i]

@guillemcordoba, to clarify: Are you suggesting that the scaffolded nix flake should not explicitly provide the Rust nixpkg in its default shell?

[guillemcordoba]

That would work, yes. In essence I'm saying it's not thaaat big a deal for me, but I have to keep up with your decisions and adapt my code to it. So you could also maintain the flake.nix as is right now if you think that it should be this way to enable devs to make their choices.

[c12i]

Cool, I would leave it there, just to not make assumptions about the hApp dev's rust installation.

Assigned you to review #379

[guillemcordoba]

I mean taking it out means that they still get it from the inputsFrom.holonix.devShells.default. For sure I would want rust to come from the nix environment.

[c12i]

For that then, I believe this would the how the devShell should look like. What do you think?

       # ...
        inputsFrom = [ inputs'.holonix.devShells.default ];

        packages = (with pkgs; [
          nodejs_20
          binaryen
        ]);

        shellHook = ''
          export PS1='\[\033[1;34m\][holonix:\w]\$\[\033[0m\] '
        '';
      };
      # ...

[guillemcordoba]

This would be my preferred choice yeah, except the shellHook. That looks much better if it's in the holonix shell, I don't think devs need to have that in their own flakes.