Fix predicate which determines whether to build a shared library

[mpickering]

Consider the which determines whether we should build shared libraries.

  pkgsUseSharedLibrary :: Set PackageId
  pkgsUseSharedLibrary =
    packagesWithLibDepsDownwardClosedProperty needsSharedLib
    where
      needsSharedLib pkg =
        fromMaybe
          compilerShouldUseSharedLibByDefault
          (liftM2 (||) pkgSharedLib pkgDynExe)
        where
          pkgid = packageId pkg
          pkgSharedLib = perPkgOptionMaybe pkgid packageConfigSharedLib
          pkgDynExe = perPkgOptionMaybe pkgid packageConfigDynExe

In English the intended logic is:

If we have enabled shared libraries or dynamic executables then we need to build a shared libary.

but, a common mistake:

(liftM2 (||) pkgSharedLib pkgDynExe)

instead says, if we explicitly request shared libraries and also explicitly configure whether we want dynamic executables then build a shared library.

It should instead use the monoid instance:

getMax <$> ((Max <$> pkgSharedLib) <> (Max <$> pkgDynExe))

which captures the original logic.

This failure is currently manifested in the way that if you write --disable-shared then --enable-shared is still passed to ./Setup.

If you pass both --disable-shared and --disable-executable-dynamic then you don't build a shared object.

Fixes #10050

Please read Github PR Conventions and then fill in one of these two templates.

---

Template Α: This PR modifies behaviour or interface

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.
• Manual QA notes have been included.
• Tests have been added. (Ask for help if you don’t know how to write them! Ask for an exemption if tests are too complex for too little coverage!)

[mpickering]

This condition is still not quite right, we don't want to disable shared library generation if only --disable-executable-dynamic is passed. I will fix this properly (with tests) in #9900

[ulysses4ever]

@mpickering should this be closed as superseeded by #9900 or is it still relevant?

[andreabedini]

Honestly, I find this unreadable. I don't think the Monoid instance for Max is quite natural here. Indeed you comment below suggests that it is not quite right.

[mpickering]

@ulysses4ever Yes I fixed the condition properly in #9900 which I deemed unnecessary to split into a different MR.