Skip to content

Directly call in-library functions to build packages#11703

Open
sheaf wants to merge 4 commits intohaskell:masterfrom
sheaf:wip/cabal-install-hooks
Open

Directly call in-library functions to build packages#11703
sheaf wants to merge 4 commits intohaskell:masterfrom
sheaf:wip/cabal-install-hooks

Conversation

@sheaf
Copy link
Copy Markdown
Collaborator

@sheaf sheaf commented Apr 3, 2026

This is a rebase of #9871. I had to create a new PR as Matthew is no longer active (and that PR was made from his personal fork).


Template B: This PR does not modify behaviour or interface.

This is a major refactoring in how cabal-install works, with a few knock-on internal changes to some Cabal library functionality.
It should not cause any breakage for users (I tested clc-stackage and it all builds unchanged with this branch).


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).

In particular, to build a package with build-type: Hooks, cabal-install will compile the package's SetupHooks file into an external hooks executable using the new hooks-exe package. All hooked operations are then performed by communicating with this external executable, instead of going through an opaque Setup executable. The hooks-exe package provides both functionality for compiling an external hooks executable and for interfacing with it. This makes use of the CommunicationHandle API that I added to process.

The main change is to SetupWrapper: the old InternalMethod becomes LibraryMethod, which builds the package by calling Cabal library functions. This is used to build all packages unless build-type: Custom or there is a Cabal library version mismatch which requires us to fall back to compiling Setup.hs and running that. The SelfExec method as well as forceExternalSetupMethod are removed: they no longer have any purpose, as builds can now be carried out concurrently thanks to 7b90583 and edb808a.

The new Distribution.Client.InLibrary module contains all the necessary framework for taking information that cabal-install has and invoking Cabal library functions to perform the appropriate action (configure, build, test, bench, ...). Most of these are pretty simple; the main difficulty is with configure as we need to jump to the part of the Cabal configure code that continues after figuring out information about the system (e.g. compiler information), to avoid wasting work with Cabal rediscovering things that cabal-install already knows. This required a bit of refactoring in Distribution.Simple.Configure.

TODO:

@sheaf sheaf force-pushed the wip/cabal-install-hooks branch 4 times, most recently from cff8372 to 0a2b46e Compare April 3, 2026 19:08
@sheaf sheaf force-pushed the wip/cabal-install-hooks branch 6 times, most recently from d100f9c to 72c1495 Compare April 17, 2026 11:04
@sheaf sheaf marked this pull request as ready for review April 17, 2026 12:23
@sheaf
Copy link
Copy Markdown
Collaborator Author

sheaf commented Apr 17, 2026

I now consider this patch ready for review. Significant changes since #9871:

  1. I further refactored the Cabal library configure function so that all the logic that pertains to adjusting BuildOptions based on compiler and toolchain capabilities (e.g. support for bytecode libraries, profiling dynamic, etc) can be shared. This avoids duplicating logic inside cabal-install and prevents the codepaths from going out of sync.
  2. I continued refactoring SetupWrapper, taking into account suggestions by @andreabedini on the original PR (e.g. moving more functions to the top level, see e.g. cabalLibFromOptions and cabalLibVersionToUse). I also took the opportunity to more clearly demarcate v1-only logic. On top of that, I was able to get rid of the self-exec setup method, now that Cabal library: allow setting the logging handle #11077 has landed. So there is only "external" (via Setup executable) and "in library" now.
  3. The test I recently added for custom-setup builds fail when constraining dependencies in the Cabal-hooks package stack #11331 revealed that the way I had set up the hooks-exe implicit dependency in the previous iteration locked out users from using a different Cabal version in setup-depends entirely, as opposed to falling back to the external setup method. So I moved the necessary logic for compiling an external hooks executable into the Cabal library, which I now realise is the only place where it makes sense for it to live. The logic for invoking the external hooks executable remains separate.

I still need to check stackage compilation. With the changes in (1) implemented, I am now significantly more confident that we are not changing behaviour.

Copy link
Copy Markdown
Collaborator

@geekosaur geekosaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that, despite not intending to change behavior, this probably warrants a changelog entry marked significant.

Comment thread cabal-install/src/Distribution/Client/ProjectPlanning.hs Outdated
Comment thread Cabal/src/Distribution/Simple/SetupHooks/Rule.hs
Comment thread hooks-exe/cli/Distribution/Client/SetupHooks/CallHooksExe/Errors.hs Outdated
Comment thread Cabal/src/Distribution/Simple/Configure.hs Outdated
@sheaf sheaf force-pushed the wip/cabal-install-hooks branch from 72c1495 to 43fd73b Compare April 20, 2026 09:49
@sheaf
Copy link
Copy Markdown
Collaborator Author

sheaf commented Apr 20, 2026

I think that, despite not intending to change behavior, this probably warrants a changelog entry marked significant.

I've written a changelog entry now, please let me know what you think!

@sheaf sheaf mentioned this pull request Apr 20, 2026
3 tasks
sheaf added 2 commits April 20, 2026 12:02
This commit adds 'Structured' instances for the 'RuleCommands' and
'RuleData' datatypes. This allows us to serialise and deserialise them,
which allows us to compute which rules have changed across builds.
This is necessary for recompilation checking for pre-build rules.
This commit adds recompilation logic for SetupHooks pre-build rules.
This implements the behaviour described in the SetupHooks API. That is,
a rule is considered stale if:

  [N] The rule is new, or
  [S1] A dependency of the rule is stale. That is, either we have
       re-run another rule that this rule depends on, or one of the
       file inputs to the rule is newer than the oldest output of the
       rule (or the rule output doesn't exist at all), or
  [S2] The rule itself has changed, e.g. the parameters stored in
       RuleData have changed.

Fixes haskell#11730
@sheaf sheaf force-pushed the wip/cabal-install-hooks branch 2 times, most recently from afeb1b3 to 4cc4945 Compare April 20, 2026 11:00
@sheaf
Copy link
Copy Markdown
Collaborator Author

sheaf commented Apr 20, 2026

I have successfully built all of clc-stackage with this patch (with the exception of the minisat-solver package which fails with a C compiler error that I'm almost certain is unrelated).

I will instrument this version of cabal-install to compare LocalBuildInfo to further validate these changes.

@sheaf
Copy link
Copy Markdown
Collaborator Author

sheaf commented Apr 22, 2026

I have spent this morning diffing the output of cabal-install before and after this patch. I have about 2GB of dumped LocalBuildInfo files, and the differences I am seeing are:

  1. ProgramDb
    a. ghc and ghc-pkg are now FoundOnSystem instead of UserSpecified. I think this is expected because we no longer have to rely on --with-ghc flags to communicate the GHC from cabal-install to the Cabal library.
    b. We consistently add data_dir environment variables to the environment of other programs. This is to ensure that build tools invoked at compile time (e.g. a call to alex in a Template Haskell splice) get the proper data directory.
  2. Cosmetic differences in values for configTests, configBenchmarks, configAllowDependingOnPrivateLibs, configCoverageFor. This is just an artifact about where in the code the default values are applied (e.g. whether we see NoFlag or Flag False); I believe it has no incidence on the actual builds.
  3. configInstallDirs: we now pass correct values for flibdir, includedir and mandir, which was not possible via the CLI interface because there are no ConfigFlags for those fields.

I also found a harmless minor inconsistency in targetBuildDepends where we weren't passing "ThisVersion" constraints corresponding to versions found by the solver. I've rectified that, but I don't expect it to make any difference given that we already pin everything by passing exact unit ids for all dependencies.

I've attached dumps of the LocalBuildInfo of Agda for reference, where the only changes are those described above (and some noise from build directories and ABI hashes changing).

Agda-HEAD.txt
Agda-InLib.txt

@philderbeast
Copy link
Copy Markdown
Collaborator

I also found a harmless minor inconsistency in targetBuildDepends where we weren't passing "ThisVersion" constraints corresponding to versions found by the solver. I've rectified that, but I don't expect it to make any difference given that we already pin everything by passing exact unit ids for all dependencies.

Are these some of those differences?

image

It looks like Agda-InLib.txt accepts any version for these.

@sheaf sheaf force-pushed the wip/cabal-install-hooks branch from 4cc4945 to 29f2db4 Compare April 22, 2026 13:00
@sheaf
Copy link
Copy Markdown
Collaborator Author

sheaf commented Apr 22, 2026

I also found a harmless minor inconsistency in targetBuildDepends where we weren't passing "ThisVersion" constraints corresponding to versions found by the solver. I've rectified that, but I don't expect it to make any difference given that we already pin everything by passing exact unit ids for all dependencies.

Are these some of those differences?
image

It looks like Agda-InLib.txt accepts any version for these.

Yes, that's what I was talking about. I don't think it makes a difference because we pass exact unit IDs for all dependencies (those determined by the solver). I updated this PR to includes the "ThisVersion" constraints determined by the solver just for the sake of consistency.

Comment thread cabal-install/src/Distribution/Client/InLibrary.hs Outdated
Comment thread cabal-install/src/Distribution/Client/InLibrary.hs Outdated
Comment thread cabal-install/src/Distribution/Client/InLibrary.hs Outdated
Comment thread cabal-install/src/Distribution/Client/InLibrary.hs Outdated
@sheaf sheaf force-pushed the wip/cabal-install-hooks branch from 29f2db4 to bb4ba48 Compare April 23, 2026 10:38
@sheaf sheaf force-pushed the wip/cabal-install-hooks branch from bb4ba48 to 7a05966 Compare April 23, 2026 11:09
@sheaf
Copy link
Copy Markdown
Collaborator Author

sheaf commented Apr 23, 2026

I believe I've addressed everything. I would appreciate a review on #11731 so that I can land that first; that's a self-contained change to recompilation checking for SetupHooks pre-build rules.

Just hWrite ->
case hooksExeArgs of
hookName : _ ->
case lookup hookName allHookHandlers of
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe 5 nested case ... of is a lot? :)

I haven't tested it, but I hope it works.

hooksMain :: SetupHooks -> IO ()
hooksMain setupHooks = do
  (inputFd, outputFd, hookName) <- getArgs >>= \case
    (i : o : n : _) -> pure (i, o, n)
    (_ : _ : [])    -> dieWithException hooksExeVerbosity NoHookType
    _               -> dieWithException hooksExeVerbosity (NoHandle Nothing)

  hRead <- (readMaybe inputFd >>= openCommunicationHandleRead) 
    >>= justOrDie (dieWithException hooksExeVerbosity (NoHandle $ Just $ "hook input communication handle '" ++ inputFd ++ "'"))

  hWrite <- (readMaybe outputFd >>= openCommunicationHandleWrite) 
    >>= justOrDie (dieWithException hooksExeVerbosity (NoHandle $ Just $ "hook output communication handle '" ++ outputFd ++ "'"))

  handleAction <- (lookup hookName allHookHandlers)
                    `justOrDie` (dieWithException hooksExeVerbosity (BadHooksExeArgs hookName (UnknownHookType (map fst allHookHandlers)))) 
                         
  handleAction (hRead, hWrite) setupHooks

  where
    justOrDie :: IO a -> Maybe a -> IO a
    justOrDie die = maybe die pure

    allHookHandlers = [(hookName h, hookHandler h) | h <- hookHandlers]

@sheaf sheaf force-pushed the wip/cabal-install-hooks branch 3 times, most recently from 30c4250 to 7aee7a3 Compare April 23, 2026 14:33
sheaf added 2 commits April 23, 2026 16:34
This architectural change ensures that we build packages using Cabal
library functions (using a Haskell library interface) instead of going
via the command-line interface of Setup.hs, as much as possible.

The main changes are in SetupWrapper: the old InternalMethod becomes
LibraryMethod, which builds the package by calling Cabal library functions.
This is used to build all packages unless build-type: Custom or there is
a Cabal library version mismatch which requires us to fall back to
compiling Setup.hs and running that.
The SelfExec method as well as forceExternalSetupMethod are removed:
they no longer have any purpose, as builds can now be carried out
concurrently thanks to 7b90583 and edb808a.
This change 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 (the local
build info file).

The new Distribution.Client.InLibrary module contains all the necessary
framework for taking information that cabal-install has and invoking
Cabal library functions to perform the appropriate action
(configure, build, test, bench, ...).
Most of these are pretty simple; the main difficulty is with configure
as we need to jump to the part of the Cabal configure code that
continues after figuring out information about the system (e.g. compiler
information, dependency information determined by the solver), to avoid
wasting work with Cabal rediscovering things that cabal-install already
knows. This required a bit of refactoring in Distribution.Simple.Configure.

Packages with 'build-type: Hooks' are now compiled with the in-library
method (instead of going via Setup.hs). This is achieved as follows:

  - The Cabal library provides 'Distribution.Simple.SetupHooks.HooksMain',
    which exposes `hooksMain :: SetupHooks -> IO ()`. This allows us
    to turn any definition of `SetupHooks` into an external hooks
    executable.
  - The new `hooks-exe` library, which `cabal-install` depends on,
    provides the logic for communicating with the external hooks
    executable (using the `CommunicationHandle` functionality from
    haskell/process#308).

This change has been extensively tested by compiling clc-stackage and
diffing the differences in LocalBuildInfo (i.e. the output of the
configure step).
@sheaf sheaf force-pushed the wip/cabal-install-hooks branch from 7aee7a3 to 89448e8 Compare April 23, 2026 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants