nix icon indicating copy to clipboard operation
nix copied to clipboard

Add setting handlers (side effects when settings are changed)

Open 9999years opened this issue 1 year ago • 6 comments

Motivation

One of the biggest differences between command-line arguments and settings is that command-line arguments have handlers, arbitrary functions that are run when the command-line argument is set, which may perform arbitrary side-effects. This is used by arguments like --log-format and --print-build-logs to make changes to global state in response to argument values.

This PR adds simple handler functions to settings, to gain parity with command-line arguments.

(Note: The use of global state in handlers for --log-format and similar arguments is unnerving and error prone. I believe this design will scale if we try to reduce global state in Nix — in the future, we can add parameters like EvalState to handler functions to make data flow explicit.)

I intend to use this functionality to implement two features:

  1. Settings with side-effects, like log-format and print-build-logs (to close tickets like #5858 and #5561).

    I have a PR which adds log-format and log-format-legacy settings written already, building on this PR: https://github.com/NixOS/nix/pull/9923

  2. Settings that change at runtime, to support commands like :set print-build-logs in the REPL/debugger (see #9944)

This PR also combines BaseSetting and Setting (there were no usages of BaseSetting on its own).

Priorities and Process

Add :+1: to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

9999years avatar Feb 03 '24 21:02 9999years

The idea of adding arbitrary side effects to configuration settings is scary to me. It makes it hard to reason about the configuration system. E.g. currently a configuration foo = 1\nfoo = 2 is equivalent to foo = 2, but if you can attach arbitrary side effects, that's no longer the case.

edolstra avatar Feb 09 '24 12:02 edolstra

@edolstra wouldn't that be OK as long as we are in control of the side effects we want to have? I think the only property we'll want to require is that handler(foo); handler(bar) is equivalent to handler(bar) (for some definition of equivalent of course), which is a reasonable-enough property. And we in indeed probably need that for things like #5561 or #9944

thufschmitt avatar Feb 09 '24 14:02 thufschmitt

I just glanced at this, but I am also skeptical. I think we should have a larger conversation about what the direction of travel should be with the settings. My highest priority is still getting rid of the global variables #5638 and supporting nested settings with JSON #8373.

Ericson2314 avatar Feb 09 '24 17:02 Ericson2314

My highest priority is still getting rid of the global variables https://github.com/NixOS/nix/issues/5638

Yeah, I was thinking about that. I think it would be pretty easy to replace the setting values with thunks, and then it would be easier to make the config values non-static.

9999years avatar Feb 09 '24 17:02 9999years

not sure why this is controversial. we already have the facility to add arbitrary side effects, it's just less convenient (requiring an explicit template instantiation and an implementation of parse or set). seems like codifying this on a per-setting basis would be more readable and clear than with custom settings types, and in any case a good first step towards refactoring to get rid of globals?

pennae avatar Feb 20 '24 18:02 pennae

I have given this more thought, and do think as part of the translation / compat layer we will end up needing something like this. But I want to work on what we are translating to first.

In particular, I would like to migrate the store settings off the current config.hh, and to something which the fetchers can also use.

Once that is done, I'll be more more comfortable adding bells and whistles like this to config.hh because it won't "pollute" the brave new world in the making.

Conversely, if we don't do things this way, I am afraid getting stuck half-way through with an even-more-complicated config.hh that everything is still closely tied to. Getting stuck halfway-through with some things already off the old system completely feels much safer to me.

Ericson2314 avatar Feb 20 '24 23:02 Ericson2314

#10562 reminds me I want the global variables gone, which reminds me that we're gonna need this as part of the transition.

Ericson2314 avatar Apr 19 '24 21:04 Ericson2314