Force Cabal >= 1.24 dep when there's no custom-setup stanza.

[23Skidoo]

Fixes #3199.

To minimise the impact we should probably add default setup dependencies only to packages affected by #3199 (see @grayjay's snippet). Note, however, that the setup wrapper will pick Cabal >= 1.24 anyway if it is installed, since it prefers new Cabal versions.

Note also that cabal configure will still fail (unlike cabal install), though the error message will be more helpful than it currently is:

Resolving dependencies...
Warning: solver failed to find a solution:
Could not resolve dependencies:
trying: test-t3199-0.1.0.0 (user goal)
next goal: test-t3199-setup.Cabal (dependency of test-t3199-0.1.0.0)
rejecting: test-t3199-setup.Cabal-1.22.5.0/installed-cfe... (conflict:
test-t3199 => test-t3199-setup.Cabal>=1.24)
Dependency tree exhaustively searched.
Trying configure anyway.
Configuring test-t3199-0.1.0.0...

/cc @dcoutts

[23Skidoo]

I considered adding a test case, but it's a bit difficult given that you need to have a pre-1.24 version of Cabal installed for the bug to manifest.

[23Skidoo]

GHC 8 failure is due to GHC 8 RC having an old snapshot of Cabal 1.24. Ugh, wish we could ignore it somehow.

[ezyang]

In principle, cabal-install should know what version of Cabal it was built with, so it can demand that from the package database. But this won't work if you're distributing a statically linked Cabal binary, in that case you can't assume the Cabal 1.24 library is installed.

[23Skidoo]

But this won't work if you're distributing a statically linked Cabal binary

Yep. Some people also like to build cabal-install inside a sandbox and then copy the executable to ~/bin.

[ezyang]

I don't understand the approach this patch takes. Wasn't the original plan to only levy this implicit dependency if there was a buildable field?

[ezyang]

I think we want a test, if only to tell if the patch actually fixes the problem.

I think the easiest way to do this is to make the integration test GHC < 8.0 only. In these cases, we can assume GHC was distributed with an older version of Cabal. This will be sufficient for Travis.

[23Skidoo]

I don't understand the approach this patch takes. Wasn't the original plan to only levy this implicit dependency if there was a buildable field?

Sure, I can do that, just wanted to get some comments first. Though I think that forcing custom setup scripts to use the same Cabal version that cabal-install was compiled with is a good idea in general (provided that the user can force a different version manually via --use-cabal-version).

[ezyang]

So, is useDependenciesExclusive == True, noDefaultCabalDep == True an impossible state?

[23Skidoo]

useDependenciesExclusive == True

In that case noDefaultCabalDep is ignored.

[dcoutts]

I hope we can simplify the setup wrapper in future, the logic is getting really rather hairy. Fortunately the new-build code path only needs a fraction of the functionality, since it knows exactly what deps to use in advance, so doesn't need any of the guesswork that the setupWrapper currently does.

[ezyang]

CC @dcoutts I know Duncan has stated we want the logic for compiling custom setups to be as simple as possible, so other tools (like Stack) can also handle them; I don't know how far the wrong way this goes.

[ezyang]

Re the Travis failure, a minor version bump should do the trick, right?

[23Skidoo]

Actually I think I can get rid of noDefaultCabalDep.

Re the Travis failure, a minor version bump should do the trick, right?

Yes, I just didn't want the first 1.24 release to have a version number other than 1.24.0.0. Next time we create a release branch we should bump the version only just before the release.

[dcoutts]

@23Skidoo Unfortunately the first patch would break the use case in new-build. Have a look at:

cabal/cabal-install/Distribution/Client/ProjectPlanning.hs

Lines 1624 to 1668 in 1223a39

	defaultSetupDeps :: Platform -> PD.PackageDescription -> [Dependency]
	defaultSetupDeps platform pkg =
	case packageSetupScriptStylePreSolver pkg of
	-- For packages with build type custom that do not specify explicit
	-- setup dependencies, we add a dependency on Cabal and a number
	-- of other packages.
	SetupCustomImplicitDeps ->
	[ Dependency depPkgname anyVersion
	| depPkgname <- legacyCustomSetupPkgs platform ] ++
	-- The Cabal dep is slightly special:
	-- * we omit the dep for the Cabal lib itself (since it bootstraps),
	-- * we constrain it to be less than 1.23 since all packages
	-- relying on later Cabal spec versions are supposed to use
	-- explit setup deps. Having this constraint also allows later
	-- Cabal lib versions to make breaking API changes without breaking
	-- all old Setup.hs scripts.
	[ Dependency cabalPkgname cabalConstraint
	| packageName pkg /= cabalPkgname ]
	where
	cabalConstraint = orLaterVersion (PD.specVersion pkg)
	`intersectVersionRanges`
	earlierVersion cabalCompatMaxVer
	-- TODO/FIXME: turns out that constraining to less than 1.23 causes
	-- problems with GHC8 as there's too many important packages
	-- with Custom build-type, for which there wouldn't be any
	-- install-plan (as GHC8 requires Cabal-1.24+). So let's
	-- set an implicit upper bound `Cabal < 2` instead.
	cabalCompatMaxVer = Version [2] []
	-- For other build types (like Simple) if we still need to compile an
	-- external Setup.hs, it'll be one of the simple ones that only depends
	-- on Cabal and base.
	SetupNonCustomExternalLib ->
	[ Dependency cabalPkgname cabalConstraint
	, Dependency basePkgname anyVersion ]
	where
	cabalConstraint = orLaterVersion (PD.specVersion pkg)
	-- The internal setup wrapper method has no deps at all.
	SetupNonCustomInternalLib -> []
	SetupCustomExplicitDeps ->
	error $ "defaultSetupDeps: called for a package with explicit "
	++ "setup deps: " ++ display (packageId pkg)

There we apply our own deps for two cases, build-type custom with no explicit deps, but also non-custom when we need to build an external setup anyway.

So rather than changing how addDefaultSetupDependencies works in the solver, instead all you need to do is to change the function you pass to it, so that like defaultSetupDeps in the new-build code, you only return non-[] in the cases you're interested in, which in this case is only build-type custom with no explicit deps.

[dcoutts]

Ah, cunning. I went to some lengths in the new-build code to do this without changing the underlying type, but yes it is rather easier if we can just stash the info here. I might switch the new-build code over to use that, rather than keeping an extra Set around.

[dcoutts]

So this fixes things for packages without custom setup deps, of course we still have the problem for packages that do specify an upper bound we would still have trouble communicating to them.

But it's ok as a quick hack before the release I'd say (modulo the fixes I've suggested above), but it'd be good to try and make it possible to use older Cabal versions, since we do want to be able to have packages explicitly specify versions they work with (especially since we do want to start breaking the Cabal lib API to let us make bigger improvements). So we may need a proper fix to this in master at some point, and perhaps also consider making releases off of older branches that work with newer compiler versions.

I'll have a think about what a fix that did allow us to talk to the older ones here would look like, for the new-build code path, where we know a bit more up front about what we want to do with the setup. I take @grayjay's point about this being tricky in the solver since we need to know both what version of Cabal will be used for Setup, and based on that decide if we include the "unnecessary" deps of non-buildable components.

[hvr]

@dcoutts

of course we still have the problem for packages that do specify an upper bound we would still have trouble communicating to them

That's no different than a package specifying an upper bound on e.g. base. The solver ought to avoid such install-plans. Just as with ordinary upper bounds, we can introduce a --setup-allow-newer=Cabal flag...

perhaps also consider making releases off of older branches that work with newer compiler versions.

...but then cabal would also need a way to know that, for instance, the preinstalled Cabal-1.16.0 version that comes with GHC 7.6.3 is incompatible, but instead Cabal-1.16.0.1 is compatible. It's a much simpler model imho to assume that there's a cut-off version of Cabal lib versions, e.g. Cabal >= 1.18 which the current cabal-install is able to speak to.

@23Skidoo

Next time we create a release branch we should bump the version only just before the release.

that would cause problems for GHC dev (we request a bump-early-in-git policy especially for major ver bumps from bundled libraries, see https://ghc.haskell.org/trac/ghc/wiki/Commentary/Libraries/EagerVersionBump) and mask problems during RCs and/or cause annoying upper-bound-change-waves if last-minute upper bound changes occur :-/

[23Skidoo]

@hvr

Just as with ordinary upper bounds, we can introduce a --setup-allow-newer=Cabal flag...

--allow-newer already works for setup dependencies.

[23Skidoo]

PR updated:

1. Removed noDefaultCabalDep.
2. Removed the first patch.
3. Limited the scope only to packages potentially affected by Package with custom Setup.hs "Encountered missing dependencies" #3199.
4. Bumped Cabal version.
5. Added a test case.

@hvr

that would cause problems for GHC dev (we request a bump-early-in-git policy especially for major ver bumps from bundled libraries

Our versioning scheme (odd numbers == development, even == official releases) worked fine for GHC snapshots so far, I just want to extend it for release branches. So e.g. the 1.24 branch would continue to have the 1.23.x.y version right until the release. If reverse dependencies will specify the Cabal dep as >= 1.23.x.y && < 1.25, then there should be no problems, I think.

The invariant I want to enforce is "version of Cabal in the repo" > "version of Cabal that came with GHC we're compiling with".

[ezyang]

Blugh, copypasta!!

[23Skidoo]

The test case could be improved: right now we always install Cabal 1.24 before running the cabal-install test suite on Travis, so the failure can't manifest.

[ezyang]

What if we tell Cabal to not use the user database for these tests?

[23Skidoo]

Something like that should work.

[23Skidoo]

Will fix the remaining issues tomorrow.

[23Skidoo]

Updated the test case.

[23Skidoo]

Moved copy-pasted utilities into the Cabal library, improved the test case. I think that this is now ready to go in, will merge once the build bots are green.

[grayjay]

I forgot to mention that I added a case to my snippet:

cor  (CNot c)    d
  | c == d                   = Lit True

Without it, the function testing whether #3199 applies doesn't ignore this use of buildable:

executable exe
  if !flag(build-exe)
    buildable: False
  else
    build-depends: package
  [...]

[hvr]

indeed, it's kinda odd that symmetry was broken for the "x or (not x)" case...

[23Skidoo]

@grayjay Updated.