Skip to content

Conversation

@MaxCan-Code
Copy link
Contributor

test with nix build .#packages.instantnix

@con-f-use
Copy link
Member

con-f-use commented Mar 29, 2022

Cool, thanks! I'll test it as soon as I get back from vacation. Much appreciated.

Edit: Or not, because life 😢

Copy link
Contributor

@ShamrockLee ShamrockLee left a comment

Choose a reason for hiding this comment

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

As outputs.packages is meant to be used as pure and plain package set, I would suggest

outputs = {
  self
, nixpkgs
, flake-utils
}: let
  systems = ...;
  inherit (nixpkgs) lib;
  instantos = import ./default.nix { pkgs = nixpkgs.legacyPackages.${system}; };
in
{
  nixosModules = import ./modules;
} // flake-utils.lib.eachSystem systems (system: {
  packages = lib.filterAttrs (n: v: lib.isDerivation v) instantos;
  legacyPackages = {
    inherit instantos;
  };
});

Also, as this package is meant to be used by people on both the stable channel and the unstable channel, we probably should avoid providing the flake.lock, but have it in .gitignore instead. That's what Home Manager people do.

@con-f-use
Copy link
Member

Also, as this package is meant to be used by people on both the stable channel and the unstable channel, we probably should avoid providing the flake.lock, but have it in .gitignore instead. That's what Home Manager people do.

The point of providing the lock file is to have a version of the dependencies that is tested against and known to work. So I think we should include it.

@MaxCan-Code
Copy link
Contributor Author

MaxCan-Code commented Apr 24, 2022

https://github.com/NixOS/nixpkgs/blob/nixos-unstable/pkgs/top-level/aliases.nix#L305-L309

conky
dunst
disper
firefox
libnotify

Without the deleted lock file nix build fails on unstable since disper was removed. This would also be the case for the NixOS 22.05 release

@ShamrockLee
Copy link
Contributor

I see. Maybe we should add it back. Users can still override it with .follows inside their flake.nix.

@ShamrockLee
Copy link
Contributor

outputs.nixosModules = import ./modules.nix;

is only valid when modules.nix is a set of name-value pairs of the modules, which is the case in the NUR README and these two NURs:
https://github.com/cwyc/nur-packages/blob/master/modules/default.nix
https://github.com/dawidsowa/nur-packages/blob/master/modules/default.nix

I think we should either change modules/default.nix to

{
  instantlock = ./instantlock.nix;
  instantwm = ./insntantwm.nix;
}

or change the corresponding flake output to

  nixosModules = (import ./modules.nix).modules;

@ShamrockLee
Copy link
Contributor

Thank you for all your work!

The point of providing the lock file is to have a version of the dependencies that is tested against and known to work. So I think we should include it.

Without the deleted lock file nix build fails on unstable since disper was removed. This would also be the case for the NixOS 22.05 release

Should we specify the "officially supported" channel (Nixpkgs branch) then?

It could be implemented by specifying the flake inputs:

{
  # ...

  inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-21.11";

  outputs = ... ;
}

After that, just run

nix flake lock

and nix will modify flake.lock according to the flake.nix content.

@con-f-use
Copy link
Member

Just for the record, I'm very happy that you two nix-savy people are on board here. ❤️

@MaxCan-Code
Copy link
Contributor Author

Happy to help!

Copy link
Contributor

@ShamrockLee ShamrockLee left a comment

Choose a reason for hiding this comment

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

One last changes:
Otherwise, LGTM.

@ShamrockLee
Copy link
Contributor

Thanks a lot for your time and effort!

AFAIK, most people would squash those PR modifications into one commit to make the history cleaner.

To squash the extra 7 commits into the first one, you can run the following commands:

git checkout flake
# Base----A----B----C----D----E----F----G----H
# ^--dev                                     ^--HEAD
git reset --soft flake~7
# Base----A----Staged changes (contains changes in B, ..., H)
# ^--dev  ^--HEAD
git commit --amend --no-edit
# Base----A'
# ^--dev  ^--HEAD
#         ^ (contains changes in A, ..., H)

and then force-push to your fork repository.

@MaxCan-Code
Copy link
Contributor Author

Done. Thanks for all the tips!

@MaxCan-Code MaxCan-Code requested a review from con-f-use April 27, 2022 14:47
Based on https://github.com/nix-community/nur-packages-template
Could also use https://github.com/numtide/flake-utils#eachsystem---system---system---attrs
Locking `inputs.nixpkgs` at nixos-21.11 should remove the need for the lock file

Co-authored-by: Shamrock Lee <44064051+ShamrockLee@users.noreply.github.com>
@con-f-use con-f-use merged commit 7d130d2 into instantOS:dev Aug 9, 2022
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.

3 participants