zalora / deployix

zalora nix expressions library
MIT License
17 stars 5 forks source link

Settle on expression style and composition #22

Closed shlevy closed 9 years ago

shlevy commented 9 years ago

Less than a week after designing it and introducing it I'm dissatisfied with the current composition setup (I blame @proger for planting the seeds of doubt :stuck_out_tongue:). Rather than waste time redesigning it a million times, I thought I'd open this to discuss the various options and try to come up with something with the team that will fit our needs best.

For terminology, I'm distinguishing between "composing expressions" that tie a bunch of things together and "terminal expressions" that actually define the package or function that users of defnix will use.

Basically there are two interrelated questions:

  1. In a terminal expression, how do we represent dependence on other values defined in other terminal expressions or externally? Note that for terminal expressions meant to define functions there is a somewhat-fuzzy distinction between "arguments to that function" and "external dependencies". For example, the php-fpm service is a function that depends on a php binary and the socket-activation function, takes a socket path and configuration file, and returns a service definition.
    1. This may not be relevant to defnix, but it will be if we start adding a lot of proper packages: There are often packages that have configuration flags (e.g. useGtk in nixpkgs.pinentry). These flags aren't really external dependencies, but we'll also want to be able to just refer to the package and get some default settings so representing them as arguments to a function impedes usability.
  2. How do we tie all of the terminal expression together?
shlevy commented 9 years ago

Some options for 1:

  1. Pull in dependencies explicitly. So for example, the fpm service might look like:

    let
    php = import ../../pkgs/php.nix;
    socket = import ../activations/socket.nix;
    in { socket-path, config }: <etc>
  2. Assume dependencies are in scope (via builtins.scopedImport). fpm service example:

    { socket-path, config }: <something-using-php-and-socket>
  3. Take dependencies as arguments, all in one function. fpm service example:

    { php, socket }:
    { socket-path, config }: <etc>
  4. Take dependencies as arguments, with a different function for each dependency category. fpm service example:

    pkgs@{ php }:
    activations@{ socket }:
    { socket-path, config }: <etc>
  5. Take dependencies as one big set. fpm example:

    deps:
    { socket-path, config }: <something-using-deps.php-and-deps.socket>
  6. Take dependencies as one set per category. fpm example:

    pkgs:
    activations:
    { socket-path, config }: <something-using-pkgs.php-and-activations.socket>
  7. One of the above, structured along with some level of introspectable information about the dependencies it uses. fpm example currently in use:

    lib.composable [ [ "defnixos" "activations" ] "pkgs" ] (
    activations@{ socket }:
    pkgs@{ multiplex-activations, execve, php }:
    { socket-path, config }: <etc>

    which evaluates to

    {
    args = [ [ "defnixos" "activations" ] "pkgs" ];
    fn = activations@{ socket }: pkgs@{ multiplex-activations, execve, php }: <etc>;
    }
shlevy commented 9 years ago

Some options for 1.i:

  1. Treat flags as dependencies, in a config dependency source
  2. Implement packages with flags as functions (possibly with default args, but still need to be called)
  3. Take a config argument as a dependency
  4. Define the package directly, and add a configure function to the resulting derivation that allows re-configuring with non-default flags
shlevy commented 9 years ago

Some options for 2, depending on choices chosen for 1:

  1. No composition at all, just import everything and let the user compose (or, if 1 is selected at all, no composition needed)
  2. Explicitly pass needed arguments to each function
  3. Automatically pass needed arguments based on the named arguments (e.g. if a function has { php }:, we know to pass php).
  4. Compose a scope to use with builtins.scopedImport
  5. Automatically pass arguments based on structured information provided in option 7
  6. Automatically pass arguments based on convention about ordering and number of dependency sets
  7. Any of the above, with the option to override values (e.g. set pkgs.php to php55)
  8. Any of the above, with the option to override arguments (e.g. set the php dependency of the fpm service to php55).
ghost commented 9 years ago

Hi @shlevy,

tl;dr: I am for Option 3!

I personally like Option 3 and Option 4:

Option 3:

{ php, socket }:
{ socket-path, config }: <etc>

Option 4:

pkgs@{ php }:
activations@{ socket }:
{ socket-path, config }: <etc>

Option 3 can be made similar to Option 4:

# Packages
    { php }:
# Activations
    { socket }:
{ socket-path, config }:

I'm assuming invocation goes something like the following for Option 3:

# Nix file using `php-fpm` Nix file
{ php-fpm, ... }:

 php-fpm {
    php = <function or path to php library>;
    socket = <socket function>;
    socket-path = <socket path>;
    config = <config record>;
}

I'm assuming invocation goes something like the following for Option 4:

# Nix file using `php-fpm` Nix file.
{ php-fpm, ... }:

php-fpm {
    pkgs = {
        php = <function or path to php library>;
    };
    socket = <socket function>;
    activations = {
        socket-path = <socket path>;
    };
    config = <config record>;
}

Generally, for both options 3 and 4, I can use the concept of closures/nested-functions to categorise the arguments to give them some contextual meaning.

Comparing both function invocation styles and function definition styles, I prefer Option 3 to 4, simply because there is lesser indent levels to deal with. Option 4 is not really very nice to write, as I have to open new block/record structure to define, and I have to incur additional level of indentation which makes the code look somewhat ugly. It would probably come in handy if I pass in an entire record to the activations or pkgs parameter, though.

With my limited knowledge in Nix language, my gut feeling is with Option 3, as both the invocation style and the definition style looks neat and nice.

Without starting a language flame war here, I love the visual style of Python code. If I were to be making a tool like Nix package manager, I'll have my expressions represented as Python code directly:

# php-fpm expression file: php_fpm.py

from nixpkg.php import php
from nixpkg.activations import socket

def php_fpm(socket_path, config):
    # Logic for building php-fpm goes here.
    php(config)
    socket(socket_path)

Its invocation would be simply the following:

# some expression file, which uses php-fpm expression file: some_expression.py

from .php_fpm import php_fpm

def some_other_expression(*args, **kwargs):
    process_arguments(args)
    php_fpm(*args)

def process_argument(args):
    '''
    This is a helper function within the expressions file.
    '''
    # Logic for processing argument goes here.
    args.append('dummy value')

For those who know how Python works, I think you would agree with me that it is obvious where the imported symbols come from.

No matter the style adopted, I would like for the Nix expressions to be readable and inferable; someone who does not know Nix should more or less know what the expressions are trying to achieve.

I should note that while I was learning Nix, I found parts of it very similar to Python. The default.nix, is akin to __init__.py, for instance. I suspect there were some Pythonic influences in Nix. :wink:

proger commented 9 years ago

Does this mean that https://github.com/zalora/defnix/pull/4 is back on the table? :)

tl;dr: dumbest solution ever

The idea is to have (almost, where applicable) every terminal expression have defnix and pkgs (this one doesn't necessary have to refer to nixpkgs but to any attrset that contains references to related derivations/packages) as its arguments. Only the top default.nix in defnix is customized. This is quite simple, doesn't require any automatic implicit things you have to think or know about. If anything requires customization, it's possible to re-import the whole defnix again and again, or, where really needed, monkey-patch things with overrideDerivation or the like.

rationale: nix expressions aren't really the thing i want to spent time writing and if they're too complex or require too much customization for every case — something is wrong somewhere

shlevy commented 9 years ago

@multoncore You have the invocation a bit mixed up for 3 and 4.

3:

let php-fpm-fun = defnix.services.php-fpm { php = ...; socket = ...; };
in php-fpm-fun { socket-path = ... };

4:

let php-fpm-fun = defnix.services.php-fpm { php = ...; } { socket = ...; };
in php-fpm-fun { socket-path = ... };

No need for extra indentation, and for the most part the end user will be presented with php-fpm-fun directly without needing to pass php or the socket activation themselves (that will only be needed when overriding the default is relevant)

shlevy commented 9 years ago

@proger That's basically option 6 above, right? Any reason it's preferable to 4 (which just makes explicit which arguments come from each set, so instead of defnix.socket you can just use socket)?

proger commented 9 years ago

it's more nixos-module-like (like any nixos module with parameters without config) rather than 6. I want to avoid handling marshaling-unmarshaling various attrsets by hand.

proger commented 9 years ago

Key examples: https://github.com/zalora/defnix/blob/95d9f7cc498a8b6a23b65bbbe88e8d8e1a3078c5/default.nix and https://github.com/zalora/defnix/blob/95d9f7cc498a8b6a23b65bbbe88e8d8e1a3078c5/defnixos/services/strongswan.nix

I am not even against the module system (its interface), but I'd rather run it multiple times to avoid strong dependency on the core parts of the system (i.e. own evalModules runs for service-related things as granularly as possible vs the core evalModules pass that handles boot/kernel/sysctls/locales/whatever boring stuff there is)

shlevy commented 9 years ago

What do you mean, marhsalling-unmarshalling various sets by hand?

shlevy commented 9 years ago

Also I don't understand your last paragraph in your latest comment.

ghost commented 9 years ago

@shlevy, is let..in mandatory in all options? I'd like to try to avoid let..in if possible. I think seeing a Nix file as a function would probably be better for readability. let..in introduces additional scoping, which I would prefer to use only when I really need to introduce new local scopes.

@proger, the only thing I'm trying to suggest here is that the code looks readable and intuitive. Nix isn't my forte, so I don't think my suggestions would be well-informed. Not sure how Option 3/4 would be dumbest here, if I didn't interpret your reply wrongly. I strongly agree with you on not having to spend so much time writing Nix expressions

jekor commented 9 years ago

@multoncore: I don't think @proger's comment was directed at yours. I think it was directed at Shea's #4.

shlevy commented 9 years ago

@rickynils Do you think you can look over my big walls of text and chip in your thoughts at some point this week?

shlevy commented 9 years ago

@multoncore Certainly let-bindings are not mandatory.

proger commented 9 years ago

@shlevy i tried to mean that we could use the actual module system, but split the configurations it builds into multiple parts (1x core system using modules solely from nixpkgs, and Nx our services that we deploy based on stuff from zalora-nix-lib, defnix and other repos)

proger commented 9 years ago

(i.e. it's interface is already well-documented and contains enough discussions within the community)

shlevy commented 9 years ago

The module system has several problems, including slow evaluation and the global namespacing of arguments, which we are already working around with hacks in eris and epsilon. Why do we want to use it?

rickynils commented 9 years ago

@shlevy I'm going to think more about this, but here's my initial thoughts:

I think the nix import statement (also along with the nix path) is a bit difficult to handle, in a sense it is more "global" than the NixOS module system, since you can simply pull in stuff from from any place with it. So IMO, we shouldn't rely on it in terminal expressions, like 1.1 does. We can import stuff on a higher level, where we compose things, but ideally we should settle for one way to pass stuff into nix. Currently we have roughly three ways: file path imports, nix path imports and top-level arguments.

Other than that, I don't have have any strong opinions about 1, as long as 2 is implementable with the style we select. I think it is important that you can override stuff in a straightforward way, especially for pkgs, where I would like to see that it is possible to use packages from different revisions of the same repo. That way, you could pull in new packages or security updates from upstream without messing up all of your current (tested) packages.

About 2: I think our end goal is to be able to freely define and configure a set of "functionalities" (or services, although that term is overloaded), not coupled to specific machines. In fact, the physical infrastructure should in some cases maybe even be computed automatically from the functional specification (although there are things like runtime state and auto-scaling to take into account also). If I understand @proger correctly, maybe we should keep NixOS modules for the things that are machine-specific (the boring parts, as Vlad puts it), and then do defnix modules for the "true" services that don't have machine-dependencies. @proger is that what you mean, or do you think we can use the NixOS module interface in defnix in some way?

shlevy commented 9 years ago

OK, based on this discussion I'm currently leaning toward 5 for 1, 1 for 1.i if it becomes relevant, and 7 for 2. So each terminal expression receives a defnix argument, and the top-level composition allows overriding the values for specific terminal expressions (so I can import defnix with defnix.pkgs.php set to php 5.5), and any more complex overriding can be done by either importing the top-level composition multiple times, or accessing the un-composed functions directly.

@proger @rickynils @soenkehahn Please register objections soon if any :smile:

shlevy commented 9 years ago

@rickynils I definitely agree that we should encapsulate boring machine-specific stuff inside of functions that take the set of services that we actually care about. Whether we use nixos under the hood should be an implementation detail (though I expect we'll end up not wanting to). @proger's point was different, I think.

soenkehahn commented 9 years ago

I agree that we should try to avoid import for pulling in things from the NIX_PATH but rather pass in all dependencies as arguments. I'm not really against importing files from the same repo through relative paths, though.

For 1.i: What's the difference between 1 and 3? Or rather: What's a dependency source?

For 2: 7 sounds good. I would shy away from automatically inspecting functions and pass in arguments as needed. That's exactly the kind of magic that obfuscates things. If we have a proper convention (i.e. every file takes a dependency argument and allows for arbitrary additional fields in that argument ({pkgs, config, ...}: *stuff*)) then that shouldn't be too inconvenient, I imagine. Also I would avoid to build custom layers of reflection like the composable things. I see it as a great advantage to stick to simple nix expressions. (We would probably want nice overriding functions for the dependency arguments we pass into expressions.)

On local imports: What about importing modules from the same library? I always found it a little weird to pass in e.g. build-support (part of defnix) into another part of defnix. in the extreme case you can build import loops that way. How do we deal with that? (As I mentioned above, I'm not really against importing local files. You could still build import loops that way, but it would be a little bit more obvious when you're creating an import loop. On the other hand we can't override anything imported that way. For something like helper functions that would be ok, but probably not for every local dependency.)

And what about the upstream pkgs (aka nixpkgs)? It seems like in defnix that's not meant to be available, at least not unwrapped. Am I misunderstanding this? Shouldn't it be easily available?

shlevy commented 9 years ago

@soenkehahn

1.i: Looking back on it I can't remember what the difference was, but I swear there was one :tongue:

2: Yeah, ultimately I think the simplicity of 7 wins out.

The problem with local imports is that the stuff in build-support needs to be composed too. Currently multiplex-activations depends on compile-c, should it instead have to know that compile-c depends on cc and coreutils and pass those in manually? And what if we want to override compile-c in a way that is more complex than just overriding cc?

For upstream pkgs, there are two reasons for the wrapping. First, I want it to be hard to accidentally update which nixpkgs revision we're using for a given package when updating for another one, which if we're passing around an imported nixpkgs set untouched will be difficult. Second, it allows us to maintain the distinction between build-support, eval-support, and pkgs which I think organizes our tree better.

shlevy commented 9 years ago

Fixed by #23