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

Cabal should perhaps reexport all the modules from Cabal-syntax to reduce breakage #7974

Closed
mpickering opened this issue Feb 14, 2022 · 62 comments · Fixed by #8064
Closed

Cabal should perhaps reexport all the modules from Cabal-syntax to reduce breakage #7974

mpickering opened this issue Feb 14, 2022 · 62 comments · Fixed by #8064
Assignees

Comments

@mpickering
Copy link
Collaborator

The changes to split up Cabal and Cabal-syntax cause most packages which use the Cabal library to fail to build (https://gitlab.haskell.org/ghc/head.hackage/-/jobs/944876).

Perhaps it would be worthwhile to make Cabal reexport all the modules in Cabal-syntax to minimise this kind of distruption.

cc @Mikolaj

@Mikolaj
Copy link
Member

Mikolaj commented Feb 14, 2022

@patrickdoc, @gbaz, @fgaz, what do you think?

@gbaz
Copy link
Collaborator

gbaz commented Feb 14, 2022

Hrm. I recall there's one general issue that discourages module reexports:

If package A re-exports package B, then the api surface of package B becomes part of the surface of package A.

So to stick to the PVP, each version of package A should in pin the dependency on package B to within a single minor revision range. This can be a bit of a pain. In our case, it may be ok, given that we control everything.

I don't recall any other things we may need to worry about offhand, but also I am not sure...

@fgaz
Copy link
Member

fgaz commented Feb 14, 2022

I'm 👎 on this. One of the reasons to separate Cabal-syntax was to decouple the two release processes. We could only reexport those module in the next version as a transitory period, but that would only delay the breakage, wouldn't it?

@mpickering
Copy link
Collaborator Author

I imagine most people who are depending on Cabal do not care at all about the difference between Cabal and Cabal-syntax. If they want to just depend on Cabal-syntax and get more updates then they can modify their programs to do so but otherwise they will be fine just using Cabal with whatever version it depends on.

@fgaz
Copy link
Member

fgaz commented Feb 14, 2022

There's more discussion about this starting from #7620 (comment)

@Mikolaj
Copy link
Member

Mikolaj commented Feb 14, 2022

For reference, here's a significant part of the original motivation: #7559, In particular, it seems re-exporting all of Cabal in Cabal-syntax would be a deal breaker, but I'm not sure about the reverse.

However, another part of the motivation, not mentioned there [edit: @fgaz just linked to it], is permitting people to bundle their own versions of Cabal-syntax that parse their .xml or .tex .cabal file mutations. Could we perhaps work towards a small and general enough API that it fits any wild experiments with .cabal files formats and so can be included in Cabal without stifling progress? Or is exposing the implementation of datatypes required?

@mpickering: in your particular use case(s), which parts of Cabal-syntax would you find convenient to be exported in Cabal?

@mpickering
Copy link
Collaborator Author

My particular concern is unnecessary downstream breakage but if the cost of this change has been evaluated and the breakage is deemed acceptable then I don't have a further opinion.

@Mikolaj
Copy link
Member

Mikolaj commented Feb 15, 2022

Any other comments? @fendor? @andreasabel?

I hope users of Cabal the library are going to try the master branch and will let us know if the migration can/should be made easier.

@fgaz
Copy link
Member

fgaz commented Feb 15, 2022

Maybe we should send a message in the mailing list alerting of this change? We should also do proper release candidates this time.

@fendor
Copy link
Collaborator

fendor commented Feb 15, 2022

Looking at hackage reverse deps, we can see that almost 700 packages depend on Cabal. Admittedly, almost 200 have an outdated dependency, but that's still potentially 500 packages that break now.

For comparison, ghc has 250 reverse deps on Hackage. The comparison is not fair at all, since every package can bump their dependencies individually, and does not have to wait for its dependencies to update as well, etc... But my point being, that a lot of packages break.

I think we have the following goals:

  • Maintainers have to update their dependency only once
  • Decouple Cabal and Cabal-syntax for good
  • Make the transition as easy as possible

Since the changes are mostly mechanical, maybe we can introduce a migration tool for cabal files (called from now on cabal-migrate)? In general, this might be helpful for other features as well and does not need to live in cabal itself.

I think we can tick most of the boxes from above in the following way:

  1. For now, re-export Cabal-syntax in Cabal (this couples it together again)
  2. Implement a cli tool that makes the transition trivial (e.g. cabal-migrate --issue cabal-syntax)
    • This requires Cabal-syntax exactprinter.
  3. Stop re-exporting Cabal-syntax from Cabal
  4. Point people to cabal-migrate (If we merge it into cabal directly, the error message can point to the command immediately once it notices what error has occurred).

Unfortunately, all of this is a fair amount of work (exactprinter, cabal-migrate) and might take longer than the release window we currently have. Maybe, it is best to just watch Hackage burn for a bit.

@gbaz
Copy link
Collaborator

gbaz commented Feb 15, 2022

I strongly suspect that most packages that build with the cabal library do so because of custom setup scripts, and furthermore those scripts only make use of one or two things exported by cabal-syntax at the very most (and perhaps most use nothing).

That said, @mpickering, I can't determine from the output of what you linked which packages fail and for what reason. I suspect that Distribution.Package may well be the primary reason they fail. If so, I would be open to a limited re-export of one or two key modules for now, if it has significant impact.

@fgaz
Copy link
Member

fgaz commented Feb 15, 2022

Also the fix is trivial: just add Cabal-syntax to the dependencies. A dummy Cabal-syntax was released so there's no need for conditionals (please correct me if I'm wrong @patrickdoc)

@patrickdoc
Copy link
Collaborator

For end users, yeah, you should be able to just add Cabal-syntax.

For packages that want to support many versions of Cabal, you can use the conditionals recommended here: https://hackage.haskell.org/package/Cabal-syntax

@mpickering
Copy link
Collaborator Author

My experience is that you can't just add a Cabal-syntax dependency as the solver is free to pick different Cabal and Cabal-syntax versions. Therefore you end up with plans which include Cabal-3.2.0.0 and Cabal-syntax-3.7 which export modules of the same name. Perhaps I did something wrong on my end.

My latest attempt at patches is here and I need to debug why these are failing.

@fgaz
Copy link
Member

fgaz commented Feb 15, 2022

Huh, I thought we made it so those kind of conflicts are excluded

@patrickdoc
Copy link
Collaborator

patrickdoc commented Feb 15, 2022

Cross-package dependencies are a little hard to express, and I don't think it's possible as a consumer of a library to express:
"I'm not using Cabal, but because I'm using Cabal-syntax you must use Cabal > 3.6"

If all of your dependencies that require Cabal have applied the conditional linked above, then you can safely request only Cabal-syntax.

If any of your dependencies that require Cabal have not applied the conditionals, then your options are:

  • Force them to use a newer Cabal version by depending on it yourself (i.e. adding Cabal > 3.6 in your .cabal)
  • Add the conditionals (which will give you a working build by pessimistically defaulting to the older Cabal version)

In short, based on my understanding of how resolution works, you can't use Cabal-syntax until all of your dependencies that care have added support for it.

Edit: That came off a little more "final" than intended. The conclusion is that you can't end up with a solved dependency tree that includes Cabal-syntax until all of your dependencies that use Cabal have handled the split. Which is sort of understandable as that would be the result of any breaking change I think.

@jneira
Copy link
Member

jneira commented Feb 16, 2022

"I'm not using Cabal, but because I'm using Cabal-syntax you must use Cabal > 3.6"

hmmm and what about adding Cabal-syntax < 0 (or something equivalent) to all conflicting Cabal versions, via revisions if needed?
EDIT: or even better Cabal-syntax < 3.7

@jneira
Copy link
Member

jneira commented Feb 16, 2022

"I'm not using Cabal, but because I'm using Cabal-syntax you must use Cabal > 3.6"

hmmm and what about adding Cabal-syntax < 0 (or something equivalent) to all conflicting Cabal versions, via revisions if needed? EDIT: or even better Cabal-syntax < 3.7

oh, i forgot you cant add new dependencies via revisions :-/
So it would need upload new Cabal minor versions for all major versions we want to be compatible. Maybe prevent breakage worths it.
We could do it for all Cabal versions >= 3.0 and do it on demand for older versions

EDIT: It would not help packages with a exact bound (f.e. Cabal == 2.4), i hope they will not be many
EDIT: i am not very proficient with regexps but this query only returns 3 packages: https://hackage-search.serokell.io/?q=Cabal.*%3D%3D%5B%5E%5C*%5D*%5B%2C%5Cn%5D

@andreasabel
Copy link
Member

@jneira

EDIT: i am not very proficient with regexps but this query only returns 3 packages:

The regex Cabal[[:space:]]*==[ 0-9.*]+ finds some more. Of the 28 hits it returns, most should be valid.

@jneira
Copy link
Member

jneira commented Feb 16, 2022

Cabal[[:space:]]==[ 0-9.]+

@jneira

EDIT: i am not very proficient with regexps but this query only returns 3 packages:

The regex Cabal[[:space:]]*==[ 0-9.*]+ finds some more. Of the 28 hits it returns, most should be valid.

hmm i see a lot of them similar to Cabal == 3.6.* and that ones will get x.y.z.1, is it?

@andreasabel
Copy link
Member

Looking at @mpickering 's patches, I am worried. Atm, there will be only one Cabal version with a -syntax sibling, but what if there are many. Do you need one impl(ghc) conditional for each ghc version to couple the correct -syntax package to Cabal?

To solve the problem of coupling Cabal and Cabal-syntax, couldn't we have:

  • Cabal-syntax.
  • Cabal-engine: This is current-Cabal minus Cabal-syntax.
  • Cabal as a shim that is simply Cabal-syntax + Cabal-engine.

If you depend on Cabal, you simply get what you used to get, but you could also depend on Cabal-syntax or depend on Cabal-engine with a different implementation of the Cabal syntax.

Would this work? (Maybe someone more immersed in the matter can answer this.)

@jneira
Copy link
Member

jneira commented Feb 16, 2022

  • Cabal as a shim that is simply Cabal-syntax + Cabal-engine.

And keep the shim idefinitely (or until all sensible packages switch to Cabal-syntax + Cabal-engine)?
Maintainers would not have any reason to do it, as you are offering the shim with lastest fixes and features.

@jneira
Copy link
Member

jneira commented Feb 16, 2022

hmm i see a lot of them similar to Cabal == 3.6.* and that ones will get x.y.z.1, is it?

Moreover hackage revisions could be done for those cases Cabal == x.y.z

@mpickering
Copy link
Collaborator Author

It turns out your can't write conditionals in a setup-depends field so for head.hackage I have

  1. Remove the allow-newer: Cabal from the cabal.project
  2. Added upper bounds (Cabal < 3.7) for all relevant packages with Setup.hs scripts

@Mikolaj
Copy link
Member

Mikolaj commented Feb 16, 2022

2. Added upper bounds (Cabal < 3.7) for all relevant packages with Setup.hs scripts

Sadly, that won't work in the long run --- all custom script packages will be stuck at Cabal 3.6 until that starts conflicting with other things and then they should all at once switch to >= 3.7 (can be done in bulk in Hackage, but it won't work if some of them have genuine problems with newer Cabals).

@andreasabel
Copy link
Member

andreasabel commented Feb 16, 2022

  • Cabal as a shim that is simply Cabal-syntax + Cabal-engine.

And keep the shim idefinitely (or until all sensible packages switch to Cabal-syntax + Cabal-engine)? Maintainers would not have any reason to do it, as you are offering the shim with lastest fixes and features.

Yes, this shim could be auto-generated by just reexporting the modules of these two packages (or is there a lighter way?).
Maintainers can chose to switch whenever it gives them benefits (like shorter compilation times if they only need Cabal-syntax).

If we want to nudge folks to get away from the shim Cabal, we could have deprecation warnings. But why, really?

@mpickering
Copy link
Collaborator Author

2. Added upper bounds (Cabal < 3.7) for all relevant packages with Setup.hs scripts

Sadly, that won't work in the long run --- all custom script packages will be stuck at Cabal 3.6 until that starts conflicting with other things and then they should all at once switch to >= 3.7 (can be done in bulk in Hackage, but it won't work if some of them have genuine problems with newer Cabals).

Right, this just unblocks me for head.hackage, not a long-term solution.

@Mikolaj
Copy link
Member

Mikolaj commented Feb 16, 2022

Yes, this shim could be auto-generated by just reexporting the modules of these two packages (or is there a lighter way?). Maintainers can chose to switch whenever it gives them benefits (like shorter compilation times if they only need Cabal-syntax).

I'm +1 on the shim. What's the cheapest scheme?

If we want to nudge folks to get away from the shim Cabal, we could have deprecation warnings. But why, really?

Custom setups are destined to die anyway, so there's not much point helping users improve them. But for tool builders depending on only Cabal-syntax or only Cabal-engine may have many advantages. OTOH, they are quite likely to become aware of the split just from changelogs or looking at the source, without any aggressive advertising. So I wouldn't worry.

@Mikolaj
Copy link
Member

Mikolaj commented Feb 16, 2022

A wizard of yore tells me

There is literally zero use cases for Cabal-engine alone. Except maybe something like
main = defaultMain
Most of Cabal-engine types are defined in Cabal-syntax

which makes me +0.5 on the shim and +0.5 on full re-export. If we want limited and mostly opt-in breakage in the short and in the long run, do we have any other options?

@bgamari
Copy link
Contributor

bgamari commented Feb 21, 2022

I completely understand the reluctance to reexport Cabal-syntax from Cabal. On the other hand, I'm quite worried about the impact of this change on users. One approach to mitigate this would be to introduce Cabal as a metapackage which reexports Cabal-syntax along with the current contents of Cabal, which would then live in a new package (e.g. Cabal-internal). This would avoid immediate breakage while preserving the ability of users to transition to Cabal-syntax if it is sufficient.

Does this sound like a plausible approach?

@gbaz
Copy link
Collaborator

gbaz commented Feb 21, 2022

I think at this point the actual cost of reexporting is not so great, and we probably should just do it. If someone could try to enumerate the disadvantages of doing so clearly, it would be good to see that all in one place so we can evaluate it.

@Mikolaj
Copy link
Member

Mikolaj commented Feb 21, 2022

@bgamari: what you propose looks similar to the shim idea discussed starting at #7974 (comment)

For the shim idea we agreed that reexporting seems preferable in this particular case, but if you have new arguments, let's restart the discussion.

@bgamari
Copy link
Contributor

bgamari commented Feb 21, 2022

@bgamari: what you propose looks similar to the shim idea discussed starting at #7974 (comment)

Ahh, apologies! Somehow I missed that comment. Yes, this is precisely what I was thinking. Sorry for the noise.

@Mikolaj
Copy link
Member

Mikolaj commented Feb 24, 2022

It seems nobody objects to the idea of (temporarily) re-exporting Cabal-syntax from Cabal. We talked at today's dev call and probably the easiest way is to re-export everything. We might also immediately warn in release notes that this shim is going to be removed at some point. We could also, at once or not, deprecate all the re-exported things with proper pragmas, somehow explaining that they are still available, but in Cabal-syntax.

Any last minute objections? Any refinements of the idea? Would anybody like to implement that after we wait for a few days for any objections?

@gbaz
Copy link
Collaborator

gbaz commented Feb 24, 2022

In writing up some notes on this, I just also came up with one more "evil trick" that we can consider -- we "simulate" flags by introducing a new "virtual flag" via a phantom dependency.

Create a new package, "cabal-syntax-split" that is empty and has two versions, call them 1 and 2. Force cabal-syntax to depend on version 2. Revise all pre-split versions of Cabal to depend on version 1. This resolves the issue raised by @phadej that

Which bound? The future Cabal-syntax-3.8 and e.g. Cabal-3.6 could exist in the same install plan. Cabal-syntax-3.8 doesn't have a dependency on Cabal. The build-depends: Cabal <3.7 in Cabal-syntax-3.6 is not sufficient.

which also I think was what motivated mpickering to raise this issue when he wrote that

My experience is that you can't just add a Cabal-syntax dependency as the solver is free to pick different Cabal and Cabal-syntax versions. Therefore you end up with plans which include Cabal-3.2.0.0 and Cabal-syntax-3.7 which export modules of the same name.

Sorry to raise this only just after our meeting, but it just only occurred to me.

@phadej
Copy link
Collaborator

phadej commented Feb 24, 2022

Revise all pre-split versions of Cabal to depend on version 1.

This is strictly worse (= new package) then just revising Cabal to depend on empty Cabal-syntax.

@gbaz
Copy link
Collaborator

gbaz commented Feb 24, 2022

Fair point. Why don't we just revise all prior cabals to depend on empty cabal-syntax then?

@Mikolaj
Copy link
Member

Mikolaj commented Feb 24, 2022

What if a sneaky cabal hides somewhere and we don't apprehend it and don't revise it?

E.g., perhaps some Hackage mirrors that people use have some extra versions of cabal? I can't easily imagine other failure scenarios but are we sure there are none?

@gbaz
Copy link
Collaborator

gbaz commented Feb 24, 2022

That seems like very much a corner case. (also I see @jneira suggested what I am suggesting above, and nobody responded).

@mpickering do you think that just making such revisions would suffice?

@jneira
Copy link
Member

jneira commented Feb 24, 2022

also I see @jneira suggested what I am suggesting above, and nobody responded.

I guess it was due to

oh, i forgot you cant add new dependencies via revisions :-/

which would need

upload new Cabal minor versions for all major versions we want to be compatible. Maybe prevent breakage worths it.

Or we could do those extraordinary revisions this time summoning some hackage demon :-P
Depending of a empty package will break no package so 🤷

@gbaz
Copy link
Collaborator

gbaz commented Feb 24, 2022

Right. In this case the hackage admins could certainly grant special ops to perform these revisions.

@phadej
Copy link
Collaborator

phadej commented Feb 24, 2022

upload new Cabal minor versions for all major versions we want to be compatible. Maybe prevent breakage worths it.

Doesn't fix anything. Unrevisioned versions exist, and they are a problem.

@phadej
Copy link
Collaborator

phadej commented Feb 24, 2022

Please remember what was the goal of Cabal-syntax split.

#7559 Which started from my comment #7548

That's the old problem of Cabal library being both the package description reading library as well as its interpration i.e. building. Former part barely changes (except of normal "let's make better library").

I'd welcome the split, as my tools use only that "parse .cabal file" part. Distribution.Simple namespace can be left for build-type: Custom packages and cabal-install use.

EDIT: even if the outer format were JSON or YAML, a library for working with it on higher level then "it's some jSON' still have to exist (c.f. cabal-plan library for plan.json files).

Note especially, that then (and now) I see the only use for Cabal library to be build-type: Custom scripts. (And a build-tools like cabal-install and maybe stack, which doesn't use Cabal library to build anything (it builds Setup script independently)

Exporting everything from Cabal is convenient. There is also no sign that Cabal and Cabal-syntax development will be decoupled (or/and release process considerably). (I tried to gauge whether Cabal and cabal-install development could be decoupled, it couldn't. Why Cabal and Cabal-syntax would be any different). And by decoupled I mean that in different repositories and potentially different people.

Also, I thought that Cabal-syntax would genuinely serve as a complete library, but as far as I can tell hackage-server is forced to use full Cabal still. (E.g. PackageDescription.Check functionality, which is a linter for cabal syntax files is in Cabal, Why? Would a linter for YAML-package-description also be in Cabal?) is still in Cabal. Also I need stuff which isn't in Cabal-syntax` (which is more on the edge).

So I expect that stuff will still be shuffled between Cabal and Cabal-syntax in the future, because the current split is just an educated guess. If it won't, then the child is born dead. And if the separation will be refined, then make it easier for everyone until we know better, and just reexport the modules, rather then keeping up to some "elegance" in package decouping. People who need Cabal will just use it, the ones who want to adopt Cabal-syntax will try to adopt and report issues. (I will, I expect them to be considered seriously, and not dismissed by simple reasons "But the module is not in Distribution.Types namespace).


As a person who put the Cabal-syntax thing moving, my expectation was that Cabal would only be used in build-type: Custom scripts and not anywhere else. For that reason reexporting modules is pragmatic solution. If you need Cabal you need Cabal-syntax types, etc. (Even if Cabal-YAML emerges, hopefully the GPD type would still be the same. Maybe further split to Cabal-types is justified, yet parser is quite as small part in the end).

If "Cabal devs" disagree, then I seriously suggest reverting, and figuring out first, why this change is made to begin with?.

Note: Cabal-syntax is still a quite heavy library.

@gbaz
Copy link
Collaborator

gbaz commented Feb 24, 2022

Oleg, respectfully, please chill out. I don't think anyone is that strongly in disagreement with you. I don't understand why you are being so combative, or what you think you are being combative against.

That said, I do see the argument that if we're expecting the dividing line to evolve in the future, then for now re-exports are just fine. Module re-exports are always somewhat irritating, and so there has been a reluctance to adopt them. We all agreed at the cabal advisors meeting that it was a pragmatic choice -- as mikolaj described earlier today!

I think it was good to have a full discussion about the design space of other alternatives. I think the revisions on hackage seem like a nice approach. I still don't understand why people are dismissive of it. But if dragging this discussion out will continue to cause such vitriol and angst, then lets just do the damn re-export, and reconsider later.

@jneira
Copy link
Member

jneira commented Feb 25, 2022

Agree we could wait to have a stable division to cut the umbilical cord. But with a concrete deprecation plan there are high probabilities of keeping the reexports forever. Almost same arguments (but the stabilization one) will be raised when trying to do.

Exporting everything from Cabal is convenient. There is also no sign that Cabal and Cabal-syntax development will be decoupled (or/and release process considerably). (I tried to gauge whether Cabal and cabal-install development could be decoupled, it couldn't. Why Cabal and Cabal-syntax would be any different)

The other way around also holds, coupling technically both packages would make more difficult decoupling them organizationally or having more than a cabal syntax so it will influence any decision about.

@Mikolaj
Copy link
Member

Mikolaj commented Feb 25, 2022

Amazing. So we have an agreement once again. :) Let me repeat myself:

It seems nobody objects to the idea of (temporarily) re-exporting Cabal-syntax from Cabal. We talked at today's dev call and probably the easiest way is to re-export everything. We might also immediately warn in release notes that this shim is going to be removed at some point. We could also, at once or not, deprecate all the re-exported things with proper pragmas, somehow explaining that they are still available, but in Cabal-syntax.

[...] Would anybody like to implement that after we wait for a few days for any objections?

@gbaz
Copy link
Collaborator

gbaz commented Feb 25, 2022

for those interested, i opened a discussion about how we might more generally be able to have a deprecation path for re-exported module shims ghc-proposals/ghc-proposals#489

@Mikolaj
Copy link
Member

Mikolaj commented Mar 21, 2022

Would it make sense to re-export now? We'd all have ample time to dogfood before the release...

@fgaz
Copy link
Member

fgaz commented Mar 22, 2022

I think we all agree to reexport, it's just that nobody made a pr yet

@bgamari
Copy link
Contributor

bgamari commented Mar 24, 2022

We discussed this in the advisors call. reexported-modules was introduced in GHC 7.10. We believe that we can narrow Cabal's support window to be lower-bounded by this release, allowing us to use reexported-modules to address this ticket.

@bgamari bgamari self-assigned this Mar 24, 2022
bgamari added a commit to bgamari/cabal that referenced this issue Mar 25, 2022
To preserve compatibility for downstream users Cabal will reexport all
of Cabal-syntax for the time being. In the future we may deprecate these
reexports.

Closes haskell#7974.
bgamari added a commit to bgamari/cabal that referenced this issue Mar 25, 2022
To preserve compatibility for downstream users Cabal will reexport all
of Cabal-syntax for the time being. In the future we may deprecate these
reexports.

This requires that we bump the cabal-version of Cabal.cabal to 1.22 and
drops support for GHC < 7.10.

Closes haskell#7974.
andreabedini pushed a commit to bgamari/cabal that referenced this issue Apr 8, 2022
To preserve compatibility for downstream users Cabal will reexport all
of Cabal-syntax for the time being. In the future we may deprecate these
reexports.

This requires that we bump the cabal-version of Cabal.cabal to 1.22 and
drops support for GHC < 7.10.

Closes haskell#7974.
@mergify mergify bot closed this as completed in #8064 Apr 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants