Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "cursor" relative to import root to the loader API #13

Open
Lukas-C opened this issue Jun 11, 2023 · 8 comments
Open

Add "cursor" relative to import root to the loader API #13

Lukas-C opened this issue Jun 11, 2023 · 8 comments
Labels
enhancement New feature or request

Comments

@Lukas-C
Copy link

Lukas-C commented Jun 11, 2023

Hi Everyone,

first of all, thank you for this library. It provides me with a simple solution for what has annoyed me the most since I've started writing my NixOS config about 4 weeks ago. During that time, I've been trying to find a more ergonomic way to manage NixOS module files and I think I now managed to find a decent way using Haumea. If you are at all interested, the dotfiles are available here, with a small preliminary write-up of how it works, but it's by no means required to understand what follows.

With that as context, now to my actual issue/proposal:
For the project I wrote a custom loader, which modifies the self, super and root attributes such that their functionality is preserved in a way that is actually useful in the context of NixOS modules. For this loader I'm actually only interested in a "cursor" relative to the root, so e.g. [ "base" "feature" ] for a file at <load path>/base/feature.nix. I then use this cursor together with nixpkgs.lib.getAttrFromPath to navigate the attribute tree.

However, the API currently passes only the absolute loading path to a loader, so e.g. /nix/store/<hash>-source/<load path>/base/feature.nix. This leads to the awkward situation that one needs to reverse engineer the cursor from this absolute path. In general, this is tricky, especially considering that the information is already available within the code of this library, namely in tree.pov in load.nix.

The original code of src/load.nix in lines 78-85 looks currently as follows:

{
   content = fix (self:
     (head matches).loader
       # inputs arg
       (inputs // {
         inherit self;
         super = getAttrFromPath tree.pov (view tree);
         root = view tree;
       }) 
       # path arg
       (src + "/${path}"));
}

My current patch to Haumea just adds a third argument to the API in load.nix:

{
   content = fix (self:
     (head matches).loader
       # inputs arg
       (inputs // {
         inherit self;
         super = getAttrFromPath tree.pov (view tree);
         root = view tree;
       })
       # path arg
       (src + "/${path}")
       # cursor arg
       (tree.pov ++ [name]));
}

There are also probably other ways of approaching this, but this one is the most straight forward in my opinion. Since I have the patch already, I could easily extend this to a pull request, but I would probably need some help with testing, as I currently don't use any other loaders than my own and the default one.

I realize this might not be worth the break in the API or even a use case you want to support. But for me it would go a long way to increase the ease of use in these edge cases where you only want to do some relative changes within the structure of the loaded files.

Thank you again for your work, best regards.

@figsoda
Copy link
Member

figsoda commented Jun 11, 2023

Thank you for the kind words!

I'm afraid that this change would complicate the loader API, and I'm not sure what is the best way to approach this. I do agree that the cursor could be helpful for loaders, but this information is somewhat duplicate for loaders, since it takes the path as an argument, as opposed to transformers which doesn't have this information without the cursor argument.

Here are some possible alternatives I was able to think of:

  • I could add a helpfer function that parses a path into a cursor, so you can do things like:

    haumea.lib.load {
      src = ./src;
      loader = inputs: path:
        let
          # placeholder name
          cursor = haumea.lib.parsePathIntoCursor ./src path;
        in
        <...>;
    }

    This would be non-breaking but not the easiest to use, as you would need to pass in ./src and wouldn't be able to have the loader in isolation.

  • I could also make haumea look at the loader's functionArgs and change its API based on that, so one would be able to do things like loader = inputs: { path, cursor }: <...> while the previous logic would also work.

    The benefit is that this would not be as much of a breaking change and it would be easier to use than the last alternative, but I am worried that this is too magic-y and would be hard to document or confusing to people that are not experienced with the library.

@figsoda figsoda added the enhancement New feature or request label Jun 11, 2023
@Lukas-C
Copy link
Author

Lukas-C commented Jun 11, 2023

I wasn't sure either, and for me the first way would probably be sufficient, if you are really concerned about API breaks. If you can reuse the function in the rest of the code base that would probably be fine, but there is some risk from a code consistency/duplication perspective, as you now would need to look over two pieces of code that deconstruct the path instead of just one.

Regarding the duplicate information, an option could be to pass src and path as separate arguments. That would already simplify stuff a great deal, as you no longer need to worry about the "fixed" part of the path and your proposed parsePathIntoCursor would no longer need to depend on src. It would still be a break in the API, but the concept would probably be pretty understandable, because its just a path split into an absolute and relative part.

Another option that just occurred to me is to add some kind of optional "flag" to haumea.lib.load that specifies the type/content of path. This way one could support an absolute path, a relative path and a cursor option at the same time, without breaking the current API at all. This would incur some complexity of course and it still needs to be documented, but I feel like this would be doable. I don't know how this would interact with the "matchers", but they could maybe just ignore the flag.

Just my two cents, maybe something to think about.

Edit: One could also use the flag to make the differentiation between loaders and matchers more explicit/less "magic-y", while adding support for arbitrarily many other options in the future. But this might make it very tempting to add a lot of complexity over time.

@figsoda
Copy link
Member

figsoda commented Jun 14, 2023

Another option that just occurred to me is to add some kind of optional "flag" to haumea.lib.load that specifies the type/content of path.

One issue is that the flag would make things in haumea.lib.loaders not work, since there is no way for the flag to propagate to the rest of the API, so I would prefer the more magic-y approach in this case, and it should have the same amount of flexibility.

@blaggacao
Copy link
Contributor

blaggacao commented Jun 14, 2023

With little time at hand I just want to quickly throw in additional use cases:

Ordinal Level Information

  1. Usable for cut-off strategies, for example in paisano-nix/core, level 1, 2 & 3 have distinct semantic meanings and implementations based on this ordinal.

  2. Usable to map level depths to module system lattice value priority ("deeper values automatically override trunk values") - to/{,path/}myconfig.toml-like interfaces for XYZ.

Ordinal information is handly encoded in the cursor.

@Lukas-C
Copy link
Author

Lukas-C commented Jun 14, 2023

One issue is that the flag would make things in haumea.lib.loaders not work

Do you mean this in the sense that this would make loaders bound to a specific flag or, in the worst case, set of flags?
I agree, when you consider this, stuff could get out of hand very quickly.

Could the cursor and its associated API maybe live outside the loader, like another transformer, but for the inputs of a file? But that seems (unnecessarily) complicated and in the end maybe not flexible enough. I mean, perhaps it could be a solution, but it certainly requires more in-depth analysis of how the data flow could look like.

Another approach: Similar to the case of passing src and path as separate arguments, one could also put src (so the /nix/store/<hash>-source/<load path> part) as the first element of the cursor. This way, one could to ignore this element if one is only interested in the relative information. And, unless you are doing very fancy stuff, you would most likely never have to do transformations on the individual parts of the src part, anyway. Keeping everything together in this way would save one of introducing another argument, but it would still require changes in existing custom loaders, so not perfect either.

Really just throwing some of the alternate Ideas I can think of out here.

@ThinkChaos
Copy link

I personally don't have a need for this, but here's an idea I didn't see mentioned: stuff the info in the first args:

loader
  (inputs // {
    inherit self;
    inherit (tree) cursor;
    super = getAttrFromPath tree.pov (view tree);
    root = view tree;
  })
  (src + "/${path}"));

The haumea docs specify the loader as ({ self, super, root, ... } -> Path -> a) so IMO it's fine to add something in there.

@Lukas-C
Copy link
Author

Lukas-C commented Dec 3, 2023

I like the simplicity of this, I will test this out with my use case once I have some more time on my hands.

@Lukas-C
Copy link
Author

Lukas-C commented Dec 17, 2023

Small Update: I have tried this out and it has basically been a drop-in replacement for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants