hackage-trustees/hackage-cli

`hackage-cli add-bound` should skip cabal files that already have the bound

Closed this issue · 1 comments

Context: haskell-infra/hackage-trustees#329 (comment)

A workflow for bulk-adding a bound, like base < 4.16 to a hackage-published package, e.g. warp, could be:

hackage-cli pull-cabal warp
hackage-cli add-bound base '<4.16' *.cabal
hackage-cli push-cabal --incr-rev [--publish] *.cabal

Unfortunately, this does not work out-of-the-box to the full satisfaction, because:

  1. add-bound will add happily a bound that is subsumed already.
  2. push-cabal will happily publish a new revision that is extensionally equal to the latest publication, e.g. when just the syntax of constraints changes while preserving their semantics.

We would get revisions like:

Pushing "warp-3.3.5.cabal" (warp-3.3.5~1) [review-mode] ...
  
 * Changed the library component&#39;s library dependency on &#39;base&#39;
                          from `<4.16 && >=4.8 && <5`
                          to `(<4.16 && <4.16) && >=4.8 && <5`

(In <4.16 && >=4.8 && <5 you can see the redundant representation of a constraint generated by a previous run of add-bound.)

While I think 2. (push-cabal) is fine, something could be done about 1. (add-bound).
add-bound could make some effort to discover that the new bound is subsumed by the old constraints (not that hard really), and in this case leave the .cabal file untouched. This smart behavior could maybe be overridden by a flag --force that will add a bound even if its subsumed (to be conservative, not sure there are frequent applications of this).

Currently add-bound has a sanity check whether the added bound has been incorporated correctly into the existing bound, but this check is entirely syntactic:

hackage-cli/src/Main.hs

Lines 802 to 811 in feb17dd

-- sanity check
let oldGpd = parseGenericPackageDescription' old
newGpd = parseGenericPackageDescription' new
oldRange = extractRange oldGpd optABPackageName
newRange = extractRange newGpd optABPackageName
oldRange' = C.intersectVersionRanges oldRange optABVersionRange
unless (C.toVersionIntervals newRange == C.toVersionIntervals oldRange') $

Note that intersectVersionRanges is literally &&, nothing semantic: https://hackage.haskell.org/package/Cabal-3.6.2.0/docs/src/Distribution.Types.VersionRange.Internal.html#intersectVersionRanges
A canonical representation of version ranges are lists of non-overlappingVersionIntervals, ordered ascendingly, see https://hackage.haskell.org/package/Cabal-3.6.2.0/docs/Distribution-Types-VersionInterval.html#v:asVersionIntervals (Cabal 3.6) or https://hackage.haskell.org/package/Cabal-2.4.0.0/docs/Distribution-Types-VersionInterval.html#v:asVersionIntervals
(Cabal 2.4).