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 an example of dogfooding (fix #121) #197

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

Atry
Copy link
Contributor

@Atry Atry commented Nov 3, 2023

This PR is a rework of #156. It avoids infinite loops by using partitions.

@Atry Atry changed the title Add an example of dogfooding Add an example of dogfooding (fix #121) Nov 3, 2023
Wily96

This comment was marked as off-topic.

Comment on lines +21 to +22
partitionedAttrs.devShells = "dogfood";
partitionedAttrs.packages = "dogfood";
Copy link
Member

Choose a reason for hiding this comment

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

Having to list these is not great UX.

I think I'd prefer to add a parameter to mkFlake.
I think we could have a modules specialArg to make that nice:

mkFlake {
  inherit inputs;
  publicModules.foo = ./foo.nix;
}
({ modules, ... }: {
  imports = [
    modules.self.foo  # based on the publicModules parameter
    modules.git-hooks-nix.default  # just a mapAttrs of inputs; lazy and cheap
  ];
})

It's a bit more custom, but it avoids the complexity of having partitions, probably improving error messages as well, and slightly less boilerplate.

It does put the module in a different scope, but I think this can be worked around, with techniques similar to withSystem, and it'd look like:

  publicModules = local@{ config, withSystem, ... }: { # not defining a module; just a function
  
    /** A module that adds a check using a package built in the local way,
        with no interference except `follows` */
    foo = userFlake@{ self, ... }: {
      perSystem = { system, pkgs, ... }:
        withSystem system (localPerSystem@{ ... }: {
          checks.foo = pkgs.runCommand "check-foo" {
            nativeBuildInputs = [ localPerSystem.config.packages.foo ];
          } "foo ${userFlake.self} && touch $out";
        });
    };
  };

However, actually needing access to the local flake seems quite rare for flake-parts modules, so we might not even need it for now.

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think a flake module that defines a flake module is a good abstract, because it distinguishes the module author's inputs from the module user's inputs. I like the idea to make publicModules a partition of preparatory step by default before the "main" partition. This design has precedent in other languages, e.g. sbt.

Copy link
Contributor Author

@Atry Atry Sep 23, 2024

Choose a reason for hiding this comment

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

The reason why we cannot put bootstrap modules to flakeModules is that flakeModules is a submodule of the mkFlake's top level module, and the mkFlake's top level module cannot depend on its own submodule.

Can we make mkFlake a submodule instead of a function, then we can create a "meta-module" as the new top-level module to set up special args for the mkFlake submodule. Eventually the user could simply use nixpkgs's evalModules to evaluate the meta-module to build the flake:

(lib.evalModules {
  modules = [
    inputs.flake-parts.modules.metaModule
    {
      inherit inputs;
      publicModules.myPublicModule = {};
      mkFlake = {modules, flake-parts-lib, lib, inputs, ...}: {
        imports = [ modules.myPublicModule ];
        perSystem = { ... };
      };
    }
  ];
}).config.mkFlake.flake

@Atry Atry requested a review from roberth September 23, 2024 16:46
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.

4 participants