tweag / cooked-validators

MIT License
39 stars 11 forks source link

Leaner build system #187

Closed gabrielhdt closed 1 year ago

gabrielhdt commented 1 year ago

CI and development tools update

This PR brings the two following updates

  1. Update Nix related components
  2. Update the CI script

Nix-related updates

The full stack of "legacy" Nix code in the directory nix/ has been dropped, in favour of a flake. Therefore, to develop, one may use nix develop.

Other CI updates

All offline (in the sense no internet) tests (i.e. ormolu and hpack) have been moved to the flake tests (they can hence be run with nix flake check). Cabal tests remain in the ci/run-tests.sh script. The CI also provides some form of cabal caching, which seems to bring the compile time down from an hour and some more to 20 minutes.

Further possible work (probably in subsequent PRs)

The other step would be to trim the cabal.project file, trying to get the dependencies from CHaP. This has already been done for the update to plutus v2 (can be seen here https://github.com/tweag/scverif-exploration/commit/ae91d3da0bb40c72952495e850423bb0bffe89bb at Plutus/lotto/offchain/cabal.project), so maybe it's no use to try using CHaP with the current main branch of plutus-libs.

Remarks

This pull request can already be used to cherry-pick this Nix setup (I should probably move that branch to the main repository)

git remote add gh git@github.com:gabrielhdt/plutus-libs
git fetch gh
git cherry-pick 8d14a973832e28b0fc6577fa5efd25ef12232530 41966c4517a329b59f74a90c1b5b9b028fcaeadb

This does not replace the ongoing work with Connor, since most dependencies are still handled by Cabal, without caching.

gabrielhdt commented 1 year ago

Besides easing transition by providing back compatibility, is there a specific reason why we still have a shell.nix instead of specifying our build dependencies in the flake directly?

No there is not, we could totally get rid of it.

Niols commented 1 year ago

We could also use this PR to (re)name the workflow and the jobs. (Edit: s/rename/(re)name/)

Niols commented 1 year ago

Hum. the workflows aren't running anymore? Is it GH or us?

gabrielhdt commented 1 year ago

Hum. the workflows aren't running anymore? Is it GH or us?

Ah that was because there were some conflicts with files.

Niols commented 1 year ago

cf https://github.com/tweag/plutus-libs/issues/208 for debugging the issue

Niols commented 1 year ago

Almost good to go! I have two more comments:

  1. We could name the workflows.

  2. I understand why it makes sense for CI to use a smaller environment. However:

    • I fear that it might lead to discrepancies between local environments and CI ones.
    • Having only one environment would simplify our already-slightly-big flake.
    • When we add a Cachix instance, it would be nice to have the CI cache the whole environment anyways.

    I think only this third point really matters, so we could also change that later if we think it makes sense then. WDYT?

gabrielhdt commented 1 year ago
2. I understand why it makes sense for CI to use a smaller environment. However:

   * I fear that it might lead to discrepancies between local environments and CI ones.

This shouldn't be an issue since the ci and the default devShells use the same tool, right?

  • Having only one environment would simplify our already-slightly-big flake.
  • When we add a Cachix instance, it would be nice to have the CI cache the whole environment anyways.

    I think only this third point really matters, so we could also change that later if we think it makes sense then. WDYT? It doesn't matter yet, but if the flake can get simpler, I'd be in favour of that. On the other hand, there are people that want to use their own development tools, so they don't want HLS to be brought in, they just want a reproducible build (I think @facundominguez is one such person). IMO it makes sense to have a minimal environment where we don't force our development setup upon users.