NixOS / jailbreak-cabal

Strip version restrictions from build dependencies in Cabal files.
http://hackage.haskell.org/package/jailbreak-cabal
BSD 3-Clause "New" or "Revised" License
10 stars 11 forks source link

Does not lift bounds in conditional branch. #7

Closed cstrahan closed 9 years ago

cstrahan commented 9 years ago

It appears jailbreak-cabal doesn't remove the bounds for dependencies listed under a conditional branch. For example, jailbreak-cabal does not lift the bounds on template-haskell in haskell-src-meta.cabal. For reference:

name:               haskell-src-meta
version:            0.6.0.8
x-revision: 1
cabal-version:      >= 1.6
build-type:         Simple
license:            BSD3
license-file:       LICENSE
category:           Language, Template Haskell
author:             Matt Morrow
copyright:          (c) Matt Morrow
maintainer:         Ben Millwood <haskell@benmachine.co.uk>
bug-reports:        https://github.com/bmillwood/haskell-src-meta/issues
stability:          experimental
tested-with:        GHC == 7.0.4, GHC == 7.2.2, GHC == 7.4.2, GHC == 7.6.2
synopsis:           Parse source to template-haskell abstract syntax.
description:        The translation from haskell-src-exts abstract syntax
                    to template-haskell abstract syntax isn't 100% complete yet.

extra-source-files: ChangeLog README examples/*.hs

library
  build-depends:   base >= 4.2 && < 4.8,
                   haskell-src-exts == 1.16.*,
                   pretty >= 1.0 && < 1.2,
                   syb >= 0.1 && < 0.5,
                   th-orphans >= 0.5 && < 0.12

  if impl(ghc >= 7.4)
    Build-depends: template-haskell >= 2.7 && < 2.10
  else
    Build-depends: template-haskell >= 2.4 && < 2.7,
                   uniplate >= 1.3 && < 1.7

  extensions:      CPP,
                   RankNTypes,
                   StandaloneDeriving,
                   TemplateHaskell,
                   TypeSynonymInstances,
                   FlexibleContexts,
                   FlexibleInstances,
                   DeriveDataTypeable,
                   PatternGuards
  hs-source-dirs:  src
  exposed-modules: Language.Haskell.Meta
                   Language.Haskell.Meta.Parse
                   Language.Haskell.Meta.Parse.Careful
                   Language.Haskell.Meta.Syntax.Translate
                   Language.Haskell.TH.Instances.Lift
                   Language.Haskell.Meta.Utils

source-repository head
  type:     git
  location: git://github.com/bmillwood/haskell-src-meta.git

(https://hackage.haskell.org/package/haskell-src-meta-0.6.0.8/revision/1.cabal)

peti commented 9 years ago

The doesn't touch those conditionals, because they are often used to let Cabal deduce some flag setting based on the available environment, and that mechanism would be broken without the version constraints. In case of haskell-src-meta, this is unfortunate, because jailbreak-cabal effectively doesn't work for that library, but if we'd change the behavior, then in turn we'd break many other builds that currently do succeed after jailbreak-cabal has been run.

In fact, we did process conditionals but ended up reverting the change in https://github.com/peti/jailbreak-cabal/commit/99eac40deb481b185fd93fd307625369ff5e1ec0. Then the subject came up once more in https://github.com/peti/jailbreak-cabal/pull/4. @ryantrinkle and I discussed the subject in e-mail (which I hope he won't mind me citing here):

I think that stripping version bounds even within conditionals is probably the right thing to do.

It would probably be best to approach this question empirically. One one hand, there is a potential gain because the extended jailbreak will fix some packages that it otherwise wouldn't have, and those packages would have required some manually written patch phase to get the constraints out.

On the other hand, the extended jailbreak may break some builds that succeed fine today, with the normal jailbreak, because Cabal gets confused by alternative configure paths that don't seem to differ. These kind of failures are hard'ish to detect (because the build may be subtly incorrect instead of failing outright), and fixing them requires manual effort as well.

The big question is: which of the two alternatives is more likely to occur?

Personally, I feel like I cannnot guess the outcome. IMHO, we'd have to measure that somehow.

Some packages use flags to select different versions of dependencies; however, we have control over both what flags and what dependencies the package sees. It's rarely the case that Cabal is actually making any decisions about which version of a dependency to use - which means that if we get it wrong, it's going to fail regardless. So, I'm not sure that leaving conditionals alone is buying us much.

I'm not sure whether that's accurate. Some Cabal file define CPP flags based on the version of a build inputs. Consider the following (contrived) example:

flag oldTime
  Default:       True

if flag(oldTime)
  build-depends:  time < 1.5
  cpp-options: -DOLDTIME
else
  build-depends:  time >= 1.5

I believe that Jailbreak would screw that decision up by removing the version constraints.

cstrahan commented 9 years ago

I see. Thanks for the explanation :).

tobiasBora commented 4 years ago

Actually, would it be possible to provide maybe a second command to force this behavior also in conditional branches? I understand the issues with putting that option by default, but it could be nice to have a way to force this behavior. Indeed sometimes I want to lift bounds in a package, and I don't care to explicitly specify a set of arguments. However I can't use doJailbreak because of these conditional branches, so I need to manually patch the file. For example, I'd like to install in nix Hakyll, and even if I manually specify the configure flags, it fails to compile due to pandoc version. So I'd love to be able to avoid to write a patch file:

# hpkgs.nix

{ compiler ? "ghc884"
, pkgs
}:

let
  inherit (pkgs.lib.trivial) flip pipe;
  inherit (pkgs.haskell.lib) appendPatch appendConfigureFlags;

  hakyllFlags = [ "-f" "watchServer" "-f" "previewServer" ];

  haskellPackages = pkgs.haskell.packages.${compiler}.override {
    overrides = hpNew: hpOld: {
      hakyll =
        # pipe is used to apply all the following rules
        # sequentially in order to override the initial hpOld.hakyll derivation
        pipe
          hpOld.hakyll
          [
            # We force the above flags, since cabal may silently disable them if a dependency is missing
            (flip appendConfigureFlags hakyllFlags)
            pkgs.haskell.lib.unmarkBroken
            # I'd like to use a "forced" version (since I already provide the configure flags) of:
            pkgs.haskell.lib.doJailbreak
            # What I need to use (a patch):
            # (flip appendPatch ./hakyll.patch)
          ];
    };
  };
in haskellPackages

Moreover, the condition here just depends on some parameter enabled or not, which is pretty common I guess:

  If flag(usePandoc)
    Exposed-Modules:
      Hakyll.Web.Pandoc
      Hakyll.Web.Pandoc.Biblio
      Hakyll.Web.Pandoc.FileType
    Other-Modules:
      Hakyll.Web.Pandoc.Binary
    Build-Depends:
      pandoc          >= 2.10     && < 2.11,
      pandoc-citeproc >= 0.14     && < 0.18
    Cpp-options:
      -DUSE_PANDOC