feat(wrapperModules.zsh): init

[FluxZA]

Create a wrapper module for zsh

[FluxZA]

Hi. The tests are failing because my checks currently don't pass, I have a question regarding that below. I also have not finished documenting options.

Questions regarding the implementation:

• Do you think it is necessary to add an option to skip reading the /etc/zshenv file? I think it would be possible by overriding the configuration flags used to build the base package.
• I have not set options for common zsh options or features like aliases etc. This is because they would only be able to be added if the zdotFiles option is used, not the zdotdir option. Do you think this is fine or should I add them anyway with a caveat.

Questions regarding the checks:

• I wanted to test that the config in the store was being used, but from what I can see the test is running before the build is fully finished? Trying to ls the store directory fails claiming that it does not exist, however, when I check it manually after the tests stop, the directory exists and contains the file it should.

[BirdeeHub]

Do you think it is necessary to add an option to skip reading the /etc/zshenv file? I think it would be possible by overriding the configuration flags used to build the base package.

Not really no but for completeness maybe it could be offered one day, but overriding the base package means you rebuild it, and that is expensive.

I have not set options for common zsh options or features like aliases etc. This is because they would only be able to be added if the zdotFiles option is used, not the zdotdir option. Do you think this is fine or should I add them anyway with a caveat.

Yeah this is a conundrum slightly.

How about this.

What if you secretly make your own zdotdir every time. If they supply zdotdir then you can source the files from it if they exist as it would from each of your files in your zdotdir, but that would allow you to ALSO accept things from the zdotFiles options.

Also you should link the zdotFiles rather than copying them, or just refer to them when you source them from your secret zdotdir (probably easier to just check for existence then source from the script), that way you can include impure paths.

Also

      type = types.nullOr (types.either types.path types.str);

    type = types.nullOr wlib.types.stringable;

I wanted to test that the config in the store was being used, but from what I can see the test is running before the build is fully finished? Trying to ls the store directory fails claiming that it does not exist, however, when I check it manually after the tests stop, the directory exists and contains the file it should.

It was missing this condition, it might just be things like this.

      env.ZDOTDIR = lib.mkIf (config.zdotdir != null || config.zdotFiles != null) zdotdirEnv;

Because if you build with no arguments right now it refers to a place in placeholder out that doesnt get created. So its probably something of that nature. Your drv.installPhase is most definitely running before your test is, your test needs that whole zshWrapped derivation to resolve before it can get the path to it.

So, whatever is going wrong with the tests, it is not related to sequencing of stuff in the test being ran before stuff in the zshWrapped derivation. It is likely confusing because the test itself is a "build", but it is a runCommand which uses the result of zshWrapped. zshWrapped is computed already.

[BirdeeHub]

I just did

    zdotdir = builtins.path { path = ./test-zdotdir; };

and it worked but IDK why that makes any sense.

Also

    zdotdir = "${./test-zdotdir}";

That also works too for some reason.

I think this is lazy trees.......?

      zdotdirEnv = lib.mkIf (config.zdotdir != null || config.zdotFiles != null) (
        lib.trivial.warnIf (config.zdotdir != null && config.zdotFiles != null)
          "Using both zdotdir and zdotFiles options is not compatible. Only zdotdir will be used."
          (
            if config.zdotdir != null then
              if builtins.isPath config.zdotdir then builtins.path { path = config.zdotdir; } else config.zdotdir
            else
              "${placeholder "out"}/${zdotFilesDirname}"
          )
      );

Weird but the above passes the ls test

However, you should make your own zdotdir and source the stuff we provide, which hopefully will make this issue go away on its own? We will see.

The path to the test-zdotdir is in fact making it into the derivation. And I suppose your store would have that path. But its context somehow isnt making it.

But all I do is pass them to lib.escapeShellArg and then write them into the buildCommand of the drv. The path you are passing does in fact make it into the drv build command. It really should be provisioning this I feel like but maybe I am not understanding how it works?

---

Im going to swap back to upstream and see if this still happens

---

Edit: Ok, so it also happens on upstream. So it is not lazy-trees

But something is making it not provision that path, despite the fact that that path is very much making it into the derivation and is a nix path, and is only being passed through lib.escapeShellArg (this happens on all the backends it is not specific to the binary one)

But regardless, hopefully this goes away on its own once you change the design to always provide an internal zdotdir and source the users stuff yourself from there. If not, then call builtins.path on it if it is a path. I am not sure why this is needed. It seems like when you do stuff like lib.generators.toLua or builtins.toJSON it does this for you, but if you are just using toString it does not necessarily?

I wonder if this is something that I can do in wlib.types.stringable for convenience for the user?

---

Edit:

Should I do this?

  stringable = lib.mkOptionType {
    name = "stringable";
    descriptionClass = "noun";
    description = "str|path|drv";
    check = lib.isStringLike;
    merge =
      loc: defs:
      let
        res = lib.mergeEqualOption loc defs;
      in
      if builtins.isPath res then builtins.path { path = res; } else res;
  };

[FluxZA]

Thank you for the quick feedback!

I like the idea of using a secret zdotdir that sources the given files. If I use the approach of sourcing the zdotFiles, when they aren't impure paths should I check for existence in the store? My understanding of files in vs out of the store is still not the best.

[FluxZA]

I think it would be helpful to include that as part of stringable for the users, as some programs might need to test against directories.

[BirdeeHub]

A string may be an impure path, like "/home/flux/mynixos" or whatever. But if they are path types, they are pure paths, they just might not be provisioned apparently. I think this is because /home/flux/mynixos is a path, but not a pure path. But if something is providing something like that, they are already using --impure, so it still should be safe to call builtins.path on it?

builtins.path can be called multiple times. And you dont have to use wlib.types.stringable but where people would use it they probably would expect that thing to get provisioned. So I think I will do that.

[FluxZA]

A string may be an impure path, like "/home/flux/mynixos" or whatever. But if they are path types, they are pure paths, they just might not be provisioned apparently. I think this is because /home/flux/mynixos is a path, but not a pure path. But if something is providing something like that, they are already using --impure, so it still should be safe to call builtins.path on it?

is that because an absolute path is handled differently to a relative path? I thought that specifying a relative path is a pure path?

Also, what is the difference between provisioning vs a path existing in the store?

[BirdeeHub]

And, you shouldnt check that they are in the store no. They explicitly are wlib.types.stringable which means, anything that I can interpolate into a string, which includes an impure paths. If you want to test it with a path not already in the store, you should check that your secret one is there, and for the other options then you should test it with some paths that are "impure" inside the derivation if you want to, i.e. use a relative path within the test or something.

And yeah, ngl this one was a surprise to me as well, I did not know store paths worked like that. Oddly, you can still import from them even if they are like that, but they arent really "realized" yet, which is weird. This has to be some sort of optimization that allows it to not scan stuff in nixpkgs.

[BirdeeHub]

A string may be an impure path, like "/home/flux/mynixos" or whatever. But if they are path types, they are pure paths, they just might not be provisioned apparently. I think this is because /home/flux/mynixos is a path, but not a pure path. But if something is providing something like that, they are already using --impure, so it still should be safe to call builtins.path on it?

is that because an absolute path is handled differently to a relative path? I thought that specifying a relative path is a pure path?

Also, what is the difference between provisioning vs a path existing in the store?

"/home/flux" is a string, it is a string that looks like a path but wont be provided by nix

"./somedir" and this is the same. It will look in wherever that is at runtime. Nix has no idea where that is.

Colloquially, the above 2 are known as impure paths. This is because nobody acknowledges the next form of impure path:

/home/flux is an impure path. If you dont use --impure and you are using a flake this will throw an error if you try to use it.

./somedir this is a store path. you can call import on it or check for its existence without an IFD. But apparently it isnt always realized. Who knew? I didnt. I mean, I knew you could turn it into a string and then call unsafeDiscardStringContext on it to make sure that it does not get provisioned. But I kinda thought that otherwise it always would be? Generally one operates with these like they are always there, and are shocked when in some odd scenario like now they are not, and then call builtins.path on them to fix it.

[FluxZA]

Okay, thanks for clarifying.

[BirdeeHub]

To the best of my ability anyway 😆

[BirdeeHub]

#286

Gonna add this.

This also takes care of it for wlib.types.file as well. its path value is of the stringable type

Im not really sure if this needs to be done at the makeWrapper level or not yet. Maybe just a different default escapeShellArg that also calls this if it is a path first could be used for esc-fn? Regardless it should still be done in that type as well, and for our purposes here is enough.

[BirdeeHub]

Suggested change

	type = types.nullOr (types.either types.path types.str);
	type = types.nullOr wlib.types.stringable;

[BirdeeHub]

make sure to only make this if something was provided (although, now that you will always be providing one, that might not be relevant anymore) but regardless it should behave as much like normal if nothing was provided as possible

[BirdeeHub]

Left the comments in the specific places they were mentioning. They may or may not be relevant by the time you finish implementing the suggestion where you source stuff yourself.

Anyway, hope I was helpful. Good luck!

[FluxZA]

Thank you.

[FluxZA]

One question I forgot to ask. Is there a way to run tests for a specific module only? I tried to look at the ci flake, but I didn't see any way to pass in arguments. Which now that I think of it makes sense as it is a flake.

[BirdeeHub]

One question I forgot to ask. Is there a way to run tests for a specific module only? I tried to look at the ci flake, but I didn't see any way to pass in arguments. Which now that I think of it makes sense as it is a flake.

nix build ./ci#checks.<system>.wrapperModule-zsh

Also with zsh that probably means ur gonna have to escape the #

nix flake check -Lv ./ci runs all the checks outputs of the flake with the checks in it. To run them individually you just build them.

I have the checks in a second flake so that I can do stuff like pull flake-parts to test that at some point, pull random stuff for docs but still run the docgen as part of the tests, mock things that are maybe complicated and need dependencies, etc. while leaving the top-level flake with just 1 input

[FluxZA]

Perfect thank you, that makes sense.

[FluxZA]

What do you think of this approach to building the files:

      baseZshenv = /* bash */ ''
        # zsh-wrapped zshenv: DO NOT EDIT -- this file has been generated automatically.
        # This file is read for all shells.

        # Ensure this is only run once per shell
        if [[ -v __WRAPPED_ZSHENV_SOURCED ]]; then return; fi
        __WRAPPED_ZSHENV_SOURCED=1

        # Cover some of the work done by zsh NixOS program if it is not installed
        if [[ ! (-v __ETC_ZSHENV_SOURCED) ]]
        then
          WRAPPER_MOCK_ETC_ZSHENV=1
          HELPDIR="${pkgs.zsh}/share/zsh/$ZSH_VERSION/help"

          # Tell zsh how to find installed completions.
          for p in ''${(z)NIX_PROFILES}; do
              fpath=($p/share/zsh/site-functions $p/share/zsh/$ZSH_VERSION/functions $p/share/zsh/vendor-completions $fpath)
          done
        fi

        # Get zshenv from wrapped options if they exist
        ${lib.optionalString (config.zdotFiles != null) /* bash */ ''
          if [[ -f "${config.zdotFiles.zshenv.path}" ]]
          then
            source "${config.zdotFiles.zshenv.path}"
          fi
        ''}
        ${lib.optionalString (config.zdotdir != null) /* bash */ ''
          if [[ -f "${config.zdotdir}/.zshenv" ]]
          then
            source "${config.zdotdir}/.zshenv"
          fi
        ''}
      '';
    in
    {
      drv.installPhase = ''
        mkdir $out/wrapped_zdot
        cat >$out/${zdotFilesDirname}/.zshenv <<'EOL'
        ${baseZshenv}
        EOL
      '';

[BirdeeHub]

yeah

You would have to do the thing you did for .zshenv for all of them probably, but basically that yeah.

Looks like you got some (most? all? IDK) of the stuff from nix that one might want to source too with the completions thing.

You should use buildPhase instead of installPhase and you should put the runHook for pre and post build like

drv.buildPhase = ''
  runHook preBuild;
  thestuff;
  ...;
  runHook postBuild
'';

But yes that looks like a fine approach.

Also, you should set config.passthru.shellPath = "/bin/zsh"; as discussed by this discussion about bash