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

Reinstate 'initialBuildSteps' function (backport #9950) #9961

Merged
merged 1 commit into from
May 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 61 additions & 1 deletion Cabal/src/Distribution/Simple/Build.hs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ module Distribution.Simple.Build
, writeBuiltinAutogenFiles
, writeAutogenFiles

-- ** Legacy functions
, componentInitialBuildSteps
, initialBuildSteps

-- * Internal package database creation
, createInternalPackageDB
) where
Expand Down Expand Up @@ -928,6 +932,61 @@ replFLib flags pkg_descr lbi exe clbi =
GHC -> GHC.replFLib flags NoFlag pkg_descr lbi exe clbi
_ -> dieWithException verbosity REPLNotSupported

-- | Runs 'componentInitialBuildSteps' on every configured component.
--
-- Legacy function: does not run pre-build hooks or pre-processors. This function
-- is insufficient on its own to prepare the build for a package.
--
-- Consumers wanting to prepare the sources of a package, e.g. in order to
-- launch a REPL session, are advised to run @Setup repl --repl-multi-file=<fn>@
-- instead.
Comment on lines +941 to +942
Copy link
Collaborator

Choose a reason for hiding this comment

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

My only thought - and it is not a 'blocker' - is that Setup repl --repl-multi-file=<fn> could have greater consistency with the Cabal User Guide, which is written in terms of:

runhaskell Setup.hs repl --repl-multi-file=<fn>

If that is applicable, it is also applicable below.

Copy link
Member

Choose a reason for hiding this comment

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

Let me try to understand: Setup assumes the setup script is compiled (with something like ghc --make Setup.hs, I guess?) and the binary is invoked, while runhaskell Setup.hs interprets it and thus runs is? Is that it? Do both of these forms work so that it's just a user choice? If so, I'd rather avoid creating a new PR on master to change that (the original PR is already merged), unless @mpilgrem feels strongly one of the forms is misleading or bad style. But if one of these is actually wrong, please let me know and I will create the PR myself (and let's also fix here in that case).

Copy link
Collaborator

Choose a reason for hiding this comment

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

My review comment was a subtle one, and I do not feel strongly about it. I was exploring the following:

What does it mean to advise: run @Setup repl --repl-multi-file=<fn>@?

It means (as documented in the Cabal User Guide for other uses of the Cabal Haskell 'script' file named (by convention) Setup.hs) to command runhaskell Setup.hs --repl-multi-file=<fn> (or, equivalently, runghc Setup.hs --repl-multi-file=<fn>). My suggestion was to state things more plainly in the Haddock documentation and in a way consistent with the User Guide.

Copy link
Member

@Mikolaj Mikolaj May 1, 2024

Choose a reason for hiding this comment

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

I get it now, thank you. Yes, consistency is good. I'm glad that's the only problem you see in this PR, so I suggest we merge it and remember to keep docs consistent in the future (e.g., if the comment on master branch survives longer than the deprecated feature and so we'd be editing it again in the future).

initialBuildSteps
:: FilePath
-- ^ "dist" prefix
-> PackageDescription
-- ^ mostly information from the .cabal file
-> LocalBuildInfo
-- ^ Configuration information
-> Verbosity
-- ^ The verbosity to use
-> IO ()
initialBuildSteps distPref pkg_descr lbi verbosity =
withAllComponentsInBuildOrder pkg_descr lbi $ \_comp clbi ->
componentInitialBuildSteps distPref pkg_descr lbi clbi verbosity
{-# DEPRECATED
initialBuildSteps
"This function does not prepare all source files for a package. Suggestion: use 'Setup repl --repl-multi-file=<fn>'."
#-}

-- | Creates the autogenerated files for a particular configured component.
--
-- Legacy function: does not run pre-build hooks or pre-processors. This function
-- is insufficient on its own to prepare the build for a component.
--
-- Consumers wanting to prepare the sources of a component, e.g. in order to
-- launch a REPL session, are advised to run
-- @Setup repl <compName> --repl-multi-file=<fn>@ instead.
componentInitialBuildSteps
:: FilePath
-- ^ "dist" prefix
-> PackageDescription
-- ^ mostly information from the .cabal file
-> LocalBuildInfo
-- ^ Configuration information
-> ComponentLocalBuildInfo
-- ^ Build info about the component
-> Verbosity
-- ^ The verbosity to use
-> IO ()
componentInitialBuildSteps _distPref pkg_descr lbi clbi verbosity = do
let compBuildDir = componentBuildDir lbi clbi
createDirectoryIfMissingVerbose verbosity True compBuildDir
writeBuiltinAutogenFiles verbosity pkg_descr lbi clbi
{-# DEPRECATED
componentInitialBuildSteps
"This function does not prepare all source files for a component. Suggestion: use 'Setup repl <compName> --repl-multi-file=<fn>'."
#-}

-- | Pre-build steps for a component: creates the autogenerated files
-- for a particular configured component.
preBuildComponent
Expand All @@ -939,7 +998,8 @@ preBuildComponent
preBuildComponent verbosity lbi tgt = do
let pkg_descr = localPkgDescr lbi
clbi = targetCLBI tgt
createDirectoryIfMissingVerbose verbosity True (componentBuildDir lbi clbi)
compBuildDir = componentBuildDir lbi clbi
createDirectoryIfMissingVerbose verbosity True compBuildDir
writeBuiltinAutogenFiles verbosity pkg_descr lbi clbi

-- | Generate and write to disk all built-in autogenerated files
Expand Down
21 changes: 21 additions & 0 deletions changelog.d/pr-9950
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
synopsis: Re-instate `initialBuildSteps`
packages: Cabal
issues: #9856
prs: #9950

description: {

The `initialBuildSteps` function from `Distribution.Simple.Build`, which had
been hastily removed, has been reinstated.

It now comes with a deprecation warning: calling that function does not suffice
to prepare the sources for a package, as there are other steps that one might
also need to perform:

- running pre-processors (such as alex/happy)
- running pre-build hooks or custom logic
(in build-type: Hooks or build-type: Custom or Configure)

Consumers wanting to prepare the sources of a package, e.g. in order to launch a
REPL session, are advised to run `setup repl --repl-multi-file=<fn>` instead.
}
Loading