Switch to Cardano haskell package repository #4411

andreabedini · 2022-09-01T10:34:39Z

@michaelpj and I have been working to establish an ~~internal~~ custom haskell package repository. This simplifes cabal's configuration and improves DX.

The "repository" is a static website that implements the same paths as hackage-server and it is hosted on GitHub. The repository is built from a static description of its content, with the idea that teams will be able to publish a package with a simple PR, see the repository's README for details.

This patch:

Removes all source-repository-package stanzas (package repository has
been pre-populated with the exact same versions)
Removes cabal work-around disabling tests for packages erroneously
considered local. Cabal now does the right thing
Removes cabalWrapped, as now unnecessary

andreabedini · 2022-09-01T10:36:54Z

cabal.project

-- Disable all tests by default
-
-tests: False
+tests: True


Do we need this? The documentation seems to suggest it is not useful:

Force test suites to be enabled. For most users this should not be needed, as we always attempt to solve for test suite dependencies, even when this value is False; furthermore, test suites are automatically enabled if they are requested as a built target.

Hmm, I think in the past it used to be necessary so cabal wouldn't re-solve for tests when you switched to running tests, perhaps they fixed that. But that's an orthogonal issue I guess.

yes, before you had to do tests: False and enabled them one by one for the local packages. This is now solved (which is why I left tests: True which was the original intent). Now I wonder if one needs to enable them at all.

Yes, I was talking about "do we need tests: True at all". I don't know (we have it for plutus!).

When using source-repository-package the directive tests: True turns on tests within all source-repository-package dependencies, which (especially higher up the stack) just needless runs tests in dependencies for which it should be possible to avoid re-rumning tests.

In say db-sync we have tests: False to disable all tests and the specific stanzas to turn on tests for packages in the db-sync repo.

When using source-repository-package the directive tests: True turns on tests within all source-repository-package dependencies, which (especially higher up the stack) just needless runs tests in dependencies for which it should be possible to avoid re-rumning tests.

Is this still true in recent versions of cabal? We have tests: True in plutus and I've never observed it running or building tests for s-r-ps, and I tried again just now.

cabal.project

nix/haskell.nix

michaelpj · 2022-09-01T14:28:34Z

@kosyrevSerge there is some code in the workbench scripts that scrapes the cabal.project for dependency versions and is now failing:

We think that can just be replaced by using ghc-pkg field --simple-output <package> version, which will give the package version. That's what I think we want now that we're fetching released versions from a repository instead. Does that sound right to you?

erikd · 2022-09-01T21:07:56Z

From the sound of this "Internal Hackage repository" its internal and not accessible by people outside IOG. If that is the case, then how are people outside IOG supposed to build our software?

cabal.project

erikd · 2022-09-01T22:47:08Z

Even if it is publicly available, we need to look at at how much more difficult (if at all) the new way of accessing packages is in comparison to the old method.

michaelpj · 2022-09-02T08:50:16Z

From the sound of this "Internal Hackage repository" its internal and not accessible by people outside IOG. If that is the case, then how are people outside IOG supposed to build our software?

It's not. It's publicly accessible. There should be no difference for anyone building our software, this is all understood perfectly well by cabal. The most you need to do is call cabal update.

Let's try and avoid the "internal" descriptor. I've been trying to call it just the "Cardano Haskell package repository", which is a bit of a mouthful but hopefully not misleading.

andreabedini · 2022-09-02T10:15:09Z

In bf585f5 I have temporarily disabled parts of the workbench scripts. The script extracted the commit has of some cardano components from the cabal.project file, information which is not available anymore. Probably the best equivalent would be either the versions from the install plan (which AFAIU is not available when those scripts run) or just the repos and their index-state. It kinda depends on what the goal of those scripts is.

ch1bo · 2022-09-06T16:46:03Z

"Cardano Haskell package repository", which is a bit of a mouthful but hopefully not misleading.

We could call it Cardano HAskell Package rePositorY -> CHAPPY 🐶 .. unfortunately spelled wrong to be a reference to this movie

andreabedini · 2022-09-07T07:42:24Z

Another option is to just drop the "repository": Cardano Haskell Packages. "Repository" is a confusing word here because it has two meaning (as in Hacakge and in git).

andreabedini · 2022-09-07T14:50:12Z

Note to self, I made some cabal file revisions on the repository that end up breaking the build plan for cardano-node 😞 Do not bump the index-state until I fix the issue. 🙏

andreabedini · 2022-09-08T14:34:30Z

Note to self, I made some cabal file revisions on the repository that end up breaking the build plan for cardano-node disappointed Do not bump the index-state until I fix the issue. pray

We run into a bit of a roadblock with haskell.nix not liking cabal file revisions. As an emergency measure we removed them from the repository, so we can unblock this PR.

ch1bo · 2022-09-09T05:59:19Z

haskell.nix not liking cabal file revisions

Good haskell.nix. Allowing released code to be revised was never a good idea also IMO.

andreabedini · 2022-09-13T08:16:25Z

Good haskell.nix. Allowing released code to be revised was never a good idea also IMO.

cabal file revisions don't change the code, only the metadata about it so we know what version works with what other version. We can do it in a completely declarative and reproducible way so this should not be a problem.

andreabedini · 2022-09-20T11:31:12Z

@erikd

Even if it is publicly available, we need to look at at how much more difficult (if at all) the new way of accessing packages is in comparison to the old method.

See this repo for an example downstream consumer. It should work with just cabal build. Note that I have libsecp256k1 installed in my system, I don't remember it is needed or not.

deepfire · 2022-09-22T11:33:32Z

nix/workbench/manifest.sh

-    grep "^[ ]*location: .*/${dep}\$" "${project_file}" -A1 \
-        | tail -n-1 | sed 's/^.* tag: //'
+    # FIXME
+    echo 0123456789ABCDEF0123456789ABCDEF01234567


Knowing exact gitrevs for key dependencies is useful, since it simplifies discussions around reproduction.

Can we make the cardanoHaskellPackages (and I specifically mean the Nix flake) supply the gitrevs for all constituent packages it provides?

Then we could plug in this information here.

I maybe missing some context but if you depend on versions, is not even better? The versions are not supposed to be mutable either.

I think it would be more appropriate to use the package versions? That should be enough for reproducibility, right? I think @andreabedini had a way to do that with just ghc-pkg, so that should be straightforward.

In general, this is going to be at git revision granularity -- since we don't always work with released versions of packages.

Think feature branch development, for example.

So, having gitrevs uniformly available would be a consistent approach.

Having versions in addition -- would be nice as well.

I tried to update this part but I couldn't really do it without understanding its intended use. I don't think there's a way to push the gitrevs through the index. I suggest we stick to versions. @deepfire, just to make ure I understand, what are gitrevs useful for? looking/finding the right source used? We are thinking of adding html pages for the index, they could include a link to the corresponding source (maybe).

I never said versions are bad to have!

Only suggesting that losing the gitrevs is unfortunate, because gitrevs are precise and not subject to human errors and other deviations.

Having a cabal version, on the other hand, does not guarantee you a gitrev.

Why a gitrev is important? Automation.
For example, a valid gitrev means a github URL. You can also mindlessly punch in a gitrev into arbitrary git commands -- without having any second thoughts.

Yes, all good projects should have tags with names exactly matching versions at precisely the right revisions.

But the key point is -- should.

@deepfire while it is true that hackage.haskell.org packages do not have revisions, due to the design of hackage allowing arbitrary source management, and a such just release tarball uploads, cardano-haskell-packages does have revisions. So at least for cardano-haskell-packages you can go from version number to git revision e.g. measures-0.1.0.0;

@andreabedini maybe we should ensure that people don't put tags as revisions in the url, as those may change over time (or resolve them at the point of index constructions).

@angerman At the moment we are pointing to arbitrary tarballs, when I implement a proprer github backend I can do this.

@andreabedini, in the interim it'd be nice to have cardanoHaskellPackages provide at least the versions in some kind of a package-name -> version string attrset.

..so we'd be able to wire this information to this point.

@deepfire 👍 I am working toward this direction

nix/workbench/run.sh

nix/workbench/shell.nix

deepfire · 2022-09-22T13:22:51Z

Thank you @michaelpj @andreabedini !

ch1bo · 2022-09-23T13:40:48Z

cabal.project

-  location: https://github.com/input-output-hk/flat
-  tag: ee59880f47ab835dbd73bea0847dab7869fc20d8
-  --sha256: 1lrzknw765pz2j97nvv9ip3l1mcpf2zr4n56hwlz0rk7wq7ls4cm
-


cabal.project

michaelpj · 2022-09-27T13:53:46Z

I think this is ready to go. I added a note about using the cardano-node packages from elsewhere, and consolidated some development documentation.

Jimbo4350

I'm generally for this but I will take a closer look after the cli release

CONTRIBUTING.md

andreabedini · 2022-09-29T14:24:33Z

@michaelpj I suggest a stronger wording (and a clear policy)

--
-- Open PR upstream, maintainer unresponsive, hopefully short-lived fork.
--

-- Note: these dependencies are at project level and not at package level.
-- This means that they won't be part of the dependencies of any package
-- released from this repository and they won't be seen by downstream
-- consumers.
-- It is ok to keep these during development but it is essential that
-- they are not needed by any released package.

What do you think?

michaelpj · 2022-09-29T14:27:33Z

I'm not sure if that's realistic. Maybe if we just release a patched version of ekg-json to CHaP using our padding scheme we can get past it. I'm unsure about threepenny-gui. It would be nice to enforce a "no source-repository-packages at release time policy, though.

andreabedini · 2022-09-29T14:38:16Z

It would be nice to enforce a "no source-repository-packages at release time policy, though.

🤞

michaelpj · 2022-10-07T12:51:50Z

@Jimbo4350 if the release is going ahead off the branch, can we move forward with this on master?

Jimbo4350

A few comments. @andreabedini can you squash/tidy up the commits?

CONTRIBUTING.rst

- Remove all source-repository-package stanzas (package repository has been pre-populated with the exact same versions) - Remove cabal work-around disabling tests for packages erroneously considered local. Cabal now does the right thing - Remove cabalWrapped, as now unnecessary

michaelpj · 2022-10-07T14:36:43Z

The GHA actions checks are segfaulting, but they're also segfaulting on master, so I don't think it's this PR's fault.

This reflects the policy that we have to follow now that the node is [using CHaP](IntersectMBO/cardano-node#4411). It also lays out a draft of a policy for when it can be appropriate to release to Hackage, and when it is appropriate to use `source-repository-package`s.

This reflects the policy that we have to follow now that the node is [using CHaP](IntersectMBO/cardano-node#4411). It also lays out a draft of a policy for when it can be appropriate to release to Hackage, and when it is appropriate to use `source-repository-package`s. Fixes #13.

This reflects the policy that packages have to follow now that the node is [using CHaP](IntersectMBO/cardano-node#4411). It also lays out a draft of a policy for when it can be appropriate to release to Hackage, and when it is appropriate to use `source-repository-package`s. Fixes #13.

michaelpj · 2022-10-07T15:56:01Z

@Jimbo4350 squashed and hydra passed.

Jimbo4350

LGTM!

michaelpj · 2022-10-10T09:35:32Z

bors r+

iohk-bors · 2022-10-10T10:14:33Z

Build succeeded:

andreabedini commented Sep 1, 2022

View reviewed changes

michaelpj reviewed Sep 1, 2022

View reviewed changes

ltouro reviewed Sep 1, 2022

View reviewed changes

cabal.project Outdated Show resolved Hide resolved

michaelpj changed the title ~~Switch to internal haskell package repository~~ Switch to Cardano haskell package repository Sep 2, 2022

andreabedini marked this pull request as ready for review September 14, 2022 08:08

andreabedini requested review from deepfire, mgmeier, fmaste, dcoutts, Jimbo4350, erikd, newhoggy and JaredCorduan as code owners September 14, 2022 08:08

andreabedini requested review from angerman and michaelpj September 14, 2022 10:04

deepfire reviewed Sep 22, 2022

View reviewed changes

nix/workbench/run.sh Outdated Show resolved Hide resolved

deepfire suggested changes Sep 22, 2022

View reviewed changes

nix/workbench/shell.nix Outdated Show resolved Hide resolved

deepfire self-requested a review September 22, 2022 13:58

ch1bo reviewed Sep 23, 2022

View reviewed changes

shlevy reviewed Sep 26, 2022

View reviewed changes

cabal.project Show resolved Hide resolved

deepfire approved these changes Sep 27, 2022

View reviewed changes

michaelpj requested a review from LudvikGalois as a code owner September 27, 2022 13:45

michaelpj force-pushed the andrea/cardano-haskell-package-repo branch from 51c5e67 to 737e10d Compare September 27, 2022 13:53

Jimbo4350 requested changes Sep 27, 2022

View reviewed changes

CONTRIBUTING.md Outdated Show resolved Hide resolved

Jimbo4350 requested changes Oct 7, 2022

View reviewed changes

CONTRIBUTING.rst Show resolved Hide resolved

CONTRIBUTING.rst Show resolved Hide resolved

CONTRIBUTING.rst Show resolved Hide resolved

michaelpj force-pushed the andrea/cardano-haskell-package-repo branch from 9b78ec6 to 814b6d8 Compare October 7, 2022 14:21

michaelpj requested a review from CarlosLopezDeLara as a code owner October 7, 2022 14:21

michaelpj mentioned this pull request Oct 7, 2022

Add a policy on Haskell package distribution input-output-hk/cardano-engineering-handbook#23

Merged

Jimbo4350 approved these changes Oct 9, 2022

View reviewed changes

iohk-bors bot merged commit a0b0003 into master Oct 10, 2022

iohk-bors bot deleted the andrea/cardano-haskell-package-repo branch October 10, 2022 10:14

Switch to Cardano haskell package repository #4411

Switch to Cardano haskell package repository #4411

Conversation

andreabedini commented Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelpj commented Sep 1, 2022

erikd commented Sep 1, 2022 • edited Loading

erikd commented Sep 1, 2022

michaelpj commented Sep 2, 2022

andreabedini commented Sep 2, 2022

ch1bo commented Sep 6, 2022 • edited Loading

andreabedini commented Sep 7, 2022 • edited Loading

andreabedini commented Sep 7, 2022

andreabedini commented Sep 8, 2022

ch1bo commented Sep 9, 2022

andreabedini commented Sep 13, 2022

andreabedini commented Sep 20, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepfire Sep 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepfire Sep 27, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

deepfire commented Sep 22, 2022

Choose a reason for hiding this comment

michaelpj commented Sep 27, 2022

Jimbo4350 left a comment

Choose a reason for hiding this comment

andreabedini commented Sep 29, 2022

michaelpj commented Sep 29, 2022

andreabedini commented Sep 29, 2022

michaelpj commented Oct 7, 2022

Jimbo4350 left a comment • edited Loading

Choose a reason for hiding this comment

michaelpj commented Oct 7, 2022

michaelpj commented Oct 7, 2022

Jimbo4350 left a comment

Choose a reason for hiding this comment

michaelpj commented Oct 10, 2022

iohk-bors bot commented Oct 10, 2022

andreabedini commented Sep 1, 2022 •

edited

Loading

erikd commented Sep 1, 2022 •

edited

Loading

ch1bo commented Sep 6, 2022 •

edited

Loading

andreabedini commented Sep 7, 2022 •

edited

Loading

deepfire Sep 22, 2022 •

edited

Loading

deepfire Sep 27, 2022 •

edited

Loading

Jimbo4350 left a comment •

edited

Loading