Build-time nushell config generation

[jaredmontoya]

Description

Instead of running programs that print their shell config to stdout every time nushell initializes and saving their output to disk in .cache directory then sourcing it in the main config.nu file, just generate the config during nix build and source it directly from the nix store.

If you want you can implement something similar for all other shells too. It's just that for nushell this seems to be the only rational way.

also pay-respects nushell integration didn't ever work, now it generates the configuration correctly.
but even though the configuration that pay-respects generates is added to nushell config.nu correctly now it still doesn't work for me. On invocation it gives suggestions for the command that invoked it so not the last one before it that it is supposed to fix.

By the way, I have no idea why but tests fail even though I edited them according to my changes and error messages are either not helpful or I can't read them, I may know how to make nix modules but I never had to deal with home-manager test framework and am not ready to spend weeks trying to figure it out myself so help me with that.

like this for example:

error: builder for '/nix/store/96d9f0h8sp47b5m4yjxssl8rbcx11nwn-carapace-nushell-config.drv' failed with exit code 127;
       last 1 log lines:
       > /build/.attr-0l2nkwhif96f51f4amnlf414lhl4rv9vh8iffyp431v6s28gsr90: line 1: @carapace@/bin/carapace: No such file or directory
       For full logs, run 'nix log /nix/store/96d9f0h8sp47b5m4yjxssl8rbcx11nwn-carapace-nushell-config.drv'.

full log does not say anything new because the log is 1 line long.
I don't know why tests expect @carapace@/bin/carapace while generating nushell config if they are only supposed to for other shells. There are more test failures of the same kind.

Checklist

• Change is backwards compatible.

• Code formatted with ./format.

• Code tested through nix-shell --pure tests -A run.all
  or nix build --reference-lock-file flake.lock ./tests#test-all using Flakes.

• Test cases updated/added. See example.

• Commit messages are formatted like

  {component}: {description}
  
  {long description}
  

  See CONTRIBUTING for more information and recent commit messages for examples.

Maintainer CC

@ALameLlama @Weathercold @bobvanderlinden @water-sucks @terlar

[khaneliman]

By the way, I have no idea why but tests fail even though I edited them according to my changes and error messages are either not helpful or I can't read them, I may know how to make nix modules but I never had to deal with home-manager test framework and am not ready to spend weeks trying to figure it out myself so help me with that.

like this for example:

error: builder for '/nix/store/96d9f0h8sp47b5m4yjxssl8rbcx11nwn-carapace-nushell-config.drv' failed with exit code 127;
       last 1 log lines:
       > /build/.attr-0l2nkwhif96f51f4amnlf414lhl4rv9vh8iffyp431v6s28gsr90: line 1: @carapace@/bin/carapace: No such file or directory
       For full logs, run 'nix log /nix/store/96d9f0h8sp47b5m4yjxssl8rbcx11nwn-carapace-nushell-config.drv'.

full log does not say anything new because the log is 1 line long. I don't know why tests expect @carapace@/bin/carapace while generating nushell config if they are only supposed to for other shells. There are more test failures of the same kind.

To avoid CI failing due to upstream build failures and fetching dependencies on each CI run, most packages are stubbed with a fake path. If a test is testing the generated binary path, use the stubbed name.

[ALameLlama]

There was a PR to fix the nu shell integration in pay-respects but I believe it ended up getting split into separate PR and the nu shell side never got done.

In most tests it doesn't download the binary so trying to do pipe it out ${payRespectsCmd} nushell ${cfgOptions} >> "$out" doesn't return anything.

pretty much what I said here still applies to this PR #6257 (comment)

[jaredmontoya]

I think I more or less understand what is happening now.

But that just means that the test framework of home manager is faulty.

The only rational solution to this is to either make home manager test framework use real binaries or just disable nushell tests that fail because the thing they are trying to test cannot be tested correctly by the current test framework.

There is no reason why home manager implementation should be limited by it's test framework, it's the test framework that should adapt to the needs of the implementation.

If you disagree with me then it just means that nobody can use pkgs.runCommand for no reason

[nyabinary]

I think I more or less understand what is happening now.

But that just means that the test framework of home manager is faulty.

The only rational solution to this is to either make home manager test framework use real binaries or just disable nushell tests that fail because the thing they are trying to test cannot be tested correctly by the current test framework.

There is no reason why home manager implementation should be limited by it's test framework, it's the test framework that should adapt to the needs of the implementation.

If you disagree with me then it just means that nobody can use pkgs.runCommand for no reason

You can probably submit a PR to fix testing first then and mark this as a dependent on that PR.

[khaneliman]

I think I more or less understand what is happening now.

But that just means that the test framework of home manager is faulty.

The only rational solution to this is to either make home manager test framework use real binaries or just disable nushell tests that fail because the thing they are trying to test cannot be tested correctly by the current test framework.

There is no reason why home manager implementation should be limited by it's test framework, it's the test framework that should adapt to the needs of the implementation.

If you disagree with me then it just means that nobody can use pkgs.runCommand for no reason

If you need access to a real binary, just pass realPkgs. Numerous tests do this, if they can't figure out how to stub a package. There are examples in this repo of using realPkgs or even stubbing pkgs.runCommand{Local}

[jaredmontoya]

I think I more or less understand what is happening now.
But that just means that the test framework of home manager is faulty.
The only rational solution to this is to either make home manager test framework use real binaries or just disable nushell tests that fail because the thing they are trying to test cannot be tested correctly by the current test framework.
There is no reason why home manager implementation should be limited by it's test framework, it's the test framework that should adapt to the needs of the implementation.
If you disagree with me then it just means that nobody can use pkgs.runCommand for no reason

If you need access to a real binary, just pass realPkgs. Numerous tests do this, if they can't figure out how to stub a package. There are examples in this repo of using realPkgs or even stubbing pkgs.runCommand{Local}

Thank you for telling me that, I thought that what was said here: #6257 (comment) is the only option.

[feathecutie]

This change seems quite important on its own, and I'm not sure if this PR is the right place to mention this, but this change made me think about the recent addition of autoload directories in nushell.

Since with this PR, we would be generating derivations for each sourced file anyways, I wonder if it's possible to generate the files in a way where they would be automatically be picked up by nushell's autoload and not have to be sourced explicitly, e.g. as described in #6434 (comment).

The approach mentioned in #6434 (comment) seems like it would be quite easy to implement, but I'm not sure what people think about having to add derivations to home.packages.

For further reference in case anyone wants to take a look at this, autoload directories seem to be documented

• in the nushell configuration docs
• in the nushell 0.96.0 update notes
• in the nushell 0.102.0 update notes
• and in the comment i mentioned, which is how i found out about this feature

[jaredmontoya]

Since with this PR, we would be generating derivations for each sourced file anyways, I wonder if it's possible to generate the files in a way where they would be automatically be picked up by nushell's autoload and not have to be sourced explicitly, e.g. as described in #6434 (comment).

The approach mentioned in #6434 (comment) seems like it would be quite easy to implement, but I'm not sure what people think about having to add derivations to home.packages.

If this is indeed as easy as described in that comment then I could change the implementation to use autoloading instead of sourcing configs manually. Also any of the existing nushell tests will be irrelevant and removed. because there is nothing to check for in the config file.

But I am also not sure what people think about that.

[nyabinary]

Since with this PR, we would be generating derivations for each sourced file anyways, I wonder if it's possible to generate the files in a way where they would be automatically be picked up by nushell's autoload and not have to be sourced explicitly, e.g. as described in #6434 (comment).
The approach mentioned in #6434 (comment) seems like it would be quite easy to implement, but I'm not sure what people think about having to add derivations to home.packages.

If this is indeed as easy as described in that comment then I could change the implementation to use autoloading instead of sourcing configs manually. Also any of the existing nushell tests will be irrelevant and removed. because there is nothing to check for in the config file.

But I am also not sure what people think about that.

I mean I would be for it :P

[khaneliman]

If this is indeed as easy as described in that comment then I could change the implementation to use autoloading instead of sourcing configs manually. Also any of the existing nushell tests will be irrelevant and removed. because there is nothing to check for in the config file.

But I am also not sure what people think about that.

I think the test would simply be, does the file exist in the autload directory with integration enabled.

If this is a supported upstream behavior, I think it makes sense to leverage their own loading mechanisms and offload that maintenance burden.

[jaredmontoya]

As an example I tried to do this:

    home.packages = [ cfg.package ] ++ lib.optionals cfg.enableNushellIntegration [
      (pkgs.runCommand "carapace-nushell" { } ''
          mkdir -p $out/share/nushell/vendor/autoload
          ${bin} _carapace nushell > $out/share/nushell/vendor/autoload/carapace.nu
        '')
    ];

but unlike

      nushell = lib.mkIf cfg.enableNushellIntegration {
        extraConfig = ''
          source ${
            pkgs.runCommand "carapace-nushell-config" { } ''
              ${bin} _carapace nushell >> "$out"
            ''
          }
        '';
      };

It seems that nix still tries to build the runCommand package even when enableNushellIntegration is supposed to be false.
When tests for other shells that have no _module.args.pkgs = lib.mkForce realPkgs; like nushell tests(because it is needed for the config package to build) are run:

{
  carapace-bash = ./bash.nix;
  carapace-zsh = ./zsh.nix;
  carapace-fish = ./fish.nix;
  carapace-nushell = ./nushell.nix;
}

And it tries to build the runCommand package it fails to build the package because binaries are stubbed. When it shouldn't be trying to build it in the first place.

I can just force real binaries in tests for other shells too but that seems wrong because it shouldn't build the package if nushell is not enabled.

[khaneliman]

LGTM for the main changes, not a fan of having to introduce realPkgs for all the tests atm since it'll introduce potential for upstream packaging breaking our CI. But, not certain of a good alternative, atm.

[khaneliman]

Hmm, wasn't expecting that... The others worked fine, just this one is blowing up.

> Error: could not load client settings
┃        >
┃        > Caused by:
┃        >    0: could not create dir "/homeless-shelter/.config/atuin"
┃        >    1: failed to create directory `/homeless-shelter/.config/atuin`
┃        >    2: Permission denied (os error 13)
┃        >
┃        > Location:
┃        >     crates/atuin-client/src/settings.rs:816:14