cabal icon indicating copy to clipboard operation
cabal copied to clipboard

Directly call in-library functions to build packages

Open sheaf opened this issue 1 year ago • 13 comments

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:

  • [x] Rebase on top of #9551.
  • [x] Land process PR #308.
  • [x] Rebase on top of #9966, #9967, #9968, #9969, #9988, #9992.
  • [x] Add an implicit dependency on the hooks-exe library when compiling the hooks executable for a package with build-type: Hooks.
  • [x] Fix recompilation checking for SetupHooks.hs.
  • [x] 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.
  • [x] 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 #9912.
  • [x] Fix BuildToolPaths test which currently occasionally triggers openBinaryFile: invalid argument (Bad file descriptor)
  • [x] Fix issue with not being able to find version of hsc2hs.
  • [x] Delay compiling Setup for build-type: Hooks unless we absolutely need it.
  • [x] Update bootstrap plans
  • [x] Build stackage with this patch, diffing the LocalBuildInfo between in-library and Setup.
  • [ ] Rebase on top of #10991, #10992, #10993, #10994.

sheaf avatar Apr 08 '24 10:04 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?

andreabedini avatar Apr 10 '24 10:04 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?

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 avatar Apr 11 '24 08:04 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).

sheaf avatar May 06 '24 16:05 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?

andreabedini avatar May 10 '24 09:05 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?

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.

sheaf avatar May 10 '24 10:05 sheaf