[OPS-958] Add template for a haskell.nix project

[zhenyavinogradov]

Adds flake template for a trivial haskell.nix project which builds the project and runs tests, with relevant CI configuration. Also has optional setup for hlint and for weeder. It should be helpful for developers who want to set up CI for their projects. There are instructions how to use this template in the readme: https://github.com/serokell/templates/tree/zhenya/ops958-haskell-nix/haskell.nix#readme.

For haskell.nix I added serokell-latest branch to our fork to track current revision used in our CI, and specified it here in flake.nix, so that new projects will automatically get the same haskell.nix version.

I also tried to add internal output to the flake.nix instead of outputs = let ... in construct, to export intermediary definitions and make it easy to explore them with nix-build or nix repl (as I discussed with @balsoft). But it seemed to make the flake harder to understand, because a huge internal attrset output would make the flake less readable, and because other outputs would need to specify internal explicitly to refer to any definition inside it. So I kept let ... in ... for the outputs, it should be more intuitive for a layperson.

[balsoft]

Otherwise looks ok, I'll try using it for a couple of our old projects just to see how easy it is

[srid]

Probably tangential / out-of-scope - but a README section documenting how to build static binaries (using haskell.nix) would be nice to know. (static binaries are currently broken in nixpkgs, btw).

[gromakovsky]

-O0 is good for builds within PRs, but if we build something that will be launched (e. g. deployed to our server) or uploaded somewhere to potentially be used by somebody (e. g. to releases), -O1 or -O2 is likely desired.

We can have a flag or something to control optimization settings. Probably it shouldn't be part of this template to keep it simple, but I'd add a comment as a reminder for people who'll need to build something with optimizations.

[zhenyavinogradov]

Yeah, there would be more concerns in this case, e.g. producing a static executable/a docker image, or setting up deployment step, and the setup may depend on the project itself, so it's hard to include this into the template. But I tried to rephrase the comment now, to make it more clear that it's only for non-release builds.

[gromakovsky]

In edna we are not using nix run, we are using nix shell instead, something like nix-shell --run 'hlint backend -j'. I don't know/remember the reasons and nix run actually looks neater to me for two reasons:

1. You can explicitly specify what you need in environment while in case of nix shell you have to pass everything to devShell = pkgs.mkShell and then each invocation of nix-shell builds all stuff.
2. nix run is just shorter than nix-shell --run (but ends up being longer due to explicit passing of all required environment).

IIRC the reason we are using nix shell is that nix run changed in newer Nix (≥ 3.0 or something like that) and thus nix shell is more future-proof? Or not? I don't know 🤷

[zhenyavinogradov]

You need to be clear that what's used in edna is nix-shell and not nix shell. nix shell is actually what nix run got renamed to in nix 3.0 NixOS/nix@5d8504b (and it might be renamed again later :pk:). And nix-shell is an older command which has a stable interface since nix 1.x.

It's somewhat a matter of taste which one to use, but to me nix run seems preferable too, for the first reason you mentioned (and also because nix-shell semantics are just weird). It's true that using nix-shell will help against the nix 3.0 renames, but we can deal with them when we switch to it anyway.

[gromakovsky]

That's more or less what I wanted to see and I think it's a good start. I reviewed it as a potential user without checking details very carefully

[gromakovsky]

It would be great to describe how one can get unstable nix and how to enable experimental features.

[zhenyavinogradov]

I agree, I suppose we should have a separate guide for this, as well as for updating dependencies.

[balsoft]

Tried it for yt-utilities, and it doesn't work pretty much at all 🤔

Even after I fixed all the issues below, it still fails at GHC configure phase for nix-tools, suggesting that our haskell.nix pin is broken.

[balsoft]

Doesn't work in pure mode

[balsoft]

Doesn't work in pure mode

[zhenyavinogradov]

Fixed it

[balsoft]

We don't actually output any of the package's components, which is strange

[zhenyavinogradov]

CI builds them through checks, it's better not to add them as packages if we don't intend to use them for anything, because there'll be an overhead in maintaining them. E.g. if we decide to add parameters to a the package definition, we would need to decide which values to use for packages, and to check that it still works

[notgne2]

I see no reason we can't just at least export executable components under packages here (and maybe add an apps definition too) as an example and it can just be deleted if not needed, otherwise this is an incomplete template

[zhenyavinogradov]

This template is mainly to help devs set up simple CI to check that their haskell projects builds and to run tests. The problem with expanding it to packaging is that there are many ways we may want to package the executables, e.g. making dynamically/statically linked binaries, making packages with extra resource files (like migration scripts), making docker images, making executables for different systems. And for many projects we don't need to package anything at all, we just need tests in CI.

So the way we would do packaging depends a lot on the project, we would have to add some explanation for the devs on how to choose a packaging method, and it's hard to do without a specific practical usecase (e.g. deploying to a server or providing downloadable packages for users). And just using a random way to do it will be confusing, I think, because people won't know what it's used for, and what to do with it when they want to make conflicting changes, like adding parameters to the derivations.

If we want to also make templates for devs for packaging/deploying packages from CI, I think it's better to do it separately, because it's an issue of a big scope by itself.

[notgne2]

There's already assumptions being made for the components which are used in the flake checks, and I think there is an objectively most generic way to export as a package, which I think should at least be included.

This is a template after all, the idea is to provide a good base which can be easily modified to fit each project, so if you don't want to export packages you can delete that, if you want to do it differently then modify it, and if the default generic methodology is appropriate for your project then you don't need to do anything at all.

I also wouldn't be against adding more than a single example, such as to catch the other use cases that you mentioned too, and disagree that having it separately would be a good idea, as it would end up being incomplete (and require developers to manually merge implementations from both templates) or end up containing all of the same code here copy-pasted. I think this is the most appropriate place to include any of those examples.

[zhenyavinogradov]

By separately I just mean in a different issue/PR, so that we can get some basic template ready for people sooner. I'm not sure if a separate template would be better for this.

I'm not sure what would a generic package look like, it would be different for different usecases. And I think it would be confusing to devs to provide an example of a package without explaining when this exact implementation is appropriate, and how to use it for actually deploying to a server or delivering to users, so it does not seem useful to add an example of a package without also adding explanation on how to do these tasks as well. Which looks like a considerate amount of work to me, so I'd really like to not do it right now.

[balsoft]

Maybe consider flake-utils.lib.eachSystem [ "x86_64-linux" ] instead of hardcoding, so that if we need to also do this for x86_64-darwin we can

[zhenyavinogradov]

I think it's better to keep the template simpler and not add eachSystem if we don't use it, it can be especially problematic when you want to add a flake output that does not depend on the system

[balsoft]

Well, we do have Haskell projects which must have x86_64-darwin outputs (e.g. bridge-web), so if we want to use this template there it will become a problem.

it can be especially problematic when you want to add a flake output that does not depend on the system

Perhaps add an empty set in which non-system-dependent outputs go? Like

outputs = {...}:
  {
    # put non-system-dependent outputs here
  } // flake-utils.lib.eachSystem ....

[zhenyavinogradov]

The problem is that with eachSystem the common layout is to use let like

outputs = {...}:
  {
    ...
  } // flake-utils.lib.eachSystem (system: let ... in { ... })

And when you want to add non-system-dependent outputs, you often want to use definitions from the let block, but they are not available because they are defined with dependency on system. And it can get confusing to refactor, to make the code look sensible. So I would avoid eachSystem in the template, and if we actually need it in some projects, we can add it there.

Also, in some cases, I'd prefer to define outputs for other systems explicitly, e.g. define packages.x86_64-linux and packages.x86_64-darwin, it should be simpler to grasp for people new to nix.

[notgne2]

I would argue that the difficulty of this is yet another good reason to iterate systems, by not doing so we're shipping an incomplete template which people will write code for which will become an awkward refactoring task once they eventually need to support different systems, if they are working with it from the beginning correctly then this will be less of an issue.

[zhenyavinogradov]

There are a lot of other reasons people might need to refactor this template: using a stack project with multiple packages, adding non-haskell packages, adding extra options for the build, adding extra checks to the CI. So I'd keep this template as simple as possible, and deal with the situations where it's not sufficient separately, because trying to cover more usecases will make it harder to understand, and will make it harder to make changes which conflict wtih a cover-all template, e.g. when they want to make changes to the build which are not compatible with a specific system.

Even for situations where people do want to add definitions for non-linux systems, eachSystems may not be the best choice sometimes, like if you only want to maintain one or two outputs for non-linux systems, but most of the outputs you only need for linux, then it's better to add an output like packages.x86_64-darwin explicitly.

[notgne2]

I still don't think that makes sense to me, even if you want to list platforms explicitly you will still want to reuse a lot of the code, so you would still need it to be wrapped in a function which consumes system regardless of how you want to implement producing the attribute set, and conveniently flake-utils supports that too as @balsoft initially mentioned.

I also don't think supporting extra platforms suffices as an edge case enough to dismiss it along with other things that will require a refactor, most other things you mentioned are dependent on the way the project is made, whereas most projects are by default in theory compatible with more than one platform, so it only makes sense that supporting a new platform explicitly should only involve a single line of code modification.

It is also something that may not be immediately obvious to developers if not provided by example, so including it is important to discourage copy pasting large chunks of Nix.

[zhenyavinogradov]

even if you want to list platforms explicitly you will still want to reuse a lot of the code, so you would still need it to be wrapped in a function which consumes system regardless of how you want to implement producing the attribute set

You don't necessarily have to wrap outputs into a function consuming system to achieve that, you can also make definitions in the let block accept system argument, and then make separate outputs for different systems which would call that with a different system. This approach might be preferable when you have a lot of outputs, but there are only one or two of them which you want to maintain for different systems.

I also don't think supporting extra platforms suffices as an edge case enough to dismiss it along with other things that will require a refactor

What I'd say is that it's a usecase we rarely run into, in ~25 haskell.nix projects we've made so far, I can only remember ligo, morley-debugger, and I believe something in stakerdao, where we run haskell.nix builds on non-linux systems. And for example, in both ligo and morley-debugger it wouldn't be enough to just add "x86_64-darwin" to the list of systems, because they both use it for packaging a vscode extension, which requires a lot of extra code by itself.

most projects are by default in theory compatible with more than one platform, so it only makes sense that supporting a new platform explicitly should only involve a single line of code modification

"Single line of code" is far from true in practice, we often had situations where adding support for a new platform would be non-trivial and required digging into haskell.nix or coming up with some workarounds. We can't rely on haskell.nix to just work when we add a new system. We could also have to use some system-specific tools in the definitions ourselves, e.g. packages which are hard to get for non-linux systems.

It is also something that may not be immediately obvious to developers if not provided by example, so including it is important to discourage copy pasting large chunks of Nix.

Still there are also many other features we often use in CI, it would need a lot of work to make a template to cover all of them. And some of them may just conflict with each other and require a lot of work to be supported at the same time. So I'd start with a template which covers just the most basic ones, and discuss everything else after we've done with that.

[zhenyavinogradov]

Even after I fixed all the issues below, it still fails at GHC configure phase for nix-tools, suggesting that our haskell.nix pin is broken.

Yeah, that's weird, I had a placeholder package I was testing this template against, and it was working before, but now it just fails to build. I just switched inputs to use our flake registry, to get a newer haskell.nix version.

[balsoft]

Alternatively, we could just drop hackage and stackage inputs alltogether and just use nix flake lock --update-input haskell.nix/hackage (which I think should work)

[zhenyavinogradov]

I think it's better to keep them as local inputs so that simple nix flake update will work, it's easy to forget about the extra flags

[balsoft]

Ok

[balsoft]

Oh, also, there probably shouldn't be a flake.lock file.

[balsoft]

I'm still not so sure about using pataq everywhere instead of specifying the packageName once and then using it as ${packageName} or something, but if you are sure it's better than whatever

[zhenyavinogradov]

It does seem nice to have to replace the package name only once, but there is another difficulty that we also need people to update the name for the test component, I think there's no really convenient way to derive it. So I've changed flake.nix to have package name specified only once, and added a notice about updating the name for the test component as well

[zhenyavinogradov]

Oh, also, there probably shouldn't be a flake.lock file.

Removed, thanks for catching it

[notgne2]

For portions we aren't leaving enabled by default, I don't know if I'm missing out on some standardized convention here but they seem to be commented backwards, such as that uncommenting the whole block would turn the comment into an expression and leave the code commented.

This whole template also seems confusing in that it only seems interested in providing checks in CI and is otherwise useless for packaging something with haskell.nix, I don't really know why this is the case, and providing a nice mix of both should be very possible, even building checks with -O0 and everything else with a more reasonable optimization level.

There's also some other concerns already raised in other review threads that I'm going to comment on.

[notgne2]

We can actually use nix flake check now on our Buildkite

[zhenyavinogradov]

I think using nix-build here is better so we don't have to worry about bugs emerging in nix flake, or the interface changing. I know nix flake check has some extra checks for the general flake validness, but I think they are not important for the CI.

[notgne2]

I guess understand wanting to avoid interface changes and stick with something more stable (although it seems like that will most likely remain the same) but I still disagree with this for 2 reasons.

I don't like how explicit the command is, as it includes both the name of the check and the name of the platform, which makes updating it alongside other changes more necessary rather than something which is likely to adapt. Also I think that checking flake validity is a good thing, while the goal of this template (which I still disagree with) seems to be just to utilize Nix for reproducible CI for Haskell code, these Nix definitions will become a component of the project once included, so CI checking that too also makes sense, plus it in theory helps ensure that the Haskell CI is implemented and functioning correctly.

[zhenyavinogradov]

I don't like how explicit the command is, as it includes both the name of the check and the name of the platform, which makes updating it alongside other changes more necessary rather than something which is likely to adapt.

I actually think this explicitness is better to people not super-familiar with nix, otherwise it may be confusing to them why we specify system in some places but not in others.

I'm not sure what kind of updates you mean that require too many changes.

Also I think that checking flake validity is a good thing

Again, I'm not sure which checks you mean and in which situations they would help us. To me it does not seem like there is anything useful for us, that would be worth sacrificing some portion of CI stability.

[zhenyavinogradov]

What we can do is try to add a separate buildkite/gitlab job just for the flake validness. Though I don't see how it would be useful, except for situations where we make a flake which also needs to be used externally, not only in CI, which we would almost never need for our haskell projects. So it seems unnecessary for the basic template we have here.

[zhenyavinogradov]

For portions we aren't leaving enabled by default, I don't know if I'm missing out on some standardized convention here but they seem to be commented backwards

People would need to uncomment whichever ## blocks they want to use. The idea is to just make them look annoying so maybe people will remove the blocks they don't use and keep the code cleaner :pled: