Fix preferred-versions format#66
Conversation
|
Thank you! @michaelpj @yvan-sraka can you review (and merge)? It's public holiday on Monday. @amesgen do we have already wrongly formatted preferred-versions on chap? If yes, this means we need to reset the index (😥) as the fix will change old entries. |
No, CHaP does not yet have any deprecations, e.g. this returns no entries: curl -sL https://input-output-hk.github.io/cardano-haskell-packages/01-index.tar.gz \
| tar tzf - | rg preferred-versions |
andreabedini
left a comment
andreabedini
left a comment
There was a problem hiding this comment.
I suggest we pretty-print it as a Dependency. Would you be able to check if this works correctly? (since you already have the setup to reproduce the problem :P)
app/Foliage/CmdBuild.hs
Outdated
| createTarEntry (ts, effectiveRange) = mkTarEntry prettyEffectiveRange (IndexPkgPrefs pn) ts | ||
| where | ||
| -- We have to prepend the package name despite this file residing | ||
| -- in a package-specific directory. | ||
| prettyEffectiveRange = | ||
| BL.pack $ unPackageName pn <> " " <> prettyShow effectiveRange |
There was a problem hiding this comment.
Since you tracked down that cabal parses this as a Dependency I suggest we pretty-print it as a Dependency too, so to avoid any incompatibility.
| createTarEntry (ts, effectiveRange) = mkTarEntry prettyEffectiveRange (IndexPkgPrefs pn) ts | |
| where | |
| -- We have to prepend the package name despite this file residing | |
| -- in a package-specific directory. | |
| prettyEffectiveRange = | |
| BL.pack $ unPackageName pn <> " " <> prettyShow effectiveRange | |
| createTarEntry (ts, effectiveRange) = mkTarEntry (BL.pack $ prettyShow dep) (IndexPkgPrefs pn) ts | |
| where | |
| dep = mkDependency pn effectiveRange mainLibSet |
Where mkDependency and mainLibSet are defined in Distribution.Types.Dependency.
There was a problem hiding this comment.
Yeah, that makes sense; I still find Dependency a bit misleading in this specific context as sublibraries are not applicable here (Cabal will simply ignore them, see here), but I will just add a comment 👍
There was a problem hiding this comment.
Also confirming that this works just as well as the previous approach
There was a problem hiding this comment.
Yeah, that makes sense; I still find
Dependencya bit misleading in this specific context as sublibraries are not applicable here
100% agree with you. This also made me just realise per-component solving won't work with deprecated-packages OOTB (since they only deprecate the main lib and the solver could still pick some other lib). This should be reworked in cabal-install ... (one day 🙄)
Co-authored-by: Andrea Bedini <andrea@andreabedini.com>
|
Note for future ourselves (and new visitors 👋). A deprecated package is avoided if possible but it will still be choosen if it's the only solution. Consider the following setup: With this PR, foliage produces the correct but then cabal-install picks tagged-0.8.6.1 anyway! This because If we add cabal-install is not picking the latest version of tagged anymore, because it's now deprecated. Setting index-state to just before the deprecation makes tagged-0.8.6.1 available again. |
|
Thank you @amesgen <3 |
Bump foliage to include fix for wrong formatting of preferred-versions. See input-output-hk/foliage#66
Bump foliage to include fix for wrong formatting of preferred-versions. See input-output-hk/foliage#66
Bump foliage to include fix for wrong formatting of preferred-versions. See input-output-hk/foliage#66
2 participants
Cabal can not parse the
preferred-versionsformat as written by foliage before this PR as the preceding package name is missing, so all deprecations were ignored before this PR. E.g. themtl/preferred-versionsfile from the Hackage index looks like this:Relevant links to the Cabal source code:
preferred-versionsfiles are parsed here. It is a line-wise format with simple comments, where lines are parsed via theParsecinstance forDependency.parsePreferredVersions, which silently ignores parse errors. This could be reported upstream, especially as warnings are emitted forRepoLocalNoIndex.I used this code to create this repo (src) for haskell/cabal#8997; this can be used to verify that deprecations now work with this PR.