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

Refactor flake.nix to remove flake-utils #177

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 0 additions & 44 deletions flake.lock

This file was deleted.

42 changes: 24 additions & 18 deletions flake.nix
Original file line number Diff line number Diff line change
@@ -1,25 +1,31 @@
{

inputs.flake-utils.url = "github:numtide/flake-utils";

outputs =
{ flake-utils, self }:
flake-utils.lib.eachDefaultSystem (
system:
let
result = import ./default.nix { inherit system; };
in
{
packages = result.packages // {
default = result;
};
{ self, nixpkgs }:
Comment on lines -6 to +3
Copy link
Member

Choose a reason for hiding this comment

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

You removed flake-utils, but added nixpkgs (and that without an inputs entry, which then runs into NixOS/nix#7422). And if you don't commit the lockfile as here, we'll run into further non-reproducibility. Alternatively if you do commit the lockfile, we'll have two Nixpkgs versions that need to be fetched for the flake.

So, I don't think we should do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How is the current version using the builtins dependencies of flake-utils being more reproducible than using the lib from Nixpkgs? Seems most folks are used to passing in inputs.nixpkgs.follows thru their flakes. I suppose you could get lib from npins, but really after thinking about it, might it just be simpler to manually write out x86_64-linux, et al. for system.exposedFlakes and remove all dependencies then? It’s more repetitious “code”, but has zero dependencies. I would be willing to write it out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IIRC, 32-bit Linux is one of the supported systems in Hydra but is missing from flake-utils’s defaultSystems.

Copy link
Member

Choose a reason for hiding this comment

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

How is the current version using the builtins dependencies of flake-utils being more reproducible than using the lib from Nixpkgs?

Just because in the current PR at least, Nixpkgs isn't pinned, while flake-utils is. Without committing the lockfile to pin it, people will get different results at different points in time.

Seems most folks are used to passing in inputs.nixpkgs.follows thru their flakes

That doesn't actually work, the npins-pinned Nixpkgs can't be overridden like that (see #166 for what works instead)

might it just be simpler to manually write out x86_64-linux, et al. for system.exposedFlakes and remove all dependencies then? It’s more repetitious “code”, but has zero dependencies. I would be willing to write it out.

Actually one thing that flake-utils allows you is to use the https://github.com/nix-systems/nix-systems pattern, which allows consumers to work around the problem of Flakes requiring a hardcoded list of systems. This wouldn't be possible anymore if we hardcode the systems locally.

Copy link
Contributor Author

@toastal toastal Apr 2, 2024

Choose a reason for hiding this comment

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

Okay. Fair assessments as I misunderstand the environment. Thank you for explaining as I don’t see many places with a flake.nix & not using Nix to pin their Nixpkgs as well (which is why it feels like Nixpkgs is always a given).

let
forAllSystems = nixpkgs.lib.genAttrs nixpkgs.lib.systems.flakeExposed;

apps.default = {
result = forAllSystems (system: import ./default.nix { inherit system; });
in
{
packages = forAllSystems (
system:
let
packages = result.${system}.packages;
in
packages // { default = packages.nixfmt; }
);

apps = forAllSystems (system: {
default = {
type = "app";
program = "${result}/bin/nixfmt";
program = nixpkgs.lib.getExe self.packages.${system}.default;
};
});

checks = forAllSystems (system: result.${system}.checks);

checks = result.checks;
}
);
devShells = forAllSystems (system: {
default = result.${system}.shell;
});
Comment on lines +27 to +29
Copy link
Member

Choose a reason for hiding this comment

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

What's the motivation for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using nix develop over nix-shell for its caching, or being able to add the shell to another dev environment if one really wanted. The information to make it exists, & it’s little to expose it.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense, sounds fine to have this :)

Personally I'm using https://github.com/nix-community/nix-direnv, which also caches the shell for speed

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 the flake-utils part is staying, I could just put in this commit & axe the other.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me!

};
}