Skip to content
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

Leaner build system #187

Merged
merged 22 commits into from
Jan 13, 2023
Merged

Leaner build system #187

merged 22 commits into from
Jan 13, 2023

Conversation

gabrielhdt
Copy link
Contributor

@gabrielhdt gabrielhdt commented Dec 9, 2022

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.

Copy link
Member

@florentc florentc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@gabrielhdt
Copy link
Contributor Author

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.

Also provide a flake for reproducibility with
- a default shell suitable for development (hls and stuff)
- a ci shell accessible with `nix develop .#ci` which doesn't have
  development related dependencies
@florentc florentc self-requested a review December 12, 2022 15:52
@gabrielhdt gabrielhdt marked this pull request as ready for review December 13, 2022 08:15
With shell hooks for plutusir
- Put tests that don't require internet (i.e. ormolu and hpack) into
  the flake checks
- Some sanity on the script 'run-tests' (advised by shellcheck)
- Update github actions dependencies
- Update github caching
.github/actions/nix-run/action.yml Outdated Show resolved Hide resolved
.github/workflows/build-and-test.yml Show resolved Hide resolved
.github/workflows/build-and-test.yml Outdated Show resolved Hide resolved
.github/workflows/build-and-test.yml Outdated Show resolved Hide resolved
ci/run-tests.sh Outdated Show resolved Hide resolved
ci/run-tests.sh Show resolved Hide resolved
shell.nix Outdated Show resolved Hide resolved
@Niols
Copy link
Member

Niols commented Jan 3, 2023

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

@Niols
Copy link
Member

Niols commented Jan 5, 2023

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

flake.nix Outdated Show resolved Hide resolved
@gabrielhdt
Copy link
Contributor Author

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

Ah that was because there were some conflicts with files.

shell.nix Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
Don't enforce an envrc, let people choose theirs :)
@Niols
Copy link
Member

Niols commented Jan 11, 2023

cf #208 for debugging the issue

@Niols
Copy link
Member

Niols commented Jan 12, 2023

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
Copy link
Contributor Author

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.

@gabrielhdt gabrielhdt merged commit 54c379a into tweag:main Jan 13, 2023
@gabrielhdt gabrielhdt deleted the gh@trythings branch January 13, 2023 09:18
gabrielhdt added a commit that referenced this pull request Jan 20, 2023
* Use Nixpkg's GHC 8.10.7

Also provide a flake for reproducibility with
- a default shell suitable for development (hls and stuff)
- a ci shell accessible with `nix develop .#ci` which doesn't have
  development related dependencies

* CI tweaks

- Put tests that don't require internet (i.e. ormolu and hpack) into
  the flake checks
- Some sanity on the script 'run-tests' (advised by shellcheck)
- Update github actions dependencies
- Update github caching

* Removed shell.nix

Tuned the setup of LD_LIBRARY_PATH

* Activate flake check in CI, fix formatting

* Add dependabot

* Replace cache by artifacts for test output

* Checks exit on error

* Remove envrc

Don't enforce an envrc, let people choose theirs :)

* Bump version of Pirouette

* Add Z3 as a dynamic library

* Also add Z3 to `buildInputs`

* Add dependabot

* Renaming workflows and steps, removed action

The run nix action was not used.

Co-authored-by: Nicolas “Niols” Jeannerod <nicolas.jeannerod@tweag.io>
mmontin pushed a commit that referenced this pull request Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants