Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build extra rules #1

Open
wants to merge 27 commits into
base: wip/setup-hooks
Choose a base branch
from

Conversation

alt-romes
Copy link
Collaborator

@alt-romes alt-romes commented Jan 8, 2024

Just to put it on the map

sheaf and others added 27 commits December 21, 2023 20:15
The aim of this commit is to modularise LocalBuildInfo to better
reflect implicit invariants.

The top-level split

  data LocalBuildInfo =
    NewLocalBuildInfo
      { localBuildDescr  :: LocalBuildDescr
      , localBuildConfig :: LocalBuildConfig
      }

reflects which part of a LocalBuildInfo are set in stone by Cabal
(the LocalBuildDescr), while LocalBuildConfig contains options that
can be controlled/modified by the user in some way.

The split

  data LocalBuildDescr =
    LocalBuildDescr
    { packageBuildDescr :: PackageBuildDescr
    , componentBuildDescr :: ComponentBuildDescr
    }

reflects that some parts of the information determined at configuration
time resides at the package level, while other pieces of information
are pertinent to individual components.

Finally the structure of LocalBuildConfig is aimed to reduce code
duplication between the Cabal library and cabal-install.
This commit leverages the changes to the LocalBuildInfo datatype in
the Cabal library to reduce code duplication. Specifically, we split
off certain fields of ElaboratedConfiguredPackage by re-using the
BuildOptions datatype.

This is purely an internal refactoring with no observable changes
in behaviour.
This commit modularises the configure phase (i.e. the function
Distribution.Simple.Configure.configure). The aim of this change
is to make explicit the control flow of the function: it starts off
with a package-wide phase, and then componentwise configuration.
This commit very slightly refactors the per-component pre-build steps
(renaming from componentInitialBuildSteps to preBuildComponent).
This commit exposes installFileGlob as a generally useful
part of the API which users might want to call, e.g. in their
custom Setup scripts.
This commit makes it so that cabal-install can explain the reason why
it used the legacy fallback, instead of building per-component.
This change means that we can refer to BuildTarget in Cabal error
constructors. (Attempting to do so before this change would cause
cyclic imports.)
This commit adds a Suffix newtype to describe suffixes as handled
by suffix handlers & preprocessors, and changes the PPSuffixHandler
type definition to use it.

It also moves some type definitions from Distribution.Simple.PreProcess
to the new module Distribution.Simple.PreProcess.Types.

As this commit changes the definition of PPSuffixHandler, it will
break custom Setup scripts which use the 'hookedPreProcessors'
functionality.
This commmit removes the dependency of the
Distribution.Simple.Program.Types on Distribution.Simple.Program.Find
by shuffling around a few definitions between the modules.

This makes the "Types" module more self-contained, which means it
can be more easily imported without causing module cycles.
This commit modifies the generation of runghc commands to pass an
additional -i argument to account for the change in working directory.

This ensures that, in the testsuite, runghc is able to see other modules.
For example, an invocation of runghc to run a Custom Setup script will
now properly see the modules imported, e.g. if one has a directory
structure like

  test.cabal
  Setup.hs
  SetupDep.hs

and Setup.hs imports SetupDep.hs.
The Semigroup and Monoid instances for ForeignLib were completely
broken: for the `foreignLibVersionInfo` and `foreignLibVersionInfo`,
we essentially had the following:

  mempty :: Maybe XYZ
  mempty = Nothing

  (<>) :: Maybe XYZ -> Maybe XYZ -> Maybe XYZ
  _ <> b = b

which is obviously not a valid Monoid, as `Just x <> Nothing = Nothing`,
violating the identity law.

The Semigroup instance for Executable was also deeply suspicious, as
it combined the module paths, which makes no sense. Now we instead error
if the two module paths are different (and both nonempty).
This commit introduces a new build-type, Hooks. A package using this
build type should provide a SetupHooks.hs module which exports
a value `setupHooks :: SetupHooks`. This is intended to replace the
Custom setup type. This allows Cabal to have finer-grained information
about the build, instead of having an opaque Setup executable to invoke.
This adds tests for error messages related to SetupHooks:

  - error when returning an invalid component diff in a
    per-component pre-configure hook
  - error when declaring pre-build rules whose dependency
    graph contains cycles
  - error when we cannot find a dependency of a pre-build rule
  - warning when there are pre-build rules that are declared but
    never demanded

It also adds some correctness tests for SetupHooks, e.g. that
pre-build rules are run in dependency order (see the
`SetupHooksRuleOrdering` test).
ROMES:TODO: We need to make sure SetupHooks users know to use
builtinBuildRules <> theirSetupHooks
The extra build sources used to be built after the Haskell sources,
which would fix problems such as the C source depending on a stub header
file generated from a Haskell module with a foreign export.

Not sure how that can be fixed. Need to discuss...
Pre-processing is done as part of building the component, which is done
in `buildComponent`, invoked right *after* `preBuildComponent runPreBuildHooks`
which executes the build rules.

Ignoring for now the problems from running the rules before pre-processing
and building the component...

The extra source build rules are determined at configure time (which is
ofc before pre-processing).

What's the problem?

Some of the extra sources are only determined after preprocessing
(function `preprocessExtras`), for example, a C file generated from
pre-processing a .hsc would be added to the component's C sources.

Because rules for extra sources are determined and even run before
preprocessing, how can we account for pre-process-generated-sources?

Another note is that we cannot check whether a file such as
Bar.hs, which is generated from Bar.hsc, exists at configure time,
because it will only be generated after pre-processing.

In any case, we may specify dependencies on files which, when compiling,
do not exist -- we'll fail accordingly.
@sheaf sheaf force-pushed the wip/setup-hooks branch from dec30b2 to 8ccfb6b Compare April 9, 2024 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants