Consensus: expose private sublibraries

[amesgen]

CHaP CI builds all components of a given project, but eg ouroboros-consensus-cardano has private-by-default sublibraries (used for tests), which themselves depend on other private-by-default sublibraries in eg ouroboros-consensus-diffusion, which causes failures at build time.

In this commit, we make the sublibraries public by enabling the respective flags.

Alternatives include:

• Patch Consensus to always make the sublibraries public.

• Don't build all (in particular private) sublibraries in CHaP CI (not sure if that is easily possible to do automatically).

[michaelpj]

What's wrong with just exposing the libraries unconditionally? It looks like they are classic "testlibs", why not just expose them? It's unclear to me what you're gaining here.

I'm unsure if this would work, but I think possibly you could fix this by also toggling the buildability status of the downstream libraries based on the flag, so:

library diffusion-testlib
  ...
  if flag(expose-sublibs)
    visibility: public
  else
    buildable: false

This seems true to me: diffusion-testlib is not buildable unless that flag is set. And I think that will stop cabal from trying to plan it, so it should work out okay.

[michaelpj]

I note that all of these schemes are going to cause trouble for anyone downstream who wants to use these libraries. I don't know if that's a goal.

[amesgen]

The original idea was to avoid downstream users accidentally/erroneously depending on the test sublibs (such that there is zero risk in us changing them as we wish).

Your suggestion sounds nice, and I would have expected that it works, but it doesn't seem like it 😢 : https://github.com/input-output-hk/cardano-haskell-packages/actions/runs/5924086096/job/16061002938?pr=446

[amesgen]

I note that all of these schemes are going to cause trouble for anyone downstream who wants to use these libraries. I don't know if that's a goal.

What trouble are you referring to? I think eg node won't notice this problem at all, as there are no Consensus sublibs in the transitive dependencies of it.

[michaelpj]

Also, I'm a little confused by the original problem. I don't think we should build anything except public sublibraries... Maybe I can fix that directly...

Your suggestion sounds nice, and I would have expected that it works, but it doesn't seem like it

I'm not sure quite what's going on there, it looks like maybe a cabal bug 🤔

What trouble are you referring to? I think eg node won't notice this problem at all, as there are no Consensus sublibs in the transitive dependencies of it.

So your intention is really for absolutely nobody outside consensus to use them? Are they not useful for downstream? As soon as someone downstream wants to depend on them this is going to be quite unpleasant.

FWIW my preferred advice would still be "don't try to be so clever and just expose them" :)

[amesgen]

Also, I'm a little confused by the original problem. I don't think we should build anything except public sublibraries... Maybe I can fix that directly...

Yeah, I couldn't find a way to get the visibility status of a sublib in haskell.nix, but maybe I missed something (or it is just a small haskell.nix PR away from being available).

So your intention is really for absolutely nobody outside consensus to use them? Are they not useful for downstream? As soon as someone downstream wants to depend on them this is going to be quite unpleasant.

Yes, at least for now, but the idea was that people could just toggle the respective flags if they really really want to use them.

FWIW my preferred advice would still be "don't try to be so clever and just expose them" :)

Yeah, that is definitely on the table, it is just too difficult to predict which constructions slightly off the well-trodden path won't cause Cabal to complain very loudly 😄

[michaelpj]

Yeah, that is definitely on the table, it is just too difficult to predict which constructions slightly off the well-trodden path won't cause Cabal to complain very loudly

My feelings are:

• Visibility is a new feature and a bit janky
• Conditional buildability is fiddly to get right
• Cabal flags are often a pain and should be used sparingly

So the combination of all of them is playing with fire a bit 😂

Yeah, I couldn't find a way to get the visibility status of a sublib in haskell.nix, but maybe I missed something (or it is just a small haskell.nix PR away from being available).

Summon @andreabedini, do you know if we can get this?

[amesgen]

We will resolve this by removing the flag, such that the sublibs are always public, @nfrisby is preparing the respective PRs.

[nfrisby]

Here is the aforementioned PR. Those clicking this link probably only care about the diff in the README.md file. IntersectMBO/ouroboros-consensus#304

[andreabedini]

This state of affairs is depressing :-/ where's the original error? We need to push to fix haskell.nix

[michaelpj]

I'm not sure it's haskell.nix's fault. The code for CHaP CI calls getAllComponents, which apparently includes private sublibs. For other stuff we don't want to build on CHaP (tests and benchmarks) we just don't enable them in the cabal.project, but for private sublibs I think we should just filter them out.

I also think the changes discussed here are reasonable: it is true that e.g. the downstream private libs were not buildable if the upstream ones were not public. So the previous state of things was not quite representing reality properly. But still: we shouldn't have even tried to build them.

[michaelpj]

Here's the failing run btw: https://github.com/input-output-hk/cardano-haskell-packages/actions/runs/5913615616/job/16038187781

[andreabedini]

This conversation dropped off my radar :-/ I understand these sublibraries were made unbuildable by a flag? (The error from the logs above was Dependency on unbuildable library 'protocol-testlib' from ouroboros-consensus-protocol). A private library should only be available as a dependency of another component in the same package, so why marking them unbuildable?
Off the top of my head, haskell.nix should have no problem building a private library. Being private does not affect the building, only the dependency resolution (planning) which is done beforehand.

[amesgen]

I understand these sublibraries were made unbuildable by a flag?

No, we made the sublibraries unconditionally public, but gave them a "scary" name (unstable- prefix) such that people hopefully don't use them unless they are willing to accept that they can change arbitrarily), see https://github.com/input-output-hk/cardano-haskell-packages/pull/445#issuecomment-1686326396 and https://github.com/input-output-hk/cardano-haskell-packages/pull/445#issuecomment-1686431827.

A private library should only be available as a dependency of another component in the same package, so why marking them unbuildable?

At the time this PR was created, we made our sublibraries only conditionally public (based on a flag that was off by default). The idea was that you would enable this flag in a dev environment, but not enable it when you depend on our libraries. In particular, it was not enabled on CHaP, but CHaP still tried to build these private sublibraries, which did now depend on private sublibraries on other packages, hence the errors.

Quite a tricky situation, hope that makes it clear!

[andreabedini]

Thank you @amesgen, I still have some doubts:

The idea was that you would enable this flag in a dev environment, but not enable it when you depend on our libraries.

Why do you need to make them public during development? Are you going to use them from other packages? like e.g. you have tests in another package that have to use something in another package internal library? wouldn't it be easier to put those tests into the same package as the internal library?

Sorry if I am being naïve but I am also curious to know :)

[amesgen]

Are you going to use them from other packages? like e.g. you have tests in another package that have to use something in another package internal library? wouldn't it be easier to put those tests into the same package as the internal library?

This is a subgraph of our current component graph (in reality, there are various test suites that depend on these testlibs):

graph
    subgraph diffusion
    oc-diffusion
    oc-diffusion:lib:unstable-diffusion-testlib
    oc-diffusion:test:infra-test
    end
    subgraph cardano
    oc-cardano
    oc-cardano:lib:unstable-shelley-testlib
    oc-cardano:test:shelley-test
    end


    oc-diffusion:lib:unstable-diffusion-testlib --> oc-diffusion
    oc-diffusion:test:infra-test --> oc-diffusion:lib:unstable-diffusion-testlib
    oc-cardano:lib:unstable-shelley-testlib --> oc-cardano
    oc-cardano:test:shelley-test --> oc-cardano:lib:unstable-shelley-testlib
    oc-cardano:test:shelley-test --> oc-cardano

    oc-cardano:lib:unstable-shelley-testlib --> oc-diffusion:lib:unstable-diffusion-testlib

Loading

Note that we can't move oc-cardano:test:shelley-test to diffusion as it depends on the main library oc-cardano.