Skip to content

feat(v1 API): document v1 API examples #468

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

Merged
merged 1 commit into from
Feb 6, 2023
Merged

feat(v1 API): document v1 API examples #468

merged 1 commit into from
Feb 6, 2023

Conversation

DavHau
Copy link
Member

@DavHau DavHau commented Feb 6, 2023

v1 API does not exist yet.
I want to make sure that all relevant use cases are sketched out before starting with the implementation.

Most notable changes that the new design will bring:

  • inspect options of any package via dream2nix man
  • override packages safely and conveniently using modules
  • no dependency on nix' flakes feature
  • better composition of outputs
  • vastly simplify integration of existing lang2nix tools into dream2nix (see examples in docs)

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/drv-parts-configure-packages-like-nixos-sytems/23629/18

Copy link
Member

@zimbatm zimbatm left a comment

Choose a reason for hiding this comment

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

I didn't finish reading the whole doc yet so it will require another pass, but I'm out of time for today

version = "2.0.0";

src = {
type = github;
Copy link
Member

Choose a reason for hiding this comment

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

The value here would a string isn't it?

dream2nix.modules.nodejs.package-lock
];

pname = "my-package";
Copy link
Member

Choose a reason for hiding this comment

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

pname is one of those historical accidents. Maybe it's a good opportunity to rename it to name.

Copy link
Member Author

Choose a reason for hiding this comment

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

I considered that, but am not sure if this would be a good idea as of now. We still use nixpkgs.mkDerivation as a builder for most derivations. If we put an adapter in front of it that changes the semantics we also have to ensure that this change is reflected by all downward compatibility mechanisms. For example we need to patch overrideAttrs to have that behavior as well.

For keeping things simple we might want to keep drv-parts based modules as close as possible to being a drop in replacement for the underlying builder API.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was resolved in the mean time. There is no pname anymore.

ref = config.version;
hash = "sha256-mia90VYv/YTdWNhKpvwvFW9RfbXZJSWhJ+yva6EnLE8=";
};

Copy link
Member

Choose a reason for hiding this comment

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

Same fetch API with patches I guess, except that it would take an array.

# The module system updates them for us.
src.hash = "sha256-LM5GDNjLcmgZVQEeANWAOO09KppwGaYEzJBjYmuSwys=";

deps = {nixpkgs, ...}: {
Copy link
Member

Choose a reason for hiding this comment

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

it's not clear why deps take another function. couldn't nixpkgs be passed to the parent function?

Copy link
Member Author

Choose a reason for hiding this comment

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

it's not clear why deps take another function

deps is our equivalent of package function args in nixpkgs. A single place to override dependencies.

couldn't nixpkgs be passed to the parent function?

We could pass nixpkgs via specialArgs but that would incurage usage like:

{config, nixpkgs, ...}: {
  buildInputs = [nixpkgs.python3];
  nativeBuildInputs = [nixpkgs.python3];
  preConfigure = ''
    ${nixpkgs.python3}/bin/python --version
  '';
}

This would make it hard to override python3, as there isn't a single source of truth.

Instead we want to limit the scope of packageSets like nixpkgs to a single attribute deps which makes overriding simple.

I should add some comments to the code to explain more about it.

Another cool thing is thatdeps is a submodule, so it allows us to specify options:

{config, lib, ...}: {
  options.deps.python3 = lib.mkOption {
    description = "Select a python3 version for building this package";
    # ...
  };
}

... which allows us to generate a manual for each individual package.

## Tied to flakes
The current api is tied to flakes. The v1 API should not depend on flakes anymore.

## Composability
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the rationale in encouraging the use of blueprints/package-functions/proto-derivations (naming in-progress). The goal would be to make flakes easier to compose, or to compose in different ways.

# the default dev shell for nodejs
dream2nix.modules.nodejs.mkShell
# adds dependencies of my-app to the dev shell
dream2nix.modules.nodejs.package-lock
Copy link

Choose a reason for hiding this comment

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

Why are these magic imports you have to replicate in each submodule instead of enable options provided by modules that you can just import once at a higher level (probably default.nix here)?

Copy link
Member Author

Choose a reason for hiding this comment

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

My intention was to keep evaluation overhead minimal. I wanted to avoid the problem of always having to import all modules, like seen in nixos as it appears to be limiting scalability and slow down evaluation.

I'm not too sure about my judgement here. Maybe it's not worth while and we should switch back to just using .enable = ....

Copy link
Member

Choose a reason for hiding this comment

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

I personally like importing the modules, because besides possible evaluation overhead IMO it can make it clearer which options are in scope.

instead of enable options provided by modules that you can just import once at a higher level (probably default.nix here)

Just to avoid misunderstandings: Those imports also affect all modules which end up the same config (or derivation in the context of drv-parts). So importing them at a higher level would also work. An argument for .enable could be made for the inverse, i think: .enable would allow us to then disable again in specific cases via enable = False which isn't as easy for imports.

Copy link

Choose a reason for hiding this comment

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

My intention was to keep evaluation overhead minimal. I wanted to avoid the problem of always having to import all modules, like seen in nixos as it appears to be limiting scalability and slow down evaluation.

I'm not too sure about my judgement here. Maybe it's not worth while and we should switch back to just using .enable = ....

I understand the desire to keep overhead low, and agree that it is very important, especially at the scale of every Nixpkgs package being a module. However, I have the concern that these repeated imports also have costs. (Ideally we could test evaluation time for different approaches over, say, 10_000 generated packages.)

  • Importing like this forces the evaluation of more than one module in virtually every case. I would like to know how this cost compares at scale.
  • Even if repeated importing saves on evaluation cost, discovery takes a hit. A language server / code intelligence that works with the Nixpkgs library module system is going to have a harder time suggesting options inside config when the modules defining the options aren't imported.
  • It's much easier to write conflicting modules when you can't develop against all modules being imported. Nix already has weak type safety and it doesn't need to be weaker.
  • Generating full option documentation should be possible in a single run by importing all the relevant modules, and having on-by-default or modules you can't disable makes this something you have to intentionally work for.

I personally like importing the modules, because besides possible evaluation overhead IMO it can make it clearer which options are in scope.

This argument is fine? I think there's space to explore scoping in the module system next to what we already have. Imports are a pretty rough tool.

instead of enable options provided by modules that you can just import once at a higher level (probably default.nix here)

Just to avoid misunderstandings: Those imports also affect all modules which end up the same config (or derivation in the context of drv-parts). So importing them at a higher level would also work. An argument for .enable could be made for the inverse, i think: .enable would allow us to then disable again in specific cases via enable = False which isn't as easy for imports.

I've been burned by non-disableable modules before. I firmly believe that, except in explicitly labelled, special cases, importing a module should not change current behavior. I also firmly believe that when modules are enabled by default, that should be accomplished through the presence of an enable option that defaults to true. Related to the scoping discussion from earlier, example.enable is basically the entry point for a module and the visible root of its options in documentation. Option paths are all we currently have for discovery & scoping inside of the module system, and choices for option paths should respect that importance. (I would personally love to see a way to reduce boilerplate when writing modules with a primary example.enable entry point option and config = mkIf config.example.enable ….)

Copy link

Choose a reason for hiding this comment

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

Actually, on the scoping note, I have a bit more to add. The Nixpkgs standard library's module system suffers from the module paths are isomorphic to lists of strings and lack of a namespaced symbol type, à la Common Lisp, and it suffers from the overloading of module paths for simultaneous module organization, structuring data, and code reuse.

As an example, the module <nixpkgs/nixos/modules/services/web-servers/nginx/default.nix> defines the option services.nginx.enable. Within the module system, to talk about this option provided by this module, all we get is the attr path [ "services" "nginx" "enable" ], for use via options."services"."nginx"."enable" or config."services"."nginx"."enable". There's no distinction between the namespacing provided here,[ "services" "nginx" ], and the option within this namespace, [ "enable" ], beyond the convention of "enable".

NixOS defines this option, but you wouldn't know it from the module path. Part of that is that Nix's syntax doesn't invite clean namespacing. If we were to namespace using Nix's current syntax, this might look like config."nixos.services"."nixos.nginx".enable, but this is ugly and repetitious. Bare attr paths are string literals, not references to lexical bindings, so even if you could bring declared services and nginx symbols from NixOS and an enable symbol from the standard library into scope (or even if you just bound lexicals like services = "nixos.services"), the syntax would be config.${services}.${nginx}.${enable} which is still really noisy. It's now clearer where an option set / option comes from, but at a cost to readability. All trying to solve this with selective imports means is that your module set is smaller. You still don't know what module declared an option without checking docs.

Because the module system runs on attrsets, the option with path [ "services" "nginx" "virtualHosts" "<name>" "locations" "<name>" "index" ] is only able to make clear the string-indexed attrsets of submodules it uses through documentation and conventions. "example" and "/" aren't special path components in [ "services" "nginx" "virtualHosts" "example" "locations" "/" "index" ]. They're not inherently notable for being strings where everything else is symbols, as could be the case in config.${services}.${nginx}.${virtualHosts}."example".${locations}."/".${index}. We overload option path components for organization of constrained option sets and for collection of arbitrary data. This also shows up with RFC 42 settings attrsets, which are fundamentally unconstrained data values. There's no syntax or visible typing in the language that brings attention to how { config = { services.xserver.displayManager.gdm.settings.debug.enable = true; }; } isn't working with a conventional enable option; you have to also know of the the settings convention.

(I don't know if or how namespaced symbols would fit into the Nix language, or if they'd really help with what I've brought up.)

@phaer phaer mentioned this pull request Apr 19, 2023
13 tasks
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.

6 participants