Directly call in-library functions to build packages

[sheaf]

Template Β: This PR does not modify cabal behaviour (documentation, tests, refactoring, etc.)

This PR modifies cabal-install to directly go through the Cabal library when building packages, instead of the Setup interface (except of course for build-type: Custom packages).

Overview of changes:

• Addition of LibraryMethod to SetupMethod(*), in which we directly invoke the Cabal configure, build, ... functions.
  • This required a bit of GADT trickery to accomodate the fact that configure returns a LocalBuildInfo which must then be passed to subsequent phases, while with the old Setup interface everything returns IO () and communication is done through the filesystem (e.g. the local build info file).
• New library hooks-exe, provisioning:
  • CallHooksExe: the API for communicating with an external hooks executable (cabal-install depends on this)
  • HooksExe: the necessary functions for creating a hooks executable (cabal-install adds a dependency on this when compiling a package with build-type: Hooks).
  • This library depends on the new CommunicationHandle functionality from process PR #308.

(*) NB: SetupMethod/SetupWrapper is now a bit of a misnomer, because the whole aim of this PR is to no longer go through the Setup interface (and its old UserHooks incarnations such as defaultMainWithHooks ). New naming conventions welcome!
The main change is in Distribution.Client.SetupWrapper: we add a new SetupMethod (probably a misnomer at this point because we no longer go through Setup; new names for this datatype are welcome) in which we directly invoke the Cabal configure, build etc functions rather than going through Setup (or the equivalent defaultMainWithUserHooks interface).

TODO:

• Rebase on top of Setup hooks #9551.
• Land process PR #308.
• Rebase on top of Haddock: use buildSettingKeepTempFiles #9966, Be more careful in handling unconfigured programs in the program database #9967, testsuite: Pass pkgdb of store used for intree Cabal #9968, Use SetupHooks for Configure build-type #9969, Add NoImplicitPrelude to buildTypeScript #9988, SetupHooks: make Location a separate data type #9992.
• Add an implicit dependency on the hooks-exe library when compiling the hooks executable for a package with build-type: Hooks.
• Fix recompilation checking for SetupHooks.hs.
• Compare the LocalBuildInfo obtained at the end of configuring, between using the in-library method and going through Setup, and make them as consistent as it makes sense to make them.
• Test building a package with build-tool-depends: blah in which blah needs to access its data directories. This is important, as data directories are another part of process-global state (like the working directory) that is set upon invoking an external Setup executable (see Distribution.Client.SetupWrapper.internalSetupMethod).
  • This has all been tackled, in a way that fixes both the old and the new behaviour, in Correctly provision build tools in all situations #9912.
• Build stackage with this patch.
• Fix BuildToolPaths test which currently occasionally triggers openBinaryFile: invalid argument (Bad file descriptor)
• Fix issue with not being able to find version of hsc2hs.

[andreabedini]

I tried compiling this with

source-repository-package
  type: git
  location: https://github.com/sheaf/process
  tag: 61b1de95e72acda5d773c972c7484b1a792ca9e5
  post-checkout-command: autoreconf -i

but it fails because System.Process.CommunicationHandle in process does not export useCommunicationHandle. Am I missing something?

[sheaf]

I tried compiling this with

source-repository-package
  type: git
  location: https://github.com/sheaf/process
  tag: 61b1de95e72acda5d773c972c7484b1a792ca9e5
  post-checkout-command: autoreconf -i

but it fails because System.Process.CommunicationHandle in process does not export useCommunicationHandle. Am I missing something?

I've updated the PR now, but please note that this PR is still work in progress and there are quite a few issues that I have yet to resolve.

I didn't know about post-checkout-command, that's neat.

[sheaf]

I have successfully built all of the clc-stackage repository with this patched version of cabal (I included an additional patch to ignore logging handles, to avoid using self-exec instead of in-library build method).

[andreabedini]

In #9735 I was bothered by this threaded mutable state. Removing it boilds down to splitting SetupScriptOptions in V1/V2 versions. The first has many unknown because it's made with miminal planning but the V2 version has already everything we might want to know.

[andreabedini]

installedCabalVersion is only called once (L949) and right after configureCompiler which pre-emptively calls maybeGetInstalledPackages knowing it's going to needed here.
If I got this right, this function signature can be simplified down to

installedCabalVersion
  :: Verbosity
  -> InstalledPackageIndex
  -> VersionRange
  -> PackageId
  -> BuildType
  -> IO (Version, Maybe InstalledPackageId)

which has the advantage of not dealing with SetupScriptOptions mutation's. See my attempt https://github.com/haskell/cabal/pull/9735/files#diff-6ff0f0024945e11d4f23279d3d025c146ef7b28d15e29fefe73f1dab10a80325R793-R799

[andreabedini]

@sheaf This is impressive. I took the liberty of making some comments about SetupWrapper. I hope you don't mind. I think this subsumes #9735, what do you think?

[sheaf]

@sheaf This is impressive. I took the liberty of making some comments about SetupWrapper. I hope you don't mind. I think this subsumes #9735, what do you think?

This is great, I definitely wanted your feedback about SetupWrapper. My changes don't entirely subsume yours at the moment, but do let me know how you would like to proceed.

[andreabedini]

If I understand correctly, in case of the Hooks build-type, this boils down to:

      _ <- compileExternalSetupMethod verbosity options pkg Hooks
      externalHooksABI <- externalSetupHooksABI $ hooksProgFilePath (useWorkingDir options) (useDistPref options)
      let internalHooksABI = hooksVersion
      let abiOk = externalHooksABI == internalHooksABI
      if abiOk
          return $ with (cabalVersion, LibraryMethod, options)
        else do
          withExternalSetupMethod

Is there an advantage of using the LibraryMethod given we had to compile an external setup anyway just to check the ABI?

Now I am wondering, is this always the case? That is, is there a case where we are sure we can use the library method without compiling the script first?

[sheaf]

You are right that currently we compile both the external hooks executable and the setup executable here, even though we only need to compile the hooks executable in this code path (falling back to the setup executable when there is an ABI mismatch). I can refactor the code to avoid the compilation of the external setup executable (it makes sense to do so).

I think the existing "cabal version" logic (what's at the top of withSetupMethod) effectively guarantees that we will have matching Cabal ABI, but given that we need to compile the external hooks executable anyway it makes sense to do an additional version check as belt-and-braces (mismatched Binary instance errors can be a real nightmare to debug). I don't think there's any other work that we can avoid doing, other than not compiling the Setup executable if we don't need it; we need to compile the hooks executable when using build-type: Hooks.

BTW, there are other benefits to going through the in-library method even if we pessimistically compile the setup executable, for instance we skip re-configuring things that cabal-install has already configured.