Nix: Use callPackage pattern

[judofyr]

This makes it possible for users of the flake to override the Zig version which this is built with.

[acristoffers]

I don't think this is going to work the way you are thinking. ZLS requires the master version of the compiler to be able to compile. They are constantly bumping the compiler version, making it incompatible with anything from last week, let alone old versions.

If you checkout an older commit (and assuming the one you checked out actually compiles) it will have a flake.lock that points to the correct version of all dependencies that can compile it. There is no guarantee that an older or newer version of the compiler will be able to compile it. For example, we just had #1620, which makes that any compiler older than something that came out last Tuesday (IIRC) won't work with today's code and any compiler version that came after that won't be able to compile code from before this pull-request.

[judofyr]

This intentionally doesn't change anything about which version of Zig we're using; it just makes it possible to override the dependency. There might be multiple reasons for users to have a custom version of Zig, and it's very convenient to use callPackage to make it possible for other users to override it.

There is a separate problem with the current way it's depending on Zig: Right now if you use github:zigtools/zls as an input to a flake then it seems that the lock file in this repo isn't being reflected. As far as I understand, the locks are being done "from scratch" in the "root flake" and doesn't look at the input flakes' lock files. This actually meant that as-is I was not able to build ZLS since it was using the latest master from zig-overlay.

I was able to workaround it by setting inputs.zig-overlays.url = "github:mitchellh/zig-overlay/<SHA1-in-lock-file>", but that's not very convenient and requires manual work every time it's being updated. If we want this flake to be fully declarative on an exact version of Zig then I would advocate to do one of two:

1. Use a specific SHA1 in the url: zig-overlay.url = "github:mitchellh/zig-overlay/KNOWN-WORKING-SHA"

   zls/flake.nix

   Line 6 in 35351a6

   zig-overlay.url = "github:mitchellh/zig-overlay";

   .
2. Specify the dependency on a fixed version: zig = zig-overlay.packages.${system}.master-2023-11-22

   zls/flake.nix

   Line 27 in 35351a6

   zig = zig-overlay.packages.${system}.master;

Number 2 seems cleanest to me, but that can be done separately from this PR.

[judofyr]

My use case is that I'm using github:zigtools/zls as an input to home-manager where I'm managing my VSCode configuration:

# Rough code:

overlays = [
  (final: prev: { zls = zls.packages.${prev.system}.zls; })
];

home-manager = {
  programs.vscode = {
    userSettings = {
      "zig.zls.path" = `${pkgs.zls}/bin/zls`;
    };
  };
};

This means that ZLS is never globally installed, but that VSCode refers directly to the binary in the store.

[acristoffers]

That's weird, because I am actually using ZLS as an input for my home-manager config like this:

    zls.url = github:acristoffers/zls;
    zls.inputs.nixpkgs.follows = "nixpkgs";
    zls.inputs.flake-utils.follows = "flake-utils";
    zls.inputs.zig-overlay.follows = "zig";

and when I have compilation errors due to zig-overlay giving a newer version of the compiler that cannot be used yet I just comment the inputs overrides and it works. The root flake always respects the lockfile from the input, and you can actually see in the root flake's lockfile that the input's values are there. For example, in a flake like mine, that has its own zig-overlay input, if I don't override I get 2 entries for zig-overlay in the lock file, one for my flake and one for zls.

Note however that I'm using my fork of ZLS. That's because the main branch of this repo is quite often broken, so it is not really reliable and it is meant for development only. The problem you may be encountering is that you are trying to compile the master branch of this repo in a point in time when it is not buildable. Right now is a good example: this branch will fail to build, but mine will work fine (we are in a state of broken nix dependencies, I have a pull-request up already for that).

I'm not saying that your proposal is bad, only that it will probably not fix your problem. But I'm not a maintainer, so the decision is not mine anyway. I just wanted to point out that this repo is rather attached to the latest version of the zig compiler (and breaks often).

[judofyr]

I'm not saying that your proposal is bad, only that it will probably not fix your problem.

Which proposal are you talking about, and what problem are you talking about? I mainly want to get this PR merged first. Hopefully this change is not very controversial as it doesn't change anything other than making it possible for other users to override attributes on the zls package.

Then we can discuss other ways of improving how ZLS specifies its dependency on Zig later.

[acristoffers]

Your proposal = this PR

[judofyr]

Well, this PR solves the problem that it's not possible to override the dependencies of the ZLS package, including zig, stdenvNoCC and so forth. It's very convenient for many different purposes!

[Techatrix]

I don't see any reason why you would want to override the dependencies of the ZLS flake.

It possible to accidentally not update the Zig flake when dealing with a breaking change. AFAIK this was the case when this PR has been opened. Since then, this has only occurred once in January and has been quickly resolved by #1699.

This problem should not prompt the user of the flake to fix the issue by overriding the dependency but instead it should be fixed in ZLS directly.

If there is a different use case that I don't know about for overriding the dependencies then please open a new Issue about this.

[judofyr]

I also used this branch to be able to set zigOptimize=Debug to get better error messages when something crashed, but other than that I haven’t had much need. Maybe there’s another way of accomplishing that?

callPackage is very idiomatic Nix so I don’t really understand why to not use it though.