-
Notifications
You must be signed in to change notification settings - Fork 4
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
[OPS-958] Add template for a haskell.nix project #2
Changes from 2 commits
1e3dd8b
9a2b3c7
5bcf179
ef8ee0d
2edb217
e9dd1c0
b390ed9
a3d19ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
steps: | ||
|
||
# build haskell components | ||
- label: build | ||
commands: | ||
- nix-build -A checks.x86_64-linux.build-all | ||
|
||
# don't run tests until all components are built | ||
- wait | ||
|
||
# run the tests | ||
- label: test | ||
commands: | ||
- nix-build -A checks.x86_64-linux.test | ||
|
||
# run hlint | ||
##- label: hlint | ||
## commands: | ||
## - nix run -f. legacyPackages.x86_64-linux.haskellPackages.hlint | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In
IIRC the reason we are using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need to be clear that what's used in edna is It's somewhat a matter of taste which one to use, but to me |
||
## -c hlint -j --hint .hlint.yaml */ | ||
|
||
# run weeder | ||
##- label: weeder | ||
## commands: | ||
## - nix-build -A weeder-script | ||
## # weeder needs .cabal file: | ||
## - nix run -f. legacyPackages.x86_64-linux.haskellPackages.hpack -c hpack | ||
## - ./result |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
default: | ||
# specify nix tag to select runner | ||
tags: [nix] | ||
|
||
stages: | ||
- build | ||
- test | ||
|
||
# build haskell components | ||
build-all: | ||
stage: build | ||
script: | ||
- nix-build -A checks.x86_64-linux.build-all | ||
|
||
# run the tests | ||
test: | ||
stage: test | ||
script: | ||
- nix-build -A checks.x86_64-linux.test | ||
|
||
# run hlint | ||
##hlint: | ||
## stage: test | ||
## script: | ||
## - nix run -f. legacyPackages.x86_64-linux.haskellPackages.hlint | ||
## -c hlint --hint .hlint.yaml */ | ||
|
||
# run weeder | ||
##weeder: | ||
## stage: test | ||
## script: | ||
## - nix-build -A weeder-script | ||
## # weeder needs .cabal file: | ||
## - nix run -f. legacyPackages.x86_64-linux.haskellPackages.hpack -c hpack | ||
## - ./result |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
Template for haskell.nix configuration for Buildkite or Gitlab CI | ||
|
||
## How to use this template | ||
|
||
1. Copy `flake.nix` file to you repository, it contains all nix dependencies and definitions. Or you can use `nix flake` to copy the whole template: `nix flake init -t github:serokell/templates#haskell-nix`. | ||
|
||
2. Copy `default.nix` and `shell.nix` files which provide compatibility with non-flake nix interface. | ||
|
||
3. Copy pipeline configuration: `.gitlab-ci.yml` for Gitlab, or `.buildkite/pipeline.yml` for Buildkite. | ||
|
||
4. Replace `pataq-package` in `flake.nix` with your haskell package name (usually specified in `package.yaml`). | ||
|
||
5. If you want to run weeder or hlint in your CI, uncomment related lines in `flake.nix` and in the pipeline configuration. If you don't need weeder or hlint, remove those lines. | ||
|
||
6. Get unstable nix with `nix-command flakes` experimental features enabled, and run `nix flake update` to generate `flake.lock` file. This file pins latest versions of the nix dependencies. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be great to describe how one can get unstable nix and how to enable experimental features. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree, I suppose we should have a separate guide for this, as well as for updating dependencies. |
||
|
||
7. Enjoy working CI. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
(import | ||
( | ||
let | ||
lock = builtins.fromJSON (builtins.readFile ./flake.lock); | ||
in | ||
fetchTarball { | ||
url = "https://github.com/edolstra/flake-compat/archive/${lock.nodes.flake-compat.locked.rev}.tar.gz"; | ||
sha256 = lock.nodes.flake-compat.locked.narHash; | ||
} | ||
) | ||
{ | ||
src = ./.; | ||
}).defaultNix |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,116 @@ | ||
{ | ||
inputs = { | ||
flake-compat = { | ||
url = "github:edolstra/flake-compat"; | ||
flake = false; | ||
}; | ||
nixpkgs.url = "github:serokell/nixpkgs"; | ||
haskell-nix.url = "github:serokell/haskell.nix/serokell-latest"; | ||
hackage-nix = { | ||
url = "github:input-output-hk/hackage.nix"; | ||
flake = false; | ||
}; | ||
stackage-nix = { | ||
url = "github:input-output-hk/stackage.nix"; | ||
flake = false; | ||
}; | ||
|
||
# if you want to run weeder: | ||
##haskell-nix-weeder = { | ||
## url = "github:serokell/haskell-nix-weeder"; | ||
## flake = false; | ||
##}; | ||
}; | ||
|
||
outputs = { self, nixpkgs, haskell-nix, hackage-nix, stackage-nix, ... }: | ||
|
||
# for weeder: | ||
##outputs = { self, nixpkgs, haskell-nix, hackage-nix, stackage-nix, haskell-nix-weeder, ... }: | ||
|
||
let | ||
haskellNix = import haskell-nix { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't work in pure mode |
||
sourcesOverride = { hackage = hackage-nix; stackage = stackage-nix; }; | ||
}; | ||
|
||
pkgs = import nixpkgs haskellNix.nixpkgsArgs; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't work in pure mode There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed it |
||
lib = pkgs.lib; | ||
|
||
# invoke haskell.nix | ||
hs-pkgs = pkgs.haskell-nix.stackProject { | ||
src = pkgs.haskell-nix.haskellLib.cleanGit { | ||
name = "pataq-package"; | ||
balsoft marked this conversation as resolved.
Show resolved
Hide resolved
|
||
src = ./.; | ||
}; | ||
|
||
# haskell.nix configuration | ||
modules = [{ | ||
packages.pataq-package = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm still not so sure about using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
ghcOptions = [ | ||
# fail on warnings | ||
"-Werror" | ||
# disable optimisations to speed up builds | ||
"-O0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
# for weeder: produce *.dump-hi files | ||
##"-ddump-to-file" "-ddump-hi" | ||
]; | ||
|
||
# for weeder: collect all *.dump-hi files | ||
##postInstall = weeder-hacks.collect-dump-hi-files; | ||
}; | ||
|
||
}]; | ||
}; | ||
|
||
hs-pkg = hs-pkgs.pataq-package; | ||
|
||
# returns the list of all components for a package | ||
get-package-components = pkg: | ||
# library | ||
lib.optional (pkg ? library) pkg.library | ||
# haddock | ||
++ lib.optional (pkg ? library) pkg.library.haddock | ||
# exes, tests and benchmarks | ||
++ lib.attrValues pkg.exes | ||
++ lib.attrValues pkg.tests | ||
++ lib.attrValues pkg.benchmarks; | ||
|
||
# all components for the current haskell package | ||
all-components = get-package-components hs-pkg.components; | ||
|
||
# for weeder: | ||
##weeder-hacks = import haskell-nix-weeder { inherit pkgs; }; | ||
|
||
# nixpkgs has weeder 2, but we use weeder 1 | ||
##weeder-legacy = pkgs.haskellPackages.callHackageDirect { | ||
## pkg = "weeder"; | ||
## ver = "1.0.9"; | ||
## sha256 = "0gfvhw7n8g2274k74g8gnv1y19alr1yig618capiyaix6i9wnmpa"; | ||
##} {}; | ||
|
||
# a derivation which generates a script for running weeder | ||
##weeder-script = weeder-hacks.weeder-script { | ||
## weeder = weeder-legacy; | ||
## hs-pkgs = hs-pkgs; | ||
## local-packages = [ | ||
## { name = "pataq-package"; subdirectory = "."; } | ||
## ]; | ||
##}; | ||
|
||
in { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't actually output any of the package's components, which is strange There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CI builds them through There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see no reason we can't just at least export executable components under There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
# nixpkgs revision pinned by this flake | ||
legacyPackages.x86_64-linux = pkgs; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe consider There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's better to keep the template simpler and not add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, we do have Haskell projects which must have
Perhaps add an empty set in which non-system-dependent outputs go? Like
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is that with
And when you want to add non-system-dependent outputs, you often want to use definitions from the Also, in some cases, I'd prefer to define outputs for other systems explicitly, e.g. define There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
You don't necessarily have to wrap
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.
"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.
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. |
||
|
||
# derivations that we can run from CI | ||
checks.x86_64-linux = { | ||
# builds all haskell components | ||
build-all = all-components; | ||
|
||
# runs the test | ||
test = hs-pkg.checks.pataq-test; | ||
}; | ||
|
||
# script for running weeder | ||
##weeder-script = weeder-script; | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
(import | ||
( | ||
let | ||
lock = builtins.fromJSON (builtins.readFile ./flake.lock); | ||
in | ||
fetchTarball { | ||
url = "https://github.com/edolstra/flake-compat/archive/${lock.nodes.flake-compat.locked.rev}.tar.gz"; | ||
sha256 = lock.nodes.flake-compat.locked.narHash; | ||
} | ||
) | ||
{ | ||
src = ./.; | ||
}).shellNix.default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can actually use
nix flake check
now on our BuildkiteThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think using
nix-build
here is better so we don't have to worry about bugs emerging innix flake
, or the interface changing. I knownix flake check
has some extra checks for the general flake validness, but I think they are not important for the CI.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.