patch applied (hackage-server): "Fix Restore monad" and 15 others

devnull at devnull at
Thu Jan 31 16:54:21 CET 2013

Thu Jan 24 09:46:43 GMT 2013  Edsko de Vries <edsko at>
  * Fix Restore monad
  Ignore-this: d64b04545adb4af8c80b75c67f8c88ff
  The presence of an explicit RestoreBind constructor meant that the monad laws
  were violated.

    M ./Distribution/Server/Framework/BackupRestore.hs -23 +56

Thu Jan 24 10:04:01 GMT 2013  Edsko de Vries <edsko at>
  * Remove getStateSize from StateComponent 
  Ignore-this: fd113eddc80e2c866cdbe495b2669850
  (We can define it once and for all in AbstractStateComponent)

    M ./Distribution/Server/Features/BuildReports.hs -1
    M ./Distribution/Server/Features/Core.hs -1
    M ./Distribution/Server/Features/Distro.hs -1
    M ./Distribution/Server/Features/Documentation.hs -1
    M ./Distribution/Server/Features/DownloadCount.hs -1
    M ./Distribution/Server/Features/HaskellPlatform.hs -1
    M ./Distribution/Server/Features/Mirror.hs -1
    M ./Distribution/Server/Features/PackageCandidates.hs -1
    M ./Distribution/Server/Features/PreferredVersions.hs -1
    M ./Distribution/Server/Features/Tags.hs -1
    M ./Distribution/Server/Features/TarIndexCache.hs -9 +5
    M ./Distribution/Server/Features/Upload.hs -3
    M ./Distribution/Server/Features/Users.hs -2
    M ./Distribution/Server/Framework/Feature.hs -5 +5

Tue Jan 29 13:00:25 GMT 2013  Edsko de Vries <edsko at>
  * Disentangle htmlFeature (1/N)
  Ignore-this: 96e266f4cc4f60137ec4e253cf6427ae

    M ./Distribution/Server/Features/Html.hs -19 +35

Tue Jan 29 14:57:45 GMT 2013  Edsko de Vries <edsko at>
  * Disentangle htmlFeature (2/N)
  Ignore-this: 8acaf1b3887228308320996c59d1696a
  This splits htmlFeature into components for each feature, but so far have made
  no effort to actually *reduce* the dependencies between these various parts.

    M ./Distribution/Server/Features/Html.hs -322 +482

Tue Jan 29 15:24:07 GMT 2013  Edsko de Vries <edsko at>
  * Disentangle htmlFeature (3/N)
  Ignore-this: d1fae7bb76774b074eb078a560d3e324
  Make dependencies more explicit (don't use {..}) 

    M ./Distribution/Server/Features/Html.hs -18 +57

Tue Jan 29 16:35:41 GMT 2013  Edsko de Vries <edsko at>
  * Disentangle htmlFeature (4/N)
  Ignore-this: 2db8b7b27a1d0e85e7e2a603af893457
  Make the overlap between features more evident

    M ./Distribution/Server/Features/Html.hs -42 +51

Tue Jan 29 17:27:27 GMT 2013  Edsko de Vries <edsko at>
  * Start having PackageCandidates implement the CoreFeature interface
  Ignore-this: deaf61445bf67c6aecf0c63c322fa588
  The purpose is that features such as BuildReports or Documentation can then
  be instantiated by both Core and PackageCandidates.

    M ./Distribution/Server/Features/Core.hs -4 +4
    M ./Distribution/Server/Features/Html.hs -7 +6
    M ./Distribution/Server/Features/PackageCandidates.hs -42 +64

Wed Jan 30 09:43:00 GMT 2013  Edsko de Vries <edsko at>
  * Use "/.." instead of "*" in ServerIntrospect 
  Ignore-this: ac10101fcc3d3007da939d9e23d57221
  This brings it inline with what we actually use in the code.

    M ./Distribution/Server/Features/ServerIntrospect.hs -2 +2

Wed Jan 30 09:52:53 GMT 2013  Edsko de Vries <edsko at>
  * Document the documentation feature
  Ignore-this: afc7ae265cdf219e3e2b1a5c74f3e040

    M ./Distribution/Server/Features/Documentation.hs -7 +12

Wed Jan 30 12:41:25 GMT 2013  Edsko de Vries <edsko at>
  * Core cleanup (1/N)
  Ignore-this: 6a82aa13b3a3ce8e3a95e6172725dd7f
  Core had
    withPackageId   ::              DynamicPath -> (PackageId   -> ServerPartE a)   -> ServerPartE a
    withPackageName :: MonadIO m => DynamicPath -> (PackageName -> ServerPartT m a) -> ServerPartT m a
  But both of these are an instance of a much more general function
    withPackageInPath :: (MonadPlus m, FromReqURI a) => DynamicPath -> (a -> m b) -> m b
  So I have removed the first two in favour of the withPackageInPath. Moreover,
  withPackageInPath belongs in CoreResource more than in CoreFeature -- it
  doesn't access the package database, but simply extracts a bit from a URL.
  CoreFeature still contains a whole plethora of withXXX functions; the logical
  place for these is indeed in Feature rather than Resource, but we don't need
  quite all the different combinations; we can offer more general functions. Will
  do that next.
  The immediate reason for all this cleanup is that we want the PackageCandidates
  feature to ultimately implement the core API so that other packages (build
  reports, documentation, tags, mirror, versions, ..?) can work with either.

    M ./Distribution/Server/Features/Core.hs -3 +6
    M ./Distribution/Server/Features/Distro.hs -2 +4
    M ./Distribution/Server/Features/Documentation.hs -2 +2
    M ./Distribution/Server/Features/Html.hs -26 +22
    M ./Distribution/Server/Features/Mirror.hs -4 +12
    M ./Distribution/Server/Features/PackageCandidates.hs -5 +10
    M ./Distribution/Server/Features/PreferredVersions.hs -3 +12
    M ./hackage-server.cabal +1

Thu Jan 31 09:07:44 GMT 2013  edsko at
  * Start removing unnecessary withXXX style
  Ignore-this: 398ce12fc151ed4e646efeb5bbdfb741
  Since these functions don't do any resource management, and live in a MonadPlus
  anyway, there is no need for withXXX style which just adds clutter.

    M ./Distribution/Server/Features/Core.hs -2 +4
    M ./Distribution/Server/Features/Distro.hs -3 +3
    M ./Distribution/Server/Features/Html.hs -77 +75
    M ./Distribution/Server/Features/Mirror.hs -32 +32
    M ./Distribution/Server/Features/PackageCandidates.hs -21 +24
    M ./Distribution/Server/Features/PreferredVersions.hs -4 +4
    M ./hackage-server.cabal -1 +1

Thu Jan 31 13:23:48 GMT 2013  edsko at
  * Further cleanup of Core
  Ignore-this: 1562fbab4220ee28f3381fdee1437d9a
  Core had this whole plethora of functions:
    withPackage            :: PackageId -> (PkgInfo -> [PkgInfo] -> ServerPartE a) -> ServerPartE a
    withPackagePath        :: DynamicPath -> (PkgInfo -> [PkgInfo] -> ServerPartE a) -> ServerPartE a
    withPackageAll         :: PackageName -> ([PkgInfo] -> ServerPartE a) -> ServerPartE a
    withPackageAllPath     :: DynamicPath -> (PackageName -> [PkgInfo] -> ServerPartE a) -> ServerPartE a
    withPackageVersion     :: PackageId   -> (PkgInfo   -> ServerPartE a) -> ServerPartE a
    withPackageVersionPath :: DynamicPath -> (PkgInfo   -> ServerPartE a) -> ServerPartE a
    withPackageTarball     :: DynamicPath -> (PackageId -> ServerPartE a) -> ServerPartE a
  The withXXX style of these functions is unnecessary; it's unclear what
  precisely these functions do, the names don't make it obvious; and there are
  too many combinations.
  In a previous we paved the way for addressing the third concern by introducing
      packageInPath :: (MonadPlus m, FromReqURI a) => DynamicPath -> m a
  We now added
      packageTarballInPath :: MonadPlus m => DynamicPath -> m PackageId
  replacing withPackageTarball (although it still feels a little adhoc). The other
  6 functions have been replaced with two:
    lookupPackageName :: PackageName -> ServerPartE [PkgInfo]
    lookupPackageId   :: PackageId   -> ServerPartE PkgInfo
  The first finds all versions of a package, while the second finds a specific
  version of a package, or the most recent if the version in the given identifier
  is empty.
  The purpose of both functions should hopefully be evident from their name, and
  obviously we got rid of the withXXX style.
    withPackage returned all packages with the given name, and the most recent
    version of that package if the version of the package id was Version [] [] or
    the specific version otherwise. This function was never called directly, but
    only through withPackagePath.
    This is a combination of withPackage and withPackageId, which is now
    BEFORE                                      AFTER
    withPackagePath dpath $ \_ pkgs ->          pkgs <- packageInPath dpath >>= lookupPackage . pkgName
    withPackagePath dpath $ \pkg _ ->           pkg <- packageInPath dpath >>= lookupPackageId
    (It was only used in one of these two ways, with one argument always ignored,
    and the first form occurred only once.) 
    This is now lookupPackageName. (Although it was often called like
    BEFORE                                      AFTER
    withPackageAll pkgname $ \_ ->              _ <- lookupPackageName pkgname
    We might want to replace that with something like checkPackageExists.
    This used withPackageName (now packageInPath) and returned the package name
    as well as the packages themselves.
    BEFORE                                      AFTER
    withPackageAllPath dpath $ \pkgname pkgs    pkgname <- packageInPath dpath
                                                pkgs <- lookupPackageName pkgname
    withPackageAllPath dpath $ \pkgname _ ->    pkgname <- packageInPath dpath
                                                _ <- lookupPackageName pkgname
    The second form occurred surprisingly often; I've marked these calls to
    lookupPackageName as "TODO: Necessary?" because it wasn't obvious to me
    if (all) of these calls were really required, or whether it was just a
    "default" option to use withPackageAllPath rather than (what used to be)
    This is like lookupPackageId, but explicitly ruled out empty versions.
    BEFORE                                      AFTER
    withPackageVersion pkgid $ \pkg ->          guard (pkgVersion pkgid /= Version [] [])
                                                pkg <- lookupPackageId pkgid
    (This occurred only once.)
    Combination of withPackageId (now packageInPath) and withPackageVersion,
    which was *only* used in the BuildReports module.
    BEFORE                                      AFTER
    withPackageVersionPath dpath $ \pkg ->      pkgid <- packageInPath dpath
                                                guard (pkgVersion pkgid /= Version [] [])
                                                pkg <- lookupPackageId pkgid
    Although the latter is a bit more verbose, it's also a bit more explicit
    about what's happening, and in many cases the original code contained a
    subsequent line
    let pkgid = pkgInfoId pkg
    anyway (in which case the 'pkg' itself was ignored; I've left in the calls to
    lookupPackageId regardless, as a check that the package does in fact exist.
    I'm not 100% all of these are necessary.)

    M ./Distribution/Server/Features/BuildReports.hs -38 +45
    M ./Distribution/Server/Features/Core.hs -83 +57
    M ./Distribution/Server/Features/Documentation.hs -26 +27
    M ./Distribution/Server/Features/Html.hs -54 +54
    M ./Distribution/Server/Features/Mirror.hs -39 +42
    M ./Distribution/Server/Features/PackageCandidates.hs -5 +4
    M ./Distribution/Server/Features/PackageContents.hs -3 +7
    M ./Distribution/Server/Features/PreferredVersions.hs -25 +27
    M ./Distribution/Server/Features/Tags.hs -14 +17

Thu Jan 31 15:23:31 GMT 2013  edsko at
  * Add guardValidPackageXXX
  Ignore-this: ba9516f019aed244dfd06e4c7c4624d8
  In our relentness quest to bring Core and Candidates closer together, add
  guardValidPackageId and guardValidPackageName to CoreResource. Many features
  just need to check if a package ID is valid, rather than get access to the
  actual package (BuildReports being one example). This means that these features
  can easily work with either Core or Candidates (which, after all, use different
  types of packages).
  This also addresses the issue mentioned in the previous log, where we used the
    _ <- lookupPackageXXX
  to check that a package exists. 

    M ./Distribution/Server/Features.hs -2 +2
    M ./Distribution/Server/Features/BuildReports.hs -24 +13
    M ./Distribution/Server/Features/Core.hs -1 +12
    M ./Distribution/Server/Features/Html.hs -6 +4
    M ./Distribution/Server/Features/PreferredVersions.hs -7 +9
    M ./Distribution/Server/Features/Tags.hs -2 +2

Thu Jan 31 15:32:04 GMT 2013  edsko at
  * Remove withXXX from BuildReports
  Ignore-this: bd91f01277dffdde57ef3623dd7bf87d
  .. while we were here anyway ..

    M ./Distribution/Server/Features/BuildReports.hs -40 +38

Thu Jan 31 15:45:49 GMT 2013  edsko at
  * Remove withXXX style from Upload
  Ignore-this: 691782ddc84077e053242ff5ec121bec
  In preparation for cleaning up Candidates, still.

    M ./Distribution/Server/Features/Documentation.hs -12 +12
    M ./Distribution/Server/Features/Html.hs -20 +18
    M ./Distribution/Server/Features/PackageCandidates.hs -4 +4
    M ./Distribution/Server/Features/PreferredVersions.hs -52 +52
    M ./Distribution/Server/Features/Upload.hs -29 +30

Thu Jan 31 15:50:14 GMT 2013  edsko at
  * Documentation now relies on CoreResource only
  Ignore-this: 9377ef7f88fc721963161d8eddb0d3d0
  This means that BuildReports and Documentation should now (in principle) be
  able to work with Core as well as Candidates.

    M ./Distribution/Server/Features.hs -1 +1
    M ./Distribution/Server/Features/Documentation.hs -9 +7

More information about the cabal-devel mailing list