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

No great way to close over flake inputs #104

Closed
clhodapp opened this issue Jan 11, 2023 · 12 comments
Closed

No great way to close over flake inputs #104

clhodapp opened this issue Jan 11, 2023 · 12 comments
Labels
enhancement New feature or request

Comments

@clhodapp
Copy link

clhodapp commented Jan 11, 2023

It would be great to be able to close over the inputs of the current flake in our exported flake modules, either by discovering and documenting a pattern or by adding some kind of scaffolding in flake-parts. This would allow us to e.g. leverage library functions from our inputs within modules or create opinionated layers over-top of other flakes.

At present, the best way to do this seems to be to force anyone who pulls in our module to take a direct dependency on our dependencies with something like inputs.blah or (throw "The flake does not have a 'blah' input. Please add it."). This is pretty fiddly / non-scalable as the ecosystem grows.

@roberth
Copy link
Member

roberth commented Jan 12, 2023

This is a general problem with flakes, considering

Alternatively, we could try to implement it in nix. We have

  • store paths to establish equality
  • lastModified to establish which version is more recent
    • tags would be nicer, because we can complain about major version differences, but also more work
  • no method for establishing the global "name" of a module (not even a flakeref that we could strip to become a unique identifier)
    • add our own? required by the flake-parts.flakeModules.flakeModules module?

Ambiguities can always be resolved by adding more <name>.inputs.<name>.follows, although this is less convenient than using version info to make the decision which version to include. Tradeoff between safe but annoying and convenient but sometimes fragile.

Doing this ourselves is cumbersome at the very least. If we don't find a way to do it through the module system, we'd need to scan all inputs recursively, which is awful for performance. It'd be best to hook into the import resolution functionality somehow. Maybe "just run" and only eval one option which contains all the metadata? Then run again if no errors, or perhaps filtering out some outdated modules. Might be fragile though. Alternatively, run just the import resolution part of the module system, or build this functionality right into the module system.

@clhodapp
Copy link
Author

For what it's worth, I worked around this by adding an initial non-modular parameter list to my modules, which expect to receive the inputs from my flake. I have to import the modules and provide the non-modular argument before exporting the modules on flakeModules or handing to flake-parts.

This way, if someone pulls in my flake, they get a version of the module that has been supplied with my inputs, as opposed to the inputs of the flake that's trying to use my module.

Of course, that makes .follows the mechanism for syncing everything up in the downstream flake, if that's desired.

@clhodapp
Copy link
Author

To illustrate more clearly, I have modules that go something like this (just showing the simplest case of re-exporting the inputs to the module composition):

# Take in my-inputs as a non-modular arg so we can close over them
{my-inputs}:
{lib, ...}:
{

  options.my-inputs = lib.mkOption {
    type = lib.types.lazyAttrsOf lib.types.raw;
    description = "The inputs for my flake";
  };

  config = { inherit my-inputs; };
}

@roberth
Copy link
Member

roberth commented Jan 29, 2023

I've previously assumed that module deduplication works reliably, but perhaps not. I'm improving this in Nixpkgs. See #111

(not a complete solution to the problem stated, but a small part of the puzzle)

@Atry
Copy link
Contributor

Atry commented May 4, 2023

I think we can provide a helper function to find the dependencies of the current input like cachix/devenv#576

@roberth
Copy link
Member

roberth commented May 4, 2023

helper function to find the dependencies

That would kill the laziness of input fetching.

@roberth
Copy link
Member

roberth commented May 4, 2023

I might have read a bit of my own concerns into this.
If this is mostly about inputs (seems to be), I think this can be resolved in large part by using the lexical scope. For example @clhodapp does this in his example by putting his module in a function that brings the local inputs into scope (in #104 (comment)). The same works for the self parameter, and then I mean the Nix flake self parameter, not the self module argument, which is the user's flake.

Another way to achieve this is to put the module not in a separate file, but directly into flake.nix, where the flake's self and inputs are in scope. Often only a small module needs to be in flake.nix and the rest could still be imports-ed from a file.

  options.my-inputs = lib.mkOption {

While it's not necessarily a problem to bring all your inputs into your users' scope, it's generally cleaner to have options only for the things that are need from those inputs. If you do decide to go with this, make sure to pick a sufficiently unique name. You may want to set mkOption { internal = true; }.

@Atry
Copy link
Contributor

Atry commented May 5, 2023

That would kill the laziness of input fetching.

Who cares the laziness of input fetching?

@clhodapp
Copy link
Author

clhodapp commented May 8, 2023

For what it's worth: Yeah, I was primarily concerned with inputs when I wrote this issue. I do see how diamond dependencies are kinda dicey at present both with flake inputs and with modules, though.

@clhodapp
Copy link
Author

clhodapp commented May 9, 2023

I'm going to take a little more time to make sure that I've wrapped my head around @Atry's concerns first, but I'm likely to close this in the next few days now that importApply is merged. There's a chance that I'll open a successor issue for deduplication if there isn't already one by that point.

@roberth
Copy link
Member

roberth commented May 9, 2023

I've made an attempt at documenting the requirements, status quo, attempts and two ways forward for flakeModules specifically #157.
I think we're most of the way there, especially when recursively dogfooding the module is not a requirement, as is with flakeModules, but not with any of the other classes of modules.

The other concerns (that aren't strictly about in-flake scope mechanics and such) seems sufficiently covered by the following issues, so feel free to close this issue.

@clhodapp
Copy link
Author

clhodapp commented May 9, 2023

Cool, #157 definitely seems to be the proper successor to cover the ground not covered by the current importApply.

@clhodapp clhodapp closed this as completed May 9, 2023
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

3 participants