Improved flake.nix - Githubissues

nix-community / napalm

Support for building npm packages in Nix and lightweight npm registry [maintainer=?]

MIT License

104 stars 17 forks source link

Improved flake.nix #37

Closed Mazurel closed 3 years ago

Mazurel commented 3 years ago

This PR improves flake.nix ~and fixes #36~ .

~In order to make repository fully compatible with flake.nix it was required to stop using callCabal2nix and to use cabal2nix to manually create napalm-registry via updateDefault.sh script.~

Flake additions:

Packages that can be built via nix build
Access to development shell via nix develop
Overlay that can be used in other flakes
~Checks that can be run to ensure that everything builds correctly~
Specified nixpkgs version
Simple template for easier Napalm usage with flakes

jtojnar commented 3 years ago

Could you please split the individual features into a separate commits?

I am also not very fond of generated files being committed. Is it required for anything other than flake check? We could always just ignore the failure until it is fixed on the Nix side.

Mazurel commented 3 years ago

Could you please split the individual features into a separate commits?

I will try to

I am also not very fond of generated files being committed. Is it required for anything other than flake check? We could always just ignore the failure until it is fixed on the Nix side.

I have investigated the problem even more and realized that ghc (on nix ?) is not being supported on these systems, so I guess the error message was about that. I will revert this change and force push. This PR will be only about improving flake and not fixing it.

Mazurel commented 3 years ago

Forced pushed.

I was not sure how I should split the commits, sorry for that. The first one looks big, but half of it are test projects that were copied over for the template.

jtojnar commented 3 years ago

Simple way of splitting commits can be done by editing https://github.com/nix-community/napalm/pull/37.patch in a text editor and then use git apply to apply the modified patch.

It is not clear to me why you are pointing the nixpkgs input to the stable branch.
And napalm works at least on aarch64-linux so removing the systems is probably not a good idea.

Mazurel commented 3 years ago

It is not clear to me why you are pointing the nixpkgs input to the stable branch.

Stable branch is more stable I guess. There is no reason to point to an unstable branch when there is no need to from what I know. Although, based on this point

And napalm works at least on aarch64-linux so removing the systems is probably not a good idea.

It seems like maybe unstable branch have improved ghc compiler compatibility, because based on my tests there were some problems with that. I will look into this.

Simple way of splitting commits can be done by editing https://github.com/nix-community/napalm/pull/37.patch in a text editor and then use git apply to apply the modified patch.

Thanks for the tip, I didn't know about it, although my point was that I have no clue how to split this commit so that it would make sense, although I will look into it again today.

Mazurel commented 3 years ago

I have changed inputs to use master branch and indeed I succeeded with building aarch64 and checking i686. Checks still fail for me when x86_64-darwin is being used, although I think it may be nix flake check or nixpkgs bug, I guess that for now it is not that simple to fix it.

Mic92 commented 3 years ago

I also think we should switch from circle ci to github actions now that the repo got moved.

Mazurel commented 3 years ago

I was considering switching it to github actions with nix flake check, although it seems like it does not work correctly.

We need to wait for nix flake check to be fixed in nixpkgs, so for now I guess that circle ci needs to be fixed in another PR related specifically to that problem (I can look into that when this PR will be merged).

Mazurel commented 3 years ago

Closed it as #38 is a new PR that contains these changes.