Skip to content

cabal haddock: imply dependencies, but gentler, i.e. don't override config/project settings #11002

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ulysses4ever
Copy link
Collaborator

This is a WIP to fix #8725 as outlined in #8725 (comment). It's not working yet and I'd appreciate suggestions on why this is the case.

High-level idea. Project configuration is built out of two pieces: config/project file settings and command-line flags: projectConfig <> cliConfig.

The way "cabal haddock implying --enable-documentation" (#8259 #8330) was implemented is it modified the latter (CLI flags). But that made the resulting code ignore anything from config/project files (flags are a Last monoid).

The idea here is instead of modifying CLI flags, add command-specific "default settings" (can differ between cabal's subcommands) that are prepended to the result, so: defaultConfigPerCommand <> projectConfig <> cliConfig.

I hope the high-level idea makes sense but I invite comments. The low-level implementation, as noted above, isn't there yet: I need to figure out where to push these new default settings exactly, as the one (or two) place(s) I tried so far didn't give me the same result as the one we get with the previous approach based on CLI flags (thankfully, there's a test for it #8259).


Template Α: This PR modifies behaviour or interface

Include the following checklist in your PR:

@ulysses4ever
Copy link
Collaborator Author

looks like global config gets documentation: False by default, represented by Flag False rather than NoFlag (which is what I hoped for). So, maybe, this idea won't work...

@ulysses4ever ulysses4ever force-pushed the cabal-haddock-gentler-imply-dependencies branch from ba00b45 to fe92864 Compare June 19, 2025 03:59
@ulysses4ever
Copy link
Collaborator Author

Trying to flip the default from Flag False to NoFlag and see how loud is the noise from failing CI...

@ulysses4ever ulysses4ever force-pushed the cabal-haddock-gentler-imply-dependencies branch 6 times, most recently from 766d075 to c245c8e Compare June 20, 2025 04:19
@ulysses4ever ulysses4ever force-pushed the cabal-haddock-gentler-imply-dependencies branch from c245c8e to 60c83c5 Compare June 20, 2025 04:20
@ulysses4ever
Copy link
Collaborator Author

@coot a haddock-project integration test:

testHaddockProjectDependencies :: ProjectConfig -> Assertion
testHaddockProjectDependencies config = do
(_, _, sharedConfig) <- planProject testdir config
-- `haddock-project` is only supported by `haddock-2.26.1` and above which is
-- shipped with `ghc-9.4`
when (compilerVersion (pkgConfigCompiler sharedConfig) > mkVersion [9, 4]) $ do
let dir = basedir </> testdir
cleanHaddockProject testdir
withCurrentDirectory dir $ do
CmdHaddockProject.haddockProjectAction
defaultHaddockProjectFlags
{ haddockProjectCommonFlags =
defaultCommonSetupFlags
{ setupVerbosity = Flag verbosity
}
}
["all"]
defaultGlobalFlags{globalStoreDir = Flag "store"}
let haddock = "haddocks" </> "async" </> "async.haddock"
hasHaddock <- doesFileExist haddock
unless hasHaddock $ assertFailure ("File `" ++ haddock ++ "` does not exist.")
cleanHaddockProject testdir
where
testdir = "haddock-project/dependencies"

fails with the patch here. This is probably due to switching from Flag False to NoFlag in defaultInstallFlags. Do you have any ideas how to fix it? I'm not very familiar with how haddock-project works, but it seems like it's making an assumption that may be defeated by this change. I'm wondering if making this assumption is a good idea or maybe the test should simply set more flags explicitly.

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.

Consider reverting default usage of --enable-documentation in cabal haddock
1 participant