hercules-ci / flake-parts

❄️ Simplify Nix Flakes with the module system
https://flake.parts
MIT License
721 stars 41 forks source link

Add findInputsByOutPath (fix #104) #146

Closed Atry closed 1 year ago

Atry commented 1 year ago

For

roberth commented 1 year ago

What do you use this for; what's the end goal?

Atry commented 1 year ago

End goal in #104

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 commented 1 year ago

How would we use this? Why is it that we know outPath, but not the flake attrset it comes from?

Atry commented 1 year ago

Suppose you are creating a library of flake parts that depends on nix2container, you can do this:

(flake-parts-lib.findInputsByOutPath ./. inputs).nix2container

Why is it that we know outPath, but not the flake attrset it comes from?

Because a flake part is a module nix file, which does not have the lexical scope of its own inputs.

Atry commented 1 year ago

This approach is not ideal because it does not support inputs'

Atry commented 1 year ago

Added inputsByOutPath and inputsByOutPath'

roberth commented 1 year ago

Would https://github.com/hercules-ci/flake-parts/pull/149 help? Maybe #121 has useful information?

Atry commented 1 year ago

I shared my concern in #149. I believe this PR is still a more solid solution than #149.

clhodapp commented 1 year ago

Hmmm, so I guess if I understand:

149 uses lexical closure to capture stuff from the upstream flake's environment while it's evaluating (including closing over inputs).

In contrast, this PR tries to give the module from the upstream flake the tools to find "its own" inputs within the structure of the inputs to the downstream flake.

At first consideration, this PR seems way more complex and fragile than #149 to me. E.g. what happens when the exact same upstream flake source exists in the dependency tree more than once but the transitive dependencies are different due to different .follows? Also, doesn't the technique used in this version require us to do the fiddly job of constructing a (correct) path to the root of the current flake ourselves with the upstream flake, and if we mess it up we'll (silently) get resolution against the inputs of the downstream flake (which will totally succeed but do the wrong thing if they have a same-named input)?

What are the benefits of doing things this way versus the technique in #149?

Atry commented 1 year ago

No matter if you think #150 is a good idea, flake-parts should allow users to create a reusable module in a reusable flake like #150

  1. The reusable module can depend on another input;
    • e.g. github:per-system/default
  2. The reusable module's input dependency should have a default value, and should not fail when the user did not provide an explicit dependency.
    • Avoid throw "The flake does not have a 'blah' input. Please add it."
  3. The reusable module can be used by another module in the same repository;
    • e.g. imports = [ ./systems.nix ];
  4. The reusable module can be used by another module in another flake repository;
    • e.g. imports = [ inputs.blah.flakeModules.systems ];
    • or imports = [ (inputsByOutPath ./.).blah.flakeModules.systems ];
  5. If the reusable module is imported more than once, it should get deduplicated automatically (see https://github.com/NixOS/nixpkgs/issues/215496);
    • e.g. no error when imports = [ ./systems.nix ./systems.nix ];
    • and no error when imports = [ (inputsByOutPath ./.).blah.flakeModules.systems (inputsByOutPath ./.).blah.flakeModules.systems ]; in another flake

When using the approach in Dogfood a Reusable Module, modules in the same flake cannot import each other, which is not good in a modularized repository.

Atry commented 1 year ago

Basically Dogfood a Reusable Module only works if you have a single module in a flake. It does not work when you have multiple modules that depend on each other in a flake.

clhodapp commented 1 year ago

Hmm. Not sure but I'd think that 3 actually works with importApply if you pass the parent module into the child module through non-modular args. Most likely, you would put a let in your flake.nix where you inflate all the modules, including passing the "canonical" reference to the parent down to the. The only challenge would be that you would have to explicitly express the dependency structure within flake.nix.

I think that this could be simplified (and 5 could potentially be solved) if we could do deduplication on the combination of the module filename AND the static args object (just a direct equality comparison). This might have an impact on laziness, though: I think that nix has to evaluate a set recursively to compare it with another set.

(edited to be less confident until I actually do the experiment)

Atry commented 1 year ago

if you pass the parent module into the child module through non-modular args.

In the same flake, the modules would not deduplicate, because you are using the unwrapped version of modules without https://github.com/hercules-ci/flake-parts/blob/main/extras/flakeModules.nix

roberth commented 1 year ago
  1. The reusable module can be used by another module in the same repository;

    • e.g. imports = [ ./systems.nix ];

This already works, as long as systems.nix is a proper module of course, and not a function to a module. Deduplication and disabledModules also work.

If it's not a module, I suppose it's a function to a module and you could use importApply. If you find multiple importApply calls gets cumbersome, you could move the imports graph into flake.nix and turn all module files into leaves of said graph, or maybe you prefer the factor out pattern.

If you have a legacy, non-flake entrypoint, you can use builtins.getFlake or flake-compat as usual.

Only the addition of a module key seems to be missing from this. That key should be present between local imports of importApply modules.

When using the approach in Dogfood a Reusable Module, modules in the same flake cannot import each other, which is not good in a modularized repository.

Did I draw the right conclusion? 3 and 5 both seem to be about the module key. Not sure if I would frame them as contradictory if it's just about having better keys.

I suppose one way to get those keys in there is to have some logic for this in mkFlake. As mkFlake will only be invoked once, we can reasonably let it make keys for each of the module files; an advantage of being less free-form than importApply.

The interface may look something like

flake.nix:

mkFlake { flakeModules = { foo = ./foo.nix; bar = ./bar.nix; }; inherit inputs; } {
  perSystem = something;
  # nothing about flakeModules here; all taken care of
}

foo.nix:

{ localFlakeModules,
  localFlake,
  withSystem # can probably add all the top level module arguments here lazily with some reflection + fixpoint magic; otherwise just expose config as localConfig
}:
{ self, # user flake
  ... }:
{
  imports = [ localFlakeModules.bar ];
}

All the modules you reference in flakeModules will receive the same arguments, which includes localFlakeModules so that they can do their own imports. Proper stand-alone modules could be imported as usual and won't need the mkFlake { flakeModules } treatment.

Atry commented 1 year ago

I think both localFlakeModules and localFlake can be achieved by turning flakeModulesOption into a submodule as described in #121. We don't have to change mkFlake's signature.

roberth commented 1 year ago

I think both localFlakeModules and localFlake can be achieved by turning flakeModulesOption into a submodule as described in #121. We don't have to change mkFlake's signature.

I don't think that would work for the dogfooding use case. Like config, options only evaluates after the imports have been resolved.

Atry commented 1 year ago

I don't think that would work for the dogfooding use case. Like config, options only evaluates after the imports have been resolved.

roberth commented 1 year ago

I can't accept this into the core because it is has a serious flaw as described in Do not traverse inputs.

I'm really sorry to close many of your PRs. I really appreciate your efforts, but please know that flake-parts' apparent simplicity is deceptive, as changes here have very significant ramifications.