Solver: Detect cycles between packages and their setup scripts (fixes #4161).

[grayjay]

The solver already detected cycles involving more than one package, but it
allowed dependencies between components within a package. This commit treats a
dependency between a package's setup script and library as a cycle in order to
allow the solver to backtrack and try to break the cycle. A more thorough
solution would involve tracking all dependencies between components, as in #4087.

This commit also fixes the internal error in issue #4980.

Please include the following checklist in your PR:

• Patches conform to the coding conventions.
• Any changes that could be relevant to users have been recorded in the changelog.
• The documentation has been updated, if necessary.
• If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

Please also shortly describe how you tested your change. Bonus points for added tests!

/cc @kosmikus @hvr

[hvr]

@grayjay so in the case of the example in #4980, what does "breaking the cycle" mean? that it uses whatever binary version is in the global package db for binary-0.4's custom setup (which is a bit of a pathological example, as its Setup.hs is a trivial main = defaultMain setup)?

This makes me wonder a bit: What would happen if binary-0.4 didn't have a src/ folder, but instead Data/Binary.hs was in the top-level folder, and Setup.hs would import Data.Binary; would the local Data.Binary take precedence or the installed binary's Data.Binary?

[grayjay]

This change just causes the solver to backtrack when a package's setup script depends on the same instance of the package. The solver could choose any other solution, such as using an installed version of the package for the setup script or avoiding the package entirely. When I ran it on the example in #4980, it used binary-0.8.2.1, which has build-type: Simple:

[__0] trying: cabal-bug-0 (user goal)
[__1] trying: text-1.2.3.0 (dependency of cabal-bug)
[__2] trying: base-4.3.1.0/installed-d22... (dependency of text)
[__3] trying: rts-1.0/installedbuil... (dependency of base)
[__4] trying: ffi-1.0/installedbuil... (dependency of rts)
[__5] trying: integer-gmp-0.2.0.3/installed-298... (dependency of base)
[__6] trying: text:!test
[__7] trying: bytestring-0.9.1.10/installed-6aa... (dependency of text +/-bytestring-builder)
[__8] trying: ghc-prim-0.2.0.0/installed-d9d... (dependency of text)
[__9] next goal: deepseq (dependency of text)
[__9] rejecting: deepseq-1.4.3.0 (conflict: base==4.3.1.0/installed-d22..., deepseq => base>=4.5 && <4.11)
[__9] trying: deepseq-1.4.2.0
[_10] trying: array-0.3.0.2/installed-143... (dependency of text)
[_11] next goal: binary (dependency of cabal-bug)
[_11] rejecting: binary-0.8.5.1, binary-0.8.5.0, binary-0.8.4.1, binary-0.8.4.0, binary-0.8.3.0, binary-0.8.2.1, binary-0.8.2.0, binary-0.8.1.0, binary-0.8.0.1, binary-0.8.0.0, binary-0.7.6.1, binary-0.7.6.0, binary-0.7.5.0, binary-0.7.4.0, binary-0.7.3.0, binary-0.7.2.3, binary-0.7.2.2, binary-0.7.2.1, binary-0.7.2.0, binary-0.7.1.0, binary-0.7.0.1, binary-0.7.0.0, binary-0.6.4.0, binary-0.6.3.0, binary-0.6.2.0, binary-0.6.1.0, binary-0.6.0.0, binary-0.5.1.1, binary-0.5.1.0, binary-0.5.0.2, binary-0.5.0.1, binary-0.5, binary-0.4.5, binary-0.4.4, binary-0.4.3.1, binary-0.4.3, binary-0.4.2, binary-0.4.1 (conflict: cabal-bug => binary==0.4)
[_11] trying: binary-0.4
[_12] trying: binary:setup.base~>base-4.3.1.0/installed-d22... (dependency of binary)
[_13] trying: containers-0.4.0.0/installed-b48... (dependency of binary)
[_14] trying: deepseq:!test
[_15] trying: text:-integer-simple
[_16] trying: text:-developer
[_17] rejecting: text:-bytestring-builder (conflict: bytestring==0.9.1.10/installed-6aa..., text -bytestring-builder => bytestring>=0.10.4)
[_17] trying: text:+bytestring-builder
[_18] trying: bytestring-builder-0.10.8.1.0 (dependency of text +bytestring-builder)
[_19] rejecting: bytestring-builder:+bytestring_has_builder (conflict: bytestring==0.9.1.10/installed-6aa..., bytestring-builder +bytestring_has_builder => bytestring>=0.10.4)
[_19] trying: bytestring-builder:-bytestring_has_builder
[_20] trying: binary:setup.rts~>rts-1.0/installedbuil... (dependency of binary:setup.base)
[_21] trying: binary:setup.ffi~>ffi-1.0/installedbuil... (dependency of binary:setup.rts)
[_22] trying: binary:setup.integer-gmp~>integer-gmp-0.2.0.3/installed-298... (dependency of binary:setup.base)
[_23] next goal: binary:setup.Cabal (dependency of binary)
[_23] rejecting: binary:setup.Cabal-1.10.2.0/installed-9af... (constraint from minimum version of Cabal used by Setup.hs requires >=1.20)
[_23] rejecting: binary:setup.Cabal-2.0.1.1, binary:setup.Cabal-2.0.1.0, binary:setup.Cabal-2.0.0.2 (conflict: binary => binary:setup.Cabal>=0 && <1.25)
[_23] rejecting: binary:setup.Cabal-1.24.2.0, binary:setup.Cabal-1.24.0.0 (conflict: binary:setup.base==4.3.1.0/installed-d22..., binary:setup.Cabal => binary:setup.base>=4.5 && <5)
[_23] rejecting: binary:setup.Cabal-1.22.8.0, binary:setup.Cabal-1.22.7.0, binary:setup.Cabal-1.22.6.0, binary:setup.Cabal-1.22.5.0, binary:setup.Cabal-1.22.4.0, binary:setup.Cabal-1.22.3.0, binary:setup.Cabal-1.22.2.0, binary:setup.Cabal-1.22.1.1, binary:setup.Cabal-1.22.1.0, binary:setup.Cabal-1.22.0.0 (conflict: binary:setup.base==4.3.1.0/installed-d22..., binary:setup.Cabal => binary:setup.base>=4.4 && <5)
[_23] trying: binary:setup.Cabal-1.20.0.4
[_24] trying: binary:setup.Cabal:!test
[_25] trying: binary:setup.template-haskell-2.5.0.0/installed-957... (dependency of binary)
[_26] trying: binary:setup.ghc-prim~>ghc-prim-0.2.0.0/installed-d9d... (dependency of binary)
[_27] trying: binary:setup.unix-2.4.2.0/installed-58b... (dependency of binary)
[_28] trying: binary:setup.transformers-0.5.5.0 (dependency of binary)
[_29] trying: binary:setup.time-1.2.0.3/installed-349... (dependency of binary)
[_30] trying: binary:setup.old-locale-1.0.0.2/installed-161... (dependency of binary:setup.time)
[_31] trying: binary:setup.process-1.0.1.5/installed-da4... (dependency of binary)
[_32] trying: binary:setup.pretty-1.0.1.2/installed-f8d... (dependency of binary)
[_33] trying: binary:setup.old-time-1.0.0.6/installed-7f1... (dependency of binary)
[_34] trying: binary:setup.filepath-1.2.0.0/installed-b4f... (dependency of binary)
[_35] trying: binary:setup.directory-1.1.0.0/installed-393... (dependency of binary)
[_36] next goal: binary:setup.deepseq (dependency of binary)
[_36] rejecting: binary:setup.deepseq~>deepseq-1.4.2.0, binary:setup.deepseq-1.4.3.0 (conflict: binary:setup.Cabal => binary:setup.deepseq>=1.3 && <1.4)
[_36] rejecting: binary:setup.deepseq-1.4.2.0 (multiple instances)
[_36] rejecting: binary:setup.deepseq-1.4.1.2, binary:setup.deepseq-1.4.1.1, binary:setup.deepseq-1.4.1.0, binary:setup.deepseq-1.4.0.0 (conflict: binary:setup.Cabal => binary:setup.deepseq>=1.3 && <1.4)
[_36] trying: binary:setup.deepseq-1.3.0.2
[_37] trying: binary:setup.containers~>containers-0.4.0.0/installed-b48... (dependency of binary)
[_38] trying: binary:setup.bytestring~>bytestring-0.9.1.10/installed-6aa... (dependency of binary)
[_39] next goal: binary:setup.binary (dependency of binary)
[_39] rejecting: binary:setup.binary~>binary-0.4 (cyclic dependencies; conflict set: binary:setup.binary)
[_39] rejecting: binary:setup.binary-0.8.5.1, binary:setup.binary-0.8.5.0, binary:setup.binary-0.8.4.1, binary:setup.binary-0.8.4.0 (conflict: binary:setup.base==4.3.1.0/installed-d22..., binary:setup.binary => binary:setup.base>=4.5.0.0 && <5)
[_39] rejecting: binary:setup.binary-0.8.3.0 (conflict: binary:setup.base==4.3.1.0/installed-d22..., binary:setup.binary => binary:setup.base>=4.5 && <5)
[_39] trying: binary:setup.binary-0.8.2.1
[_40] trying: binary:setup.binary:!bench
[_41] trying: binary:setup.binary:!test
[_42] next goal: binary:setup.array (dependency of binary)
[_42] trying: binary:setup.array~>array-0.3.0.2/installed-143...
[_43] done

The choice doesn't really make sense in this case, but I think it's still an improvement over choosing the same instance. Maybe the solver should prevent packages from directly depending on any version of the same package, to avoid the ambiguity that you mentioned. We would also need to filter the default setup dependencies added by new-build. I think we would still need to allow indirect dependencies on the same package; for example, Cabal depends on time, and some versions of time have custom build type.

I tried copying Data/Binary.hs to the top-level directory, but I wasn't sure how to make cabal use it for the setup script, even if I renamed it to not conflict with the existing binary module.

[grayjay]

I think I found a test-only bug that is causing the solver QuickCheck tests to time out. I'll try to make a PR this weekend.

[grayjay]

I made a PR to fix the QuickCheck failures: #5035