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

[WIP] Add 'perSystem' and 'systems' schema attributes #6773

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gytis-ivaskevicius
Copy link
Contributor

Very minimal POC and resolution to #3843, mostly copied merging logic from flake-utils-plus since we already merge schema without depending on nixpkgs.

TODO:

  • Clean up, think of a nicer merging implementation
  • Add schema validation (systems should be always a list, perSystem should always be a function)
  • Add tests + look for edge cases (All in all this is mostly fu/fup code that has been tested by many users, it there should not be a lot of edge cases)

If anyone wants to play around with this please use flake.nix as a usage reference

@@ -1132,7 +1132,8 @@ struct CmdFlakeShow : FlakeCommand, MixJSON
}
}

else {
// Ignore 'systems' and 'perSystem' attributes
else if (!(attrPathS[0] == "systems" || attrPathS[0] == "perSystem")) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If i do attrPathS[0] != "systems" && attrPathS[0] != "perSystem" it throws compilation error:

src/nix/flake.cc:1136:68: error: no match for ‘operator!=’ (operand types are ‘__gnu_cxx::__alloc_traits<std::allocatornix::SymbolStr, nix::SymbolStr>::value_type’ {aka ‘nix::SymbolStr’} and ‘const char [10]’)

@edolstra
Copy link
Member

edolstra commented Jul 8, 2022

I don't think this is the right approach. system should not be part of the flake spec, since there is no consensus what system type specifications should look like in the long term (e.g. to take cross-compilation into account).

call-flake.nix shouldn't have any sophisticated logic in it. In particular, it shouldn't have any special handling for specific outputs like hydraJobs.

@gytis-ivaskevicius
Copy link
Contributor Author

Yes, I did consider cross-compilation. systems can be list of anything (for example attribute set in case of cross). I think this should keep the schema future compatible for a long long time. And we are not getting rid of usual <system> dependent properties definition, these attributes are being merged together with perSystem result.

call-flake.nix shouldn't have any sophisticated logic in it. In particular, it shouldn't have any special handling for specific outputs like hydraJobs.

I disagree, having it implemented in nix allows us to easily copy-paste it over to flake-compat project.

From what I noticed community wants to get rid of flake-utils and similar projects because it's bothersome and bad UX. If this is not something that you are happy about maybe we could have a compromise of sorts? How would you like such feature to get implemented?

@roberth
Copy link
Member

roberth commented Jul 9, 2022

community wants to get rid of flake-utils and similar projects

Any expression we hardcode in the nix package becomes part of the language and will stick with us forever. Otherwise, we lose a unique selling point, which is reproducibility, including decade+ old expressions. This is at least one reason to reject "sophisticated logic".

Personally, I'd rather have a complete break from the current schema while it's experimental than an eternal migration logic that we can't get rid of. I'm just not sure if it's de facto still experimental though... The choice is between breaking a lot of experimental code now, or eventually breaking language reproducibility in the future as the compatibility logic bitrots.

Perhaps the compatibility logic should be in a library? A special input named flake-schema-compat? Or perhaps call-flake.nix's role could be reduced to bootstrapping a locked flake called call-flake or flake-wiring? It's not pretty, but at least it's reproducible and maintainable because the lock file will pin all the "sophisticated logic".

I wish it was simple... but simple will upset a lot of people after 3+ years. Quite the dilemma.

See, truly simple is reducing the inputs to { outPath, sourceInfo } and letting "user space" do the all the wiring...
Just as powerful though. Still needs an input to do the wiring, just like today.

@gytis-ivaskevicius
Copy link
Contributor Author

Any expression we hardcode in the nix package becomes part of the language and will stick with us forever.

Yes, that is true but I'm sure that we will always have some sort of iterable concept of system and as long as it's flexible enough - we should be fine.

Personally, I'd rather have a complete break from the current schema...

👍

Perhaps the compatibility logic should be in a library?

No, I think we need a design that is capable of designing a proper schema out of the box because it's causing more problems than it's worth. More info here: https://github.com/gytis-ivaskevicius/nix-patterns

See, truly simple is reducing the inputs to { outPath, sourceInfo } and letting "user space" do the all the wiring...

Yeah, I was playing around with this idea in the back of my mind, but could not come up with anything with actual nice UX

@nrdxp
Copy link
Contributor

nrdxp commented Jul 21, 2022

Personally, I'd rather have a complete break from the current schema while it's experimental than an eternal migration logic that we can't get rid of.

FWIW this is my biggest let down with flakes so far, there has basically been 0 experimentation with the flake schema since the day they were released despite the various gripes from different corners of the community. I would really like to see something materialize before it reaches stable, and I don't mind refactoring all my companies code by hand (I mean it really wouldn't be super hard honestly) if it means a better product in the end, rather than one we will be "working around" for years to come.

Although it is still always possible to have a clean break at some point in the future, I guess it might even be a little late to complain since stabilization looks like it's coming.

@roberth
Copy link
Member

roberth commented Jul 21, 2022

@nrdxp

I guess it might even be a little late to complain

We're not late to complain. The issue was raised two years ago #3843, one and a half year before flakes were first released experimentally in 2.4, reminded and raised by others on numerous occasions.

I believe the issue has been ignored by the maintainers initially because some of the initial proposals weren't improvements. The original premise of the linked issue was "builtins.currentSystem as a input to flakes", which is not the right approach, but the subsequent thread has made abundantly clear that there's an underlying problem that needs to be solved, and that it can be solved.

@gytis-ivaskevicius

Perhaps the compatibility logic should be in a library?

No, I think we need a design that is capable of designing a proper schema out of the box because it's causing more problems than it's worth. More info here: https://github.com/gytis-ivaskevicius/nix-patterns

In nix-pattern you argue for using overlays, which are known to interfere with each other. It's good for overriding, but not for composition. Usually what we need is composition.
More fundamentally though, you're using Nixpkgs overlays as the dependency injection framework, which to me suggests that we should let the "user space" (which includes Nixpkgs) to perform the dependency injection.
If flakes were to remove

anything with actual nice UX

I would work towards something like this. Here's how a "legacy" flake would look:

  outputs = args@{ flake-wiring, ... }:
    flake-wiring.lib.inject args (
      # inputs processed by flake-wiring to equal what call-flake.nix offers
      { nixpkgs, ... }: {
        # same as the current flake format
        packages.x86_64-linux.hello = ...
      }
    );

I admit that { outPath, sourceInfo } was sub-optimal. I did not intend to say that Nix shouldn't import the flake at all. Better pseudocode would be:

imported.outputs // { inherit (node) outPath sourceInfo; }

flake-wiring returns attributes that tell Nix what's in the flake. This will be an interface between Nix and the wiring framework that flake writers don't need to care about.

I haven't implemented this, but it seems feasible.

Crucially, it allows a lot of flexibility without causing too much UX "damage". Almost every flake already uses some sort of framework for creating per-system outputs. These frameworks can absorb the flake-wiring call, so that the boilerplate per flake does not actually increase.

I have zero confidence that we can hardcode a good dependency injection solution into Nix, so we should leave this to the ecosystem, where it can be pinned and where it can be collaborated on more easily.

I agree that there's a UX cost when we don't hardcode a solution in Nix, but let's stay in the realm of what's actually feasible.

Perhaps the general form of the invocation can be hardcoded, going from

# flake.nix
{
  outputs = args@{ flake-wiring, ... }:
    flake-wiring.lib.inject args <<f>>;
}

to

# flake.nix
{
  wiring = flake-wiring;
  definitions = <<f>>;
}

@gytis-ivaskevicius
Copy link
Contributor Author

In nix-pattern you argue for using overlays, which are known to interfere with each other.

No, I am arguing for using nixpkgs independent data structure. At the current time overlays is the only official structure that fits the bill. At its core its just a function that we can call without any interference. https://github.com/gytis-ivaskevicius/nix-patterns/blob/master/02-packages-producer-with-dev-deps/develop/flake.nix#L15=

Since by now I talked with several peeps that unreasonably hate overlays even if you treat them as a functions - a new data structure was proposed https://github.com/gytis-ivaskevicius/nix-patterns/tree/master/blueprints

I would work towards something like this. Here's how a "legacy" flake would look:

It involves unnecessary dependency -> dependencies cause problems -> lets remove need for dependencies -> idea behind nix-patterns.

I have zero confidence that we can hardcode a good dependency injection solution into Nix

We should never need to do that. We should not treat follows mechanism as DI

About hardcoding, I was chatting with @nrdxp and basically this copy-paste more or less summarizes my opinion:

so 'system' is more of a nixpkgs concept
in fact, nix is lagging behind since we have hostSystem targetSystem and buildSystem (or something similar)
and we don't want to get locked into this system concept
BUT
no matter how/when we will always have some sort of 'system' identifier since there will always be multiple operating systems and multiple CPU architectures

so technically we already have some requirements:
- An array of unknown elements representing `system`
- We must be able to stringify elements so it would be accessible via CLI
so basically all we need is a structure that supports any kind of unique identifier
and perSystem pr technically is backward compatible and supports covers these points

the only counterargument I can think of is if we ever start building attrs as such: `packages.x86_64.linux.default = drv`
but that's really pushing it,  I can't really imagine us doing that, and its already not compatible with our current schema anyways

Recently @edolstra stated that we are not going to be changing schema anytime soon
So here are my following ideas to resolve this issue are:

  • Get $ operator merged and get some clean pattern going Add haskell's $ operator to the nix language #5577 for all of this
  • Get some custom built-in to simplify the definition of eachSystem function. As of right now, I can't think of one that would reduce the boilerplate
    Minimal eachSystem definition:
  eachSystem = systems: f:
        let
          op = attrs: system:
            let
              ret = f system;
            in
            builtins.foldl'
              (attrs: key: attrs // { ${key} = (attrs.${key} or { }) // { ${system} = ret.${key}; }; })
              attrs
              (builtins.attrNames ret);
        in
        builtins.foldl' op { } systems;

@roberth
Copy link
Member

roberth commented Jul 21, 2022

It involves unnecessary dependency -> dependencies cause problems

I don't understand how you can then argue to hardcode dependencies into Nix.

We should never need to do that. We should not treat follows mechanism as DI

It seems that we agree on the simplification of call-flake.nix then.

so 'system' is more of a nixpkgs concept

True, but we need some interface between commands such as nix run and the expression, to select an expression that can run on the current machine. This is hostPlatform.system. Any remaining details that are not in the system double can be filled in by the expression. Cross is not special in this regard.

For nix build the current meaning of system is more ambiguous. Nix does not impose any restrictions, so it's kind of up to the user to give meaning to system in this case. It does incentivize to treat system as the buildPlatform.system, which contradicts the meaning of system in Nix run.

in fact, nix is lagging behind since we have hostSystem targetSystem and buildSystem

I don't think this is the right perspective. First off, Nix doesn't need to know about targetSystem at all, as that's a Nixpkgs internal affair. What remains seems to be a general configurability problem and ambiguity I described before.

$ operator

Let's focus on the semantics, shall we. Sure $ may be nice, but this is entirely superficial. Does it solve a real problem people encounter when they use flakes? No. Does it enable a new application of flakes? No. Does it improve our ability to improve flakes? No.

  • Get some custom built-in to simplify the definition of eachSystem function.

More hardcoded junk we won't ever get rid of? It's not like we can let users pin Nix, evolve it and let users catch up with the breaking changes. Even if we could, that destroys one of the unique selling points for Nix: being able to build any old expression.
Nix must be a small core in order to fulfill its goals. call-flake.nix and to some degree flakes itself have set a bad precedent that we need to get away from, or face the consequences with piles of tech debt and/or breakage.

reduce the boilerplate

Boilerplate isn't boilerplate anymore when you can put it in a library.

A library dependency is good, because it can be pinned, it can evolve, and it can be replaced entirely. If you think a single dependency and a single call are "boilerplate", you need to reconsider what boilerplate means.

@gytis-ivaskevicius
Copy link
Contributor Author

I don't understand how you can then argue to hardcode dependencies into Nix.

I just don't see it as a hardcoded dependency. It's more of a lets assume that now and in the future systems will have unique identifiers since there will always be multiple CPU architectures and multiple operating systems

I don't think this is the right perspective. First off, Nix doesn't need to know about targetSystem at all, as that's a Nixpkgs internal affair.

I honestly don't care if nix knows about it or not, I am just strongly against limiting users and thus not covering use cases like cross-compiling or overwriting. Currently, users tend to consume the packages attribute which has very limited cross support as well as multiple areas by the flake author where may easily screw up (importing/extending nixkpgs and lack of callPackage are main culprits)

Let's focus on the semantics, shall we. Sure $ may be nice, but this is entirely superficial. Does it solve a real problem people encounter when they use flakes? No. Does it enable a new application of flakes? No. Does it improve our ability to improve flakes? No.

I have not played around with it but I bet with help of this operator we might be able to build some clean approach to defining system-dependent attributes.
What nix needs is adoption, and fixing UX issues will push nix community very far ahead. I don't care how it's done, it just needs to get done

More hardcoded junk we won't ever get rid of?

We introduce extra builtins to speed up evaluation. If we can think of a builtin that speeds up evaluation and helps us to express eachSystems function - that's a win-win. I am not suggesting to hardcode whole eachSystems function, that's obviously a bad idea.

Boilerplate isn't boilerplate anymore when you can put it in a library.

The moment you put it into the library we return to step 1. unnecessary dependency -> dependencies cause problems -> let's remove the need for dependencies -> idea behind nix-patterns

Sometimes using a library is a great idea just because of convenience, but that is not the case for 95% of flakes since it's not necessary.

@gytis-ivaskevicius
Copy link
Contributor Author

Anyways, @roberth lets agree to disagree. This is going nowhere

@roberth
Copy link
Member

roberth commented Jul 21, 2022

What nix needs is adoption, and fixing UX issues will push nix community very far ahead.

This attitude is sometimes known as "success at all costs" and it leads to bad design that ultimately holds the system back.

Yes, we should fix UX issues, but not by compromising the design, or compromising features like reproducibility, or compromising our ability to evolve the system.

agree to disagree

Yes, we have a fundamental disagreement about simple vs easy.

Easy should follow from simple. The other way around is hard to sustain and hard to adapt to new use cases.

Unless we can agree on this, I don't see a point in continuing this discussion either.

@roberth roberth self-assigned this Dec 28, 2022
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2023-01-06-nix-team-meeting-minutes-21/24573/1

@fricklerhandwerk
Copy link
Contributor

fricklerhandwerk commented Jan 13, 2023

Discussed on the Nix team meeting 2023-01-16:

  • @roberth: brought this up because there was a Nixpkgs PR trying to address this issue, but it should be addressed in Nix
    • general idea: extend the schema with a function perSystem and a list of systems
    • @roberth: systems should actually be optional
    • @thufschmitt: don't understand how the PR changes the interface, it only adds a convention
      • @roberth: it doesn't change the interface between the flake and the CLI
        • the call looks for the persistent perSystem function, and produces the output artifacts based on that
          • we should provide compatibility
    • the system attribute is a step in the learning curve that we would smooth out that way
  • @edolstra: problem is that right now you can enumerate the contents of a flake on any system. if the system is not specified, one cannot tell what is available for which system.
    • @roberth: you could only evaluate for the current system

    • @edolstra: that's no good UX, it may work on one system but not the other

    • @roberth: that has always been that way

    • @thufschmitt: also it doesn't guarantee anything, it may still break due to broken build setup for different systems

    • @Ericson2314: even if you have a whitelist, you should not restrict what's possible otherwise

      • with release.nix one can pass parameters ot default.nix
      • this is how software is generally developed, not knowing how it will be used in the wild, and we should not restrict that
    • @tomberek: reusable components should not be hidden in a let expression

      • instead of exposing things per system, we should expose functions along the lines of callPackage which take a system as parameter
      • the alternative people are currently using is either a big let or a bunch of overlays
      • @roberth: it's the most flexible thing, but there is something in between
        • people may want to continue using the existing glue
    • @fricklerhandwerk: worried about growing the API surface

      • even if people use flakes in production, we should take advantage of them being experimental and make breaking changes when we find a design that will serve longer
        • @roberth: it's not much work to add compatibility, but we should indeed just deprecate the old style
      • what about the proposal making system an input parameter?
        • @roberth: that's a bit wrong. you'd need a system just to define an overlay
        • @edolstra: also you'd have to evaluate multiple times to access different systems
        • @roberth: one could also consider "configurable flakes", where one could imagine the system as a configuration parameter
          • @edolstra: long-term we want something more generic, as people will want to pass other things to flakes
            • something like a module system would make it discoverable, then you'd have a system option with documentation and could override it etc.
  • @roberth: think we're running into scope creep. would like to make progress with flakes, and get an answer.
    • do we want to implement this design or should we go into the extended scope of configurable flakes?
    • @thufschmitt: like the design because it's a small step
    • @Ericson2314: how do you imagine integrating it with the CLI?
      • @roberth: it will call perSystem and should eventually support flakes that don't declare a system at all
    • @tomberek: call-flake.nix itself is difficult to use directly and experiment with, because it needs the lockfile string directly
      • a primop that reveals the lockfile/string would allow people to write their own call-flake.nix and craft the interface as desired
    • agreement: make another round of discussion on just this question

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2023-01-13-nix-team-meeting-minutes-23/24644/1

@fricklerhandwerk
Copy link
Contributor

Discussed in the Nix team meeting:

This appears like high effort and needs more discussion, but given our current effort with regard to RFC 136 it is low priority. We're not tracking this PR any more for now.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2023-04-14-nix-team-meeting-minutes-48/27358/1

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/experimental-does-not-mean-unstable-detsyss-perspective-on-nix-flakes/32703/25

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