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

Setup hooks #9551

Merged
merged 3 commits into from Apr 18, 2024
Merged

Setup hooks #9551

merged 3 commits into from Apr 18, 2024

Conversation

sheaf
Copy link
Collaborator

@sheaf sheaf commented Dec 21, 2023

This PR contains the implementation of the new Hooks build-type, as described in the Haskell Foundation Tech Proposal.

It builds on several previous PRs. For review, you should only need to look at the Introduce SetupHooks and SetupHooks: add tests commits.

New modules:

  • Distribution.Simple.SetupHooks.Internal contains the internal implementation details of the API for the Hooks build type
  • Distribution.Simple.SetupHooks.Rule defines fine grained rules as specified in the proposal, for use in pre-build hooks
  • Distribution.Simple.SetupHooks.Errors defines error constructors pertaining to the Hooks build type

There are barely any changes to Cabal outside the introduction of these modules. Essentially, the changes consist of defining defaultMainWithSetupHooksArgs, and the introduction of variants build_setupHooks, configure_setupHooks... of existing functions (with e.g. build = build_setupHooks noBuildHooks) which insert the hooked actions at the appropriate times.

New libraries:

  • Cabal-hooks is a new package that exports the exposed interface for package authors to write their own SetupHooks.hs modules. For the time being, it mostly re-exports datatypes from Cabal, together with some slightly higher-level functions for use in the API, all through the single module Distribution.Simple.SetupHooks.
    As explained in the HF tech proposal, a possible future goal would be to make this package no longer depend on Cabal, perhaps making Cabal depend on it instead. This would limit the amount of internal details from Cabal we are exposing through the Hooks API, which would have the potential to improve maintainability of user packages relying on the Hooks API.

TODO:

  • Rebase this PR once preparatory MRs land.
  • Update this PR once the discussion on the tech proposal settles down, with a final design.
  • Ensure all documentation is up-to-date relative to the exposed API.
  • Add changelog entry.
  • Address the remaining SetupHooks TODOs in the code.

--

@sheaf sheaf marked this pull request as draft December 21, 2023 13:40
@sheaf sheaf changed the title Draft: Setup hooksip/setup hooks mr Draft: Setup hooks Dec 21, 2023
@sheaf sheaf force-pushed the wip/setup-hooks-mr branch 25 times, most recently from 9446424 to 0f9ad07 Compare December 22, 2023 15:15
@sheaf sheaf force-pushed the wip/setup-hooks-mr branch 3 times, most recently from 90dc2a8 to 7b94e0b Compare January 11, 2024 15:22
@sheaf sheaf force-pushed the wip/setup-hooks-mr branch 4 times, most recently from 0ce38bc to 4c81ae7 Compare April 9, 2024 14:06
Copy link
Collaborator

@andreabedini andreabedini left a comment

Choose a reason for hiding this comment

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

Great work. Let's merge this in and start using it 💪 ☺️

@alt-romes
Copy link
Collaborator

alt-romes commented Apr 10, 2024

Thank you for your reviews @gbaz @andreabedini.

We're finishing the migration of packages in the testing-overlay to this very latest versions of Hooks to ensure there is nothing missing from this final version of the MR. We'll do the merge right after.

@sheaf sheaf force-pushed the wip/setup-hooks-mr branch 3 times, most recently from 44a88d7 to d2f510b Compare April 15, 2024 16:40
@sheaf sheaf force-pushed the wip/setup-hooks-mr branch 3 times, most recently from 2c8cae9 to f70ef62 Compare April 15, 2024 18:34
@sheaf
Copy link
Collaborator Author

sheaf commented Apr 15, 2024

I've been applying a few fixes, in particular to the Binary instances of datatypes involved in pre-build rule. The main point is that we want to be able to treat arguments to rules as opaque blobs of data on the build system side, which means that any such data must always be prefaced by its length (otherwise the build system doesn't know how much data to read). I am planning to get this merged as soon as CI passes now.

@sheaf sheaf force-pushed the wip/setup-hooks-mr branch 3 times, most recently from 3089a6f to 4b173c6 Compare April 16, 2024 09:11
@sheaf sheaf added the merge me Tell Mergify Bot to merge label Apr 16, 2024
@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Apr 18, 2024
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 splits off the file monitoring types from cabal-install into
the Cabal library, so that they can be referred to in pre-build rules
for SetupHooks. This will allow package authors with Hooks build-type to
monitor files and directories specified by globbing.
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.

Tests include:

  - 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
  - correctness tests for SetupHooks, e.g. that
    pre-build rules are run in dependency order (see the
    `SetupHooksRuleOrdering` test)
@mergify mergify bot merged commit b9084c9 into haskell:master Apr 18, 2024
44 checks passed
geekosaur added a commit that referenced this pull request Apr 29, 2024
because `System.Process.Internals` just (like, within the past
hour or so) started exporting a name we are using.
geekosaur added a commit that referenced this pull request Apr 29, 2024
because `System.Process.Internals` just (like, within the past
hour or so) started exporting a name we are using.
mergify bot added a commit that referenced this pull request Apr 29, 2024
…#9956)

* CI: force MacOS jobs to use Intel runners (`macos-13`)

GitHub just switched macos-latest to the ARM chips (now alisasing
`macos-14`), and it brings a bunch of problems.

- First of all, GHC's 8.8 and 8.6 don't exist there.
- ghcup and llvm are unavailable

For the time being, lets stay on the previous version of the runner.

(cherry picked from commit d36e0d0)

# Conflicts:
#	.github/workflows/validate.yml

* CI: GitHub MacOS runners lost ghcup since 2024-04-27, so use haskell-action/setup instead

(cherry picked from commit 082d952)

* fixup! more compat with new macos runners

(cherry picked from commit 326a1f6)

* !fixup resolve conflicts

* copy an import list from #9551

because `System.Process.Internals` just (like, within the past
hour or so) started exporting a name we are using.

---------

Co-authored-by: Artem Pelenitsyn <a.pelenitsyn@gmail.com>
Co-authored-by: brandon s allbery kf8nh <allbery.b@gmail.com>
geekosaur added a commit that referenced this pull request Apr 29, 2024
because `System.Process.Internals` just (like, within the past
hour or so) started exporting a name we are using.
geekosaur added a commit that referenced this pull request Apr 29, 2024
…#9956)

* CI: force MacOS jobs to use Intel runners (`macos-13`)

GitHub just switched macos-latest to the ARM chips (now alisasing
`macos-14`), and it brings a bunch of problems.

- First of all, GHC's 8.8 and 8.6 don't exist there.
- ghcup and llvm are unavailable

For the time being, lets stay on the previous version of the runner.

(cherry picked from commit d36e0d0)

* CI: GitHub MacOS runners lost ghcup since 2024-04-27, so use haskell-action/setup instead

(cherry picked from commit 082d952)

* fixup! more compat with new macos runners

(cherry picked from commit 326a1f6)

* !fixup resolve conflicts

* copy an import list from #9551

because `System.Process.Internals` just (like, within the past
hour or so) started exporting a name we are using.

---------

Co-authored-by: Artem Pelenitsyn <a.pelenitsyn@gmail.com>
Co-authored-by: brandon s allbery kf8nh <allbery.b@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants