NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.12k stars 14.15k forks source link

Description of localSystem is not helpful #49510

Open coretemp opened 6 years ago

coretemp commented 6 years ago

Issue description

The description of localSystem is not helpful. Please rewrite this in a way where it is easy to see that if you want to do a deployment of a NixOps managed Linux machine on a Mac that you should set this.

In fact, the concept of defaulting to the platform on which one is running is flawed, because it is not functional anymore.

Functional means that "equal inputs should have equal outputs", but now we have that on a Mac there will be a different output.

Please explain why this defaulting mechanism was committed in the first place.

Additionally, until this is resolved, seeing a simpler change in which a warning is displayed if you are deploying something which does not specify this variable would be great.

    localSystem = mkOption {
      type = types.attrs; # TODO utilize lib.systems.parsedPlatform
      default = { inherit (cfg) system; };
      example = { system = "aarch64-linux"; config = "aarch64-unknown-linux-gnu"; };
      # Make sure that the final value has all fields for sake of other modules
      # referring to this. TODO make `lib.systems` itself use the module system.
      apply = lib.systems.elaborate;
      defaultText = literalExample
        ''(import "''${nixos}/../lib").lib.systems.examples.aarch64-multiplatform'';
      description = ''
        Specifies the platform on which NixOS should be built. When
        <code>nixpkgs.crossSystem</code> is unset, it also specifies
        the platform <emphasis>for</emphasis> which NixOS should be
        built.  If this option is unset, it defaults to the platform
        type of the machine where evaluation happens. Specifying this
        option is useful when doing distributed multi-platform
        deployment, or when building virtual machines. See its
        description in the Nixpkgs manual for more details.
        Ignored when <code>nixpkgs.pkgs</code> is set.
      '';
    };
matthewbauer commented 6 years ago

I'm open to suggestions for improvements here but TBH I think that's a pretty good description of what localSystem does. As for defaulting to something else, what would you suggest? On macOS we could default to localSystem = { system = "x86_64-linux"; } but that seems just as arbitrary to me. We support many architectures so the user has to be specific on what they want.

coretemp commented 6 years ago

It should be an error to not specify it given the above logic. That error could show some common use cases.

I still don't know what the option actually means.

Specifies the platform on which NixOS should be built.

I have read similar descriptions in GNU tools and those target system, etc., could be understood with some effort. This is not something you can give to someone and expect them to understand it.

It's not clear to me what the set of valid values is for example (I already looked at the source to get some sort of idea), but if I want to deploy nix to a Linux machine from a Mac system, I don't see what I should specify (nor do I see why I should specify something, because I already specified localSystem), but perhaps I missed in in one place (which brings us to the issue of defaults).

In fact, I would suggest to just link to GNU documentation or alternatively write something so clear that nobody can ask a question about it.

I might not even export the symbol in the first place, because determining the arch can be done at deploy time too given an AMI, as in my case.

roberth commented 6 years ago

It seems to me that NixOS would be better off with a 'host' and 'target' option. Conceptually:

This should probably have been represented as

They should both default to the current system.

Is it too late for this?

coretemp commented 6 years ago

I think this thread should only concern documentation efforts for specifically just this one option. Discussion of the design of the cross-compilation infrastructure should likely go elsewhere.

My current belief is that the cross-compilation infrastructure was likely designed by experienced professionals and as such their decisions are likely sound (albeit not understood by everyone).

matthewbauer commented 6 years ago

It seems to me that NixOS would be better off with a 'host' and 'target' option. Conceptually:

  • host: where it's built
  • target: the system type that the NixOS instance will be compiled for

You're going to need a 3rd one for Nixpkgs no matter what. Consider building a C compiler where you have one machine that it is built /on/, one machine where it will be /run/, and another machine that it emits code for. NixOS modules should only have to worry about pkgs.buildPlatform and pkgs.hostPlatform. I agree that the naming is bad but I think this new scheme would lead to more confusion.

@shlevy has a rename that seems like an improvement in #36217. Basically we rename them to build, run, and emit to eliminate confusion from host and target.

This should probably have been represented as

  • nixpkgs.system: target
  • nixpkgs.hostSystem: host

They should both default to the current system.

Is it too late for this?

Again, not convinced this is better. I would name them "targetSystem" and "buildSystem" to clarify the confusion from "host". "host" in Nixpkgs means where you run stuff on, not where you are building it.

roberth commented 6 years ago

Thanks @matthewbauer. I totally screwed up target and host there. Should have been

in shlevy terminology. My reason for going with for plain nixpkgs.system is that it is the only system that NixOS modules should have to care about. That it's backward compatible also helps a bit. Being backwards compatible also means we can have simple defaults that make sense, which is currently a problem. I think we should move this discussion to #36217, #49765 or a new issue, because this issue is supposed to be just about documentation.

bgamari commented 5 years ago

I would also like to point out that the example given in the documentation of this option does not actually work.

stale[bot] commented 4 years ago

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse.
  3. Ask on the #nixos channel on irc.freenode.net.
siraben commented 3 years ago

Any updates on this?

stale[bot] commented 3 years ago

I marked this as stale due to inactivity. → More info