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

nixpkgs config: apply module system #16531

Closed
wants to merge 1 commit into from
Closed

Conversation

joelmo
Copy link
Contributor

@joelmo joelmo commented Jun 26, 2016

Not much have been moved to the module system yet. I want to hear what others think first.

@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @nbp and @Ericson2314 to be potential reviewers

@joelmo
Copy link
Contributor Author

joelmo commented Jun 26, 2016

@edolstra @zimbatm @vcunat

@bjornfor
Copy link
Contributor

Maybe it's obvious to some people, but I'd like to see the explanation for why this is is needed (in the commit message).

@Ericson2314
Copy link
Member

Can you rebase to fix conflicts? I recently made a change to remove toPath as you do which I think is the conflict. Will be easy to resolve :)

@Ericson2314
Copy link
Member

Ericson2314 commented Jun 26, 2016

@joelmo I think module system for config is a good goal, and maybe this is the proper first step, but there is some philosophical differences between nixpkgs and nixos that should be brought up. With nixos, it is essential to have a single global config: there are exhaustible resources that PID 1, ports, etc whose assignment must be agreed upon. With nixpkgs, that is not the case. To save space it's nice if config is propagated to dependencies, but usually not essential.

@Ericson2314
Copy link
Member

I'd actually lean towards making nixpkgs take no config, and then using the fix'-extend to do config on top.

@Ericson2314
Copy link
Member

A few misc things:

  1. How many people actually use nixpkgs config in a significant way? My impression is that it's old and shitty and I'd like build a complete new config system to lump breaking changes together.
  2. Extending on my first comment, modules are a straight-up inappropriate way to package software, because modules become singletons, and there must be a way to use the same package with different parameters, even if by default we want config to apply globally. (I know you haven't done this yet, but wanted to get the idea out there.)

@joelmo
Copy link
Contributor Author

joelmo commented Jun 26, 2016

The last sentence in the commit message motivates this change. If you can do all of that with the fix method in a convenient way, I am all for it. But it's unclear how to do that. The module system already works for NixOS, let's try it for nixpkgs.

For 2: this don't mean modules will be used for packaging. These are meant to define what's allowed in the config. There is still the same way to use packageOverrides, only now it's easier to discover, read up how to use that.

This applies the module system that is also used for NixOS. The most
desirable benefits are type checking, resolution control, and option
discoverability.
@edolstra
Copy link
Member

I agree with @Ericson2314 that the module system is not a good fit for Nix package functions. It would be better to extend the Nix language with some way to associate types and documentation with package function arguments.

@joelmo
Copy link
Contributor Author

joelmo commented Jun 27, 2016

This change doesn't take it this far to deal with package function arguments. Everything works just as before, this is only more convenient to document.

@Ericson2314, some global options for nixpkgs are already used. See the options for the browsers, chromium/firefox, that fits into a browser module. And packageOverrides are still possible for fine grained control.

If Nix gets a type system I imagine the module system will eventually use that too, so I don't see how we can loose by trying the module system.

There is also no plan for when the type system arrives to Nix. It would be useful to see a practical example/mock-up. I sense the most important properties are: optional typing, type checking, documentation ability. Given the lazy property, types need to be interpreted before type denotations?
There are also some other features to consider: type mutability (extendable types), access control. I believe a type for Nix can be a set with optional instance methods.

I made a sketch for the syntax, would this work?:

# Declaring a generic type
type Optional<T> {
  # Constructor 1 + documentation
  new T val: { val = val; }

  # Constructor 2, no argument
  new: { val = null; }

  # Instance method
  isEmpty: val == null;
}

# Creating a instance
{  hello = new Optional<String> "hello";
}

# Typed functions
String a: b: { inherit a b; }
{String a, b}: { inherit a b; }

# Default arguments?
{ Optional<String> x ? new "hello" }: 

I don't see any benefits with being able to denote type for attributes.

@zimbatm
Copy link
Member

zimbatm commented Jun 28, 2016

@joelmo I think what @edolstra meant was something simpler, just a chunk of text that can be associated with the function in the AST such as builtins.getDoc :: lambda -> string. With the right precedence it might even just be a new builtin function too:

let myFunction = doc
  ''
    This documents my function
  ''
  (a: b: a + b);

@Profpatsch
Copy link
Member

How many people actually use nixpkgs config in a significant way? My impression is that it's old and shitty and I'd like build a complete new config system to lump breaking changes together.

yes please.

It would be better to extend the Nix language with some way to associate types and documentation with package function arguments.

even yesser.

@Ericson2314
Copy link
Member

I do agree that a type system does some a long ways off. I was thinking we use the existing __functor__ magic key to do reflection.

@garbas
Copy link
Member

garbas commented Aug 7, 2016

@edolstra @Ericson2314 would something like this be better https://www.youtube.com/watch?v=ahVu3tjrriM? that way we can avoid extending the language and just use nix to document nix

@joelmo joelmo closed this Dec 6, 2016
@Ericson2314
Copy link
Member

@garbas Oh, that does seem even better than when I last looked!

@joelmo well I do want to thank you again for starting this conversation. I'm now thinking we should differentiate between 3 things:

  • nixpkgs: plain pure set of packages
  • nixuser: infrastructure for setting up a user's environment (role of today's config, dotfiles, etc), works on darwin too, relies on nixpkgs
  • nixos: infrastructure for setting up an entire system, relies on nixpkgs and nixuser

@joelmo
Copy link
Contributor Author

joelmo commented Dec 7, 2016

The module system for NixOS seems to work well, at least from my perspective. So this was a push to apply that system for nixpkgs. I don't think this is very important, there seems not to be many customizations for nixpkgs right now. The only option I use is enabling pepper in chromium (chromium.enablePepperFlash). Also having a consistent system for options and documentation would help exposing the interface to a package manager, I think this is a must if we want to attract "regular" users who want to use a stable Linux system.
I have not seen any need to let nix manage for example my bashrc, I backup such files every day and have the rollback mechanism arranged differently.
I did not understand why the module system is not a good fit for nixpkgs, the only drawback I seen is that options is declared in two different places. But with some modification I believe this is fixable.

@Ericson2314
Copy link
Member

@joelmo so what I am thinking is the module system would be perfect for this "nixuser" layer. The singleton aspect of nixos that I was worried about doesn't apply here because the user indeed does want want not more than one firefox, chromium, etc.

@joelmo
Copy link
Contributor Author

joelmo commented Dec 7, 2016

If I remember how this patch functioned, it also wont affect the current "nixuser" layer, which i think is good if we want to migrate.

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.

8 participants