Make sure you have set up the prerequisities.
You can get an environment for developing the entire project using a nix shell.
To enter the shell run the command nix-shell.
If you get a segfault, try GC_DONT_GC=1 nix-shell instead.
This includes a variety of useful tools:
-
The right version of GHC with all the external Haskell dependencies in its package database.
-
cabal-install -
stylish-haskell -
haskell-language-server -
… and more
Have a look in shell.nix to see what’s there.
We rely heavily on this approach to ensure that everyone has consistent versions of the tools that we use. Please do make use of this, since problems due to mismatched versions of tools are particularly annoying to fix!
|
Note
|
You may want to consider using lorri as a convenient alternative to running nix-shell directly.
|
The shell comes with some tools for fixing various simple problems that the CI will complain about. Specifically:
-
fix-stylish-haskellwill re-format all the Haskell sources correctly. -
fix-cabal-fmtwill re-format all the Cabal sources correctly. -
fix-png-optimizationwill optimize all PNGs in the repository.
If you’re not using nix-shell all the time and you want to run one of these, you can use nix-shell --run.
The nix-shell environment has the correct GHC with all the external Haskell dependencies of the project.
From here you can build the project packages directly with cabal.
|
Note
|
You may need to run cabal update so that cabal knows about the index state we have pinned.
|
For stack, you may want to use the Nix integration and point it at shell.nix, which will make sure you at least get the right GHC.
|
Warning
|
You can also use
|
If you launch nix-shell with the enableHaskellProfiling argument set to true, you will get a shell where all the dependencies have been built with profiling.
Like this: nix-shell --arg enableHaskellProfiling true.
|
Warning
|
The shell with profiling dependencies is not currently cached, so this will result in you rebuilding all of our dependencies with profiling on your machine. This will take a long time. |
Once you have a shell with profiling libraries for our dependencies, add profiling: true to cabal.project.local, which will tell cabal that you want profiling (in particular, that will cause it to build our libraries with profiling).
Alternatively, you can pass the --enable-profiling option to cabal on an ad-hoc basis, but adding the option to cabal.project.local will make it apply to everything, which is probably what you want when you’re doing profiling work.
At this point you need to configure which cost centres you want GHC to insert.
The GHC user guide explains this very well.
A typical way of doing this is to add -fprof-auto to either the ghc-options in the .cabal file for the project, or in an OPTIONS_GHC pragma in the module you care about.
|
Warning
|
Do not set the |
Then you can use the RTS -p option to dump a profile e.g. cabal run plc … — +RTS -p.
There are various tools for visualizing the resulting profile, e.g. https://hackage.haskell.org/package/ghc-prof-flamegraph.
The nix-shell environment has a haskell-language-server binary for the right version of GHC.
|
Important
|
this binary is called haskell-language-server, rather than haskell-language-server-wrapper, which is what some of the editor integrations expect.
|
We don’t have a hie.yaml, the implicit cradle support in HLS seems to work fine these days.
Updating package dependencies from Hackage should work like normal in a
Haskell project. The most important thing to note is that we pin the
index-state of the Hackage package index in cabal.project. This
means that cabal will always see Hackage “as if” it was that time, ensuring
reproducibility. But it also means that if you need a package version that
was released after that time, you need to bump the index-state (and
to run cabal update locally). Please also note that index-state is a
property of your working environment (the cabal project) not of the
packages. This means that downstream consumers have no idea of your
index-state setting and you need to make sure your packages work
correctly also without index-state.
Because of how we use Nix to manage our Haskell build, whenever you do this
you will also need to pull in the Nix equivalent of the newer
index-state. You can do this by running nix flake lock --update-input
hackage-nix.
Many Cardano packages are not on Hackage and are instead in the Cardano
package repository
https://github.com/input-output-hk/cardano-haskell-packages, see the
README for (lots) more information.
Getting new packages from there works much like getting them from Hackage.
The differences are that it can have an independent index-state, and
that there is a different Nix command you need to run afterwards: nix flake
lock --update-input CHaP.
Sometimes we need to use an unreleased version of one of our dependencies, either to fix an issue in a package that is not under our control, or to experiment with a pre-release version of one of our own packages.
You can use a source-repository-package stanza to pull in the
unreleased version.
Please note that consumers of our packages will not pull these unreleased
versions when compiling our packages so consider using source-repository-package
only as a temporary solution.
For packages that we do not control, we can end up in a situation where we have a fork that looks like it will be long-lived or permanent (e.g. the maintainer is unresponsive, or the change has been merged but not released).
In that case, release a patched version to the Cardano package repository
https://github.com/input-output-hk/cardano-haskell-packages, which
allows us to remove the source-repository-package stanza.
Sometimes you may want to make a change that spans both plutus-apps and some of its dependencies (most commonly, the packages in plutus).
The obvious workflow is to make changes in the plutus repository, update the pin in plutus-apps to point to the new commit, test, repeat.
But this is very tedious and it’s much nicer to work with a local checkout where cabal can incrementally rebuild the whole thing.
Here’s an example of doing this for some packages in plutus.
You may need to adapt this example depending on what exactly you’re trying to do.
First of all, we add some lines to cabal.project.local (we do this to avoid modifying cabal.project, which is tracked by git, as much as possible).
-- Add the packages that we want to pull in locally as "local" packages
-- (assuming a `plutus-core` checkout in `../plutus`)
packages:
../plutus/plutus-core
../plutus/plutus-tx
../plutus/plutus-tx-plugin
../plutus/plutus-ledger-api
-- Sometimes cabal may get upset about dependencies, you can make its life easier
-- by turning off unnecessary features for the altered packages.
package plutus-core
benchmarks: false
tests: false
package plutus-tx
benchmarks: false
tests: false
package plutus-tx-plugin
benchmarks: false
tests: falseThen we unfortunately do need to modify the main cabal.project, to comment out the packages that we are replacing with local ones.
source-repository-package
...
subdir:
--plutus-core
--plutus-ledger-api
--plutus-tx
--plutus-tx-plugin
word-array
prettyprinter-configurable
stubs/plutus-ghc-stubAfter this, a cabal build should build with the local checkouts of plutus for the packages you specified.
We are a relatively large team working on sometimes quite abstruse problems. As such, it’s important that future people who work on the project know how things work, and just as importantly, why. These future people may even be yourself - we forget things very quickly!
When writing, try to put yourself in the position of someone coming to this code for the first time. What do they need to do to understand it and do their job? Write it down!
Code review is a good lens for this: if you have to explain something to a reviewer, then it is probably not clear in the code and should have a note.
This applies both to the code itself (structure, naming, etc.) and also to comments. How to write useful comments is a large topic which we don’t attempt to cover here, but Antirez is good. If in doubt: write more!
One special kind of comment is worth drawing attention to. We adopt a convention (stolen from GHC) of writing fairly substantial notes in our code with a particular structure. These correspond to what Antirez calls "design comments", with some conventions about cross-referencing them.
The structure is:
-
The Note should be in a multiline comment (i.e.
{- -}) -
The first line of the Note should be
Note [Name of note] -
Refer to a Note from where it is relevant with a comment saying
See Note [Name of note]
For example:
{- Note [How to write a note]
A note should look a bit like this.
Go wild, write lots of stuff!
Here's a small diagram:
A ----> B >> C
And of course, you should see Note [Another note].
-}
Notes are a great place to put substantial discussion that you need to refer to from multiple places. For example, if you used an encoding trick to fit more data into an output format, you could write a Note describing the trick (and justifying its usage!), and then refer to it from the encoder and the decoder.
If a new feature or code refactor requires you to make an "architecturally significant" decision, then you should probably write an ADR.
See the readthedocs page for more details.
We use stylish-haskell for Haskell code formatting, and cabal-fmt for cabal files.
They are run automatically as pre-commit hooks, but CI will run them again and expect that to be a no-op, so if you somehow don’t apply them your PR will not go green.
To run stylish-haskell or cabal-fmt manually over your tree, type fix-stylish-haskell or fix-cabal-fmt respectively.
They are provided by the nix-shell environment.
The CI builds Haskell code with -Werror, so will fail if there are any compiler warnings.
So fix your own warnings!
If the warnings are stupid, we can turn them off, e.g. sometimes it makes sense to add -Wno-orphans to a file where we know it’s safe.
Please make informative commit messages! It makes it much easier to work out why things are the way they are when you’re debugging things later.
A commit message is communication, so as usual, put yourself in the position of the reader: what does a reviewer, or someone reading the commit message later need to do their job? Write it down! It is even better to include this information in the code itself, but sometimes it doesn’t belong there (e.g. ticket info).
Also, include any relevant meta-information, such as ticket numbers. If a commit completely addresses a ticket, you can put that in the headline if you want, but it’s fine to just put it in the body.
There is plenty to say on this topic, but broadly the guidelines in this post are good.
A pull request is a change to the codebase, but it is also an artifact which goes through a change acceptance process. There are a bunch of things which we can do to make this process smooth which may have nothing to do with the code itself.
The key bottleneck in getting a PR merged is code review. Code review is great (see below), but it can slow you down if you don’t take the time to make it easy.
The amount of time it’s worth spending doing this is probably much more than you think.
There are two protected branches, main and next-node.
PRs should generally target the main branch, unless the change is only applicable to next-node.
When developing a non-trivial new feature, usually the best way to get the code reviewed is to break the implementation down to a chain of small diffs, each representing a meaningful, logical and reviewable step. Unfortunately GitHub doesn’t have good support for this. You basically have three options:
-
Open the first PR against
main, the second PR against the first PR’s branch, and so on. Merging a stack of PRs created this way intomaincan be non-trivial. -
Wait until one PR is merged before opening the next PR.
-
Use a single PR for the whole feature that contains multiple small commits. The problem is that Github doesn’t support approving, rejecting or merging individual commits in a PR. You can look at each individual commit, but it’s not necessarily useful or even appropriate - many PRs have quite messy commits, and commits are sometimes overwritten via force push.
The first two options are often referred to as ["trunk-based development"](https://trunkbaseddevelopment.com/), while the third "long-lived feature branches". There is no single best option for all cases, although in general we encourage adopting trunk-based development styles. Long-lived feature branches with too many commits are harmful because
-
they are difficult to review - the PR can be quite large, and it is hard to review it incrementally;
-
it can be difficult to resolve merge conflicts;
-
they make it more likely that other people need to depend on your unmerged changes.
It is fine to have partially implemented features or not well-tested features in main.
You can simply not turn them on until they are ready, or guard them with conditinal flags.
But this is not a hard rule and should be determined on a case-by-case basis. Sometimes for a small or medium-sized piece of work, you may not want to break it into multiple PRs, and wait till each PR is merged before creating the next one. You’d rather put all your code out quickly in a single PR for review. And that’s fine. Or maybe it’s a piece of performance improvement work, and you don’t know whether or not it actually improves the performance, until you finish implementing and testing the whole thing.
Whichever option you choose, please keep each of your PR to a single topic. Do not mix business logic with such things as reformatting and refactoring in a single PR.
A pull request is communication, so as usual, put yourself in the position of the reader: what does your audience (the reviewer) need to know to do their job? This information is easy for you to access, but hard for them to figure out, so write it down!
However, better to put information in the code, commit messages or changelog if possible: these persist but PR descriptions do not. It’s okay to repeat information from such places, or simply to point to it. For one-commit PRs, Github will automatically populate the PR description with the commit message, so if you’ve written a good commit message you’re done! Sometimes there is "change-related" information that doesn’t belong in a commit message but is useful ("Kris I think this will fix the issue you had yesterday").
-
Review the diff of your own PR at the last minute before hitting "create". It’s amazing how many obvious things you spot here, and it stops the reviewer having to point them all out.
-
It’s fine to make WIP PRs if you just want to show your code to someone else or have the CI check it. Use the Github "draft" feature for this.
Force pushing to main or next-node is never allowed.
There is no exception to this rule.
Rebasing and force pushing to other branches you own is fine, even when you have an open PR on the branch. Indeed, if you need to update your branch with changes from main, rebasing is typically better than merging.
Some projects do not allow force pushing to any remote branch. This is not a popular policy and we do not adopt it, because
-
This means you must only ever use the "merge commit" merge method (or occasionally, fast forward merge, which GitHub doesn’t support).
-
This means you aren’t even allowed to clean up commits in your own PR, and must eventually merge everything into
main. It discourages people from pushing commits frequently when developing. We should instead encourage cleaning up commits in PRs, at least before merging. -
The argument that this will cause massive pain for those who merge other people’s PR branch into their branch is questionable. This should be rare to begin with, if we adopt trunk-based development in general, instead of long-lived feature branches. And even if you do need to depend on other people’s unmerged work, you can instead rebase your branch on theirs, and if their branch changes, just rebase again.
Rebasing and force pushing can be used to your advantage, for example:
-
Add low-effort or WIP commits to fix review comments, and then squash them away before merging the PR.
-
If you have already had a PR review, don’t rebase away the old commits until the PR is ready to merge, so that the reviewer only has to look at the "new" commits.
-
Rewrite the commits to make the story clearer where possible.
It is advisable to always prefer git push --force-with-lease instead of git push --force to ensure that no work gets accidentally deleted.
All pull-requests should be approved by at least one other person. We don’t enforce this, though: a PR fixing a typo is fine to self-merge, beyond that use your judgement.
As an author, code review is an opportunity for you to get feedback from clear eyes. As a reviewer, code review is an opportunity for you to help your colleagues and learn about what they are doing. Make the best use of it you can!
-
Pick the right reviewer(s). If you don’t know who to pick, ask!
-
Respect your reviewers' time. Their time is as valuable as yours, and it’s typically more efficient for you to spend time explaining or clarifying something in advance than for them to puzzle it out or pose a question.
-
If someone had to ask about your code, it wasn’t clear enough so change it or add a comment.
Read this blog post for more good tips: https://mtlynch.io/code-review-love/
-
Respond to review requests as quickly as you can. If you can’t review it all, say what you can and come back to it. Waiting for review is often a blocker for other people, so prioritize it.
-
If you don’t understand something, ask. You are as clever as any person who will read this in the future, if it confuses you it’s confusing.
-
Do spend the time to understand the code. This will help you make more useful comments, help you review future changes more easily, and help you if you ever need to work on it yourself.
-
More reviewing is usually helpful. If you think a PR is interesting, you can review it even if nobody asked you to, you will probably have things to contribute and you’ll learn something.
Read these blog posts for more good tips: - https://mtlynch.io/human-code-reviews-1/ - https://mtlynch.io/human-code-reviews-2/
This repo allows two merge methods: squash and merge, and rebase and merge. Use the one you deem appropriate. As said before, sometimes people use a single PR with multiple commits for their work; other times they create multiple small PRs. The best merge method is different for different cases.
The "rebase and merge" method should be used with caution.
If you use this method, your PR must have a clean commit history: every commit should have a meaningful message, and should be buildable.
You don’t want to have commits like "fix a typo", "this may work" or "wip, done for the day" in main with a linear history.
And if some of these commits are non-buildable, it can create problems for "git bisect".
Merging a PR can break main, if the PR branch has diverged from main, even if CI on the PR is green.
This happens because the PRs conflict in a way that isn’t obvious to git, e.g. one adds a usage of a function and the other removes that function.
The problems with a broken main include inconveniencing other developers, and causing problems for "git bisect".
There are ways to guarantee main never breaks, such as GitHub’s [merge queue](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue).
We don’t use the merge queue because
-
A broken
mainhas historically been quite infrequent. -
The merge queue increases the time it takes to merge a PR, which causes productivity loss if you are waiting to create the next PR after merging the current one (which happens often).
However, if your PR branch has diverged too much from main, it is recommended that you rebase or merge main into the PR branch before merging.
And whenever you notice a broken main, please fix it ASAP.
The same holds for next-node if your PR targets next-node.
Each cabal package has its own changelog. We use scriv — a changelog management tool — to avoid conflicts.
The broad heuristic is to put yourself in the position of the consumer of the piece of software in question and ask if you would want to know about this change. If the answer is yes, then write a quick changelog entry.
It’s important to reflect the changes that affect the API and break backwards compatibility.
The basic idea is that you write a changelog "fragment" in the changelog.d directory.
When we do a release, these will be collected into the main CHANGELOG.md.
Usually we don’t edit CHANGELOG.md directly.
You can make a changelog fragment using scriv create in the package directory, but you can also just create the fragment directly with an editor.
A fragment is a markdown file beginning with a header giving the category of change.
We have a few sources of CI checks at the moment:
-
Hydra
-
ReadTheDocs
-
Github Actions
-
Buildkite
The CI will report statuses on your PRs with links to the logs in case of failure. Pull requests cannot be merged without at least the Hydra CI check being green.
|
Note
|
This isn’t strictly true: repository admins (notably Michael) can force-merge PRs without the checks being green If you really need this, ask. |
Hydra is the "standard" CI builder for Nix-based projects. It builds everything in the project, including all the tests, documentation, etc.
Hydra builds jobs based on release.nix, although currently this imports a lot of its jobs from ci.nix (was used for Hercules, may be used again in future).
Hydra should report a failed status even if release.nix fails to evaluate.
Hydra can be a bit flaky, unfortunately: - If evaluation fails saying "out of memory" or "unexpected EOF reading line", then this is likely a transient failure. These will be automatically retried, but if you’re in a hurry Michael has permissions to force a new evaluation. - If a build fails spuriously, this is a problem: please report it to whoever is responsible for that build and we should try and iron it out. Nondeterministic failures are very annoying. Michael also has permissions to restart failed builds.
The documentation site is built on ReadTheDocs. It will build a preview for each PR which is linked from the PR status. It’s useful to take a look if you’re changing any of the documentation.
Run build-and-serve-docs in nix-shell to host a local instance at http://0.0.0.0:8002/. Haddock is at http://0.0.0.0:8002/haddock.
These perform some of the same checks as Hydra, but Github Actions is often more available, so they return faster and act as a "smoke check".
-
The regular contributors to the Haskell code, all of whom can review and merge PRs are:
-
@koslambrou
-
@raduom
-
@sjoerdvisscher
-
@andreabedini
-
@berewt
-
@eyeinsky
-
@james-iohk
-
@kayvank
-
@ak3n
-
The maintainer of the documentation is @joseph-fajen.
-
If you have a technical dispute that you need help resolving, you can ask @koslambrou.
-
For problems with the developer environment setup, builds, or CI, you can ask @zeme-iohk, @Pacman99, or @koslambrou.
-
For any QA related issues, you can mainly query @james-iohk, or any of the other regular contributors.
-
The releasing of the software is handled by @ak3n, but if you have a specific problem you may also ask @koslambrou.