search

hercules-ci / flake-parts

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

Add moduleLocation to mkFlake argument #158

Closed Atry closed 10 months ago

Atry commented 1 year ago

This PR adds moduleLocation argument to mkFlake, allowing the user to set the module key by hand, addressing https://github.com/hercules-ci/flake-parts/pull/156#issuecomment-1540165822

Atry commented 1 year ago

@roberth Would you mind if I reopen #156 and rebase onto this PR, so that we don't have to break inputs.self in order to call mkFlake twice.

roberth commented 1 year ago

I meant to say "in the module files" or something; I've edited https://github.com/hercules-ci/flake-parts/pull/156#issuecomment-1540165822 to reflect this. It was supposed to be a solution for the problem of having no keys when doing local imports.

I don't think the manipulation of self is worth solving, because double mkFlake is not quite sufficient for the purpose of dogfooding.

roberth commented 1 year ago

157 is worth solving, but unless we know that we have a complete solution, we're probably just making a mess out of mkFlake.

We already have a "95%" solution of the problem and I am confident that a full solution of #157 is within reach. For this reason, I'm not going to accept a solution that solves a different 95%, and for the sake of maintainability I won't accept a 96% solution either, because that solution will not come with a proof that it leads towards the 100%. If it does come with such a proof, then it's already a 100% solution.

I've already stretched my schedule a lot by responding to your suggestions. I won't be responding quickly for some time, but that won't be too bad, because you can test changes yourself (see e.g. the dev directory) and you can validate the requirements or better understand them by applying your solution in multiple repositories. I'm sorry that I won't be quite as helpful for now, but I have to prioritize.

Atry commented 1 year ago

This PR does not aim to directly solve #157. Instead, it is a workaround for #148. I understand the best solution of #148 is https://github.com/NixOS/nix/pull/4154. But we probably would not get https://github.com/NixOS/nix/pull/4154 very soon because it is inactive.

I think it would be a net benefit to limit the use of self.outPath until https://github.com/NixOS/nix/pull/4154 gets landed, not only because self.outPath prevents double mkFlake, but also because it would produce obscure error messages about infinite loop due to #148 in other use cases, including flake-parts's own tests.

roberth commented 1 year ago

I'd like to keep it simple, so I'm going for the root cause.

roberth commented 11 months ago

root cause.

  • Blocked on

Rabbit hole too deep as that issue is itself blocked on another issue. I think I've found an extended solution that's easier to use.

I'm not sure if it (automatically) solves all the problems you intended to solve, but it should solve

Atry commented 11 months ago

Looks good to me