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

Restructure cabal file into a library with executables that depend on it #141

Merged
merged 8 commits into from
Feb 23, 2023

Conversation

basvandijk
Copy link
Contributor

@basvandijk basvandijk commented Feb 21, 2023

CI is way too slow and actually fails on darwin. The main reason for the slowness is that each executable rebuilds all the modules from the library.

This PR restructures the cabal file such that the executables depend on the library instead of on the source code of the library. This ensures the library is only built once.

The reason the cabal files was structured like this was to support ghcid. However most active maintainers are not using ghcid so we prefer to have faster builds instead.

Besides restructuring the cabal file this PR also switches building the haddocks from cabal to Nix which allows us to remove the cabal.project and cabal.project.freeze files which has the advantage that a cabal build will then pick the dependencies from the nix-shell instead of building them itself.

CI is way too slow and actually fails on darwin. The main reason for
the slowness is that each executable rebuilds all the modules from the
library.

This PR restructures the cabal file such that the executables depend
on the library instead of on the source code of the library. This
ensures the library is only built once.

The reason the cabal files was structured like this was to support
ghcid. However it seems GHCi supports loading multiple components
nowadays.
@nomeata
Copy link
Contributor

nomeata commented Feb 21, 2023

The reason the cabal files was structured like this was to support ghcid. However it seems GHCi supports loading multiple components nowadays.

It does? Last I checked GHC has some basic support, but it has not reached Cabal (for cabal repl yet).

But yes, I am rarely interactively working on this code base right now, so if none else misses this, fine with me.

@basvandijk
Copy link
Contributor Author

basvandijk commented Feb 21, 2023

It does?

I didn't actually check it but I followed the linked ticket (https://gitlab.haskell.org/ghc/ghc/-/merge_requests/395) and the associated MRs were merged so I assumed there was better support.

But yes, I am rarely interactively working on this code base right now, so if none else misses this, fine with me.

Thanks. I hope that by this and future restructurings we can lower our CI times and get passed the failing darwin builds on some of our PRs.

@nomeata
Copy link
Contributor

nomeata commented Feb 21, 2023

I still wonder how useful the Darwin build is at all. Shipping ic-ref to users never had a strong reason, and for our various CI tasks, Linux only should be fine.

@basvandijk
Copy link
Contributor Author

basvandijk commented Feb 21, 2023

I really would like to get rid of it but it seems the sdk depends on it (3cc51be).

@nomeata
Copy link
Contributor

nomeata commented Feb 21, 2023

Out of habit or because they care about it? I found https://forum.dfinity.org/t/future-of-the-ic-ref-emulator-dfx-start-emulator/9511 to be rather inconclusive.

@basvandijk basvandijk marked this pull request as ready for review February 22, 2023 17:23
@basvandijk basvandijk requested a review from a team as a code owner February 22, 2023 17:23
Copy link
Contributor

@mraszyk mraszyk left a comment

Choose a reason for hiding this comment

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

LGTM

Thank you!

@basvandijk
Copy link
Contributor Author

Last remaining problem is that the HPC coverage report (nix-build -A coverage) is empty:
Screenshot 2023-02-23 at 11 23 35
as opposed to the one from master:
Screenshot 2023-02-23 at 11 23 39
I'll see how to fix that...

@basvandijk
Copy link
Contributor Author

basvandijk commented Feb 23, 2023

Fixed.

BTW it would be cool to also publish the HPC coverage report to our GitHub pages. Will look into that in another PR.

@basvandijk
Copy link
Contributor Author

BTW I noticed that running nix-build -A coverage gets ic-ref to consume over 16GB of memory which is more than the 14GB the GitHub darwin runner has. This is likely why that job failed on darwin.

Screenshot 2023-02-23 at 12 35 37

@basvandijk basvandijk merged commit 927d815 into master Feb 23, 2023
@basvandijk basvandijk deleted the basvandijk/restructure-cabal-file branch February 23, 2023 12:14
@nomeata nomeata mentioned this pull request Mar 10, 2023
@nomeata
Copy link
Contributor

nomeata commented Mar 16, 2023

Multi-component cabal repl makes progress: https://well-typed.com/blog/2023/03/cabal-multi-unit/

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.

3 participants