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 dogfood template (fix #104 and #121) #156

Closed
wants to merge 3 commits into from

Conversation

Atry
Copy link
Contributor

@Atry Atry commented May 9, 2023

Motivation

Testing

nix flake show
git+file:///Users/twer/github/flake-parts?dir=template/dogfood
├───apps
│   ├───aarch64-darwin
│   │   └───default: app
│   └───x86_64-linux
│       └───default: app
├───flakeModule: unknown
├───flakeModules: unknown
└───packages
    ├───aarch64-darwin
    │   └───hello_22_11: package 'hello-2.12.1'
    └───x86_64-linux
        └───hello_22_11 omitted (use '--all-systems' to show)
nix repl
Welcome to Nix 2.15.0. Type :? for help.

nix-repl> :lf .
warning: Git tree '/Users/twer/github/flake-parts' is dirty
Added 20 variables.

nix-repl> flakeModules
{ anotherFlakeModule = { ... }; dogfood = { ... }; hello = { ... }; }

nix-repl> 

@Atry Atry force-pushed the example-dogfooding branch from 85657b3 to aafcaf1 Compare May 9, 2023 02:02
Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the design issues, this would need testing and better inline documentation to explain succinctly the role of each file, etc. (Although don't invest in that before making sure everything works with tests, and maybe don't even test before resolving the double mkFlake design issue)
Besides the double mkFlake broken fixpoint, I have doubts about the module keys. A test should point out that that works, e.g. a test doing a double import and a test for disabledModules.

Comment on lines +13 to +18
# Workaround for https://github.com/hercules-ci/flake-parts/issues/148
self = {
outPath = ./.;
inherit (self)
_type inputs lastModified lastModifiedDate narHash outputs sourceInfo submodules;
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks normal use of self. I don't think importing from self is important enough of a problem.
Making assumptions about the user's file layout is too magical. You are free to implement your own conventions using the specialArgs solution instead.

};
}
./modules/dogfood.nix
).flakeModules.dogfood;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By conflating the published module and the current flake, you're causing users of the published module to export the published module. This is not right.


outputs = { flake-parts, self, ... }@inputs:
flake-parts.lib.mkFlake { inherit inputs; } (
flake-parts.lib.mkFlake
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned before, this looks a lot like a broken implementation of fixpoint iteration, which is not feasible. In other words, references to the actual flake will not work as expected. We need to tie the lazy fixpoint knot, instead of providing a single fixpoint iteration step (out of a potentially infinite number of steps required).
I don't think this can be done without changing mkFlake.

@roberth
Copy link
Member

roberth commented May 9, 2023

Perhaps a way to work around the problem posed by graphs of public flakeModules is to set the module key by hand (EDIT: inside the module file, or by wrapping the module where you perform your local imports) for now.
I don't think we should bless that solution with a template until we've tried a mkFlake-based solution, but it seems that it would unblock your actual module implementation work for the time being.

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

Successfully merging this pull request may close these issues.

2 participants