-
Notifications
You must be signed in to change notification settings - Fork 697
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
cabal haddock imply enable-documentation #8259
Conversation
@ulysses4ever : Maybe good to report this internal error upstream... |
@andreasabel fair, but my fear is that it's my change breaking cabal is to blame, not haddock itself. Haddock complains that it can't find something in the cabal store. So I assume the change broke some invariant, which, in turn, made cabal to call haddock with a non-existent input. I'm very surprised that the CI is green. I wonder how good its testing of |
Okay, after applying the nuclear option -- nuking the store -- I get it working fine. Time to think about a test, I guess. |
8c7696e
to
5e0802a
Compare
Thanks to ffaf1, I managed a test. Should be good now.
|
bdecafb
to
ccf99e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. How about a changelog file?
@Mikolaj forgot to stage it... fixed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. Thank you.
da68c28
to
d633250
Compare
|
@@ -141,7 +141,8 @@ haddockAction flags@NixStyleFlags {..} targetStrings globalFlags = do | |||
runProjectPostBuildPhase verbosity baseCtx buildCtx' buildOutcomes | |||
where | |||
verbosity = fromFlagOrDefault normal (configVerbosity configFlags) | |||
cliConfig = commandLineFlagsToProjectConfig globalFlags flags mempty -- ClientInstallFlags, not needed here | |||
flags' = flags { installFlags = installFlags { installDocumentation = Flag True } } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it be that this unconditional setting leads to --disable-doc
being ignored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think you're right.
The problem described below was solved by nuking the store. All good now. Fixing #7462.
I hoped that implementing #7462 should be a breath. Alas, an obvious fix turns into a disfunctional
cabal haddock
: it keeps failing withFor a simple project like this:
Putting up this draft in case someone can spot the problem...
Please include the following checklist in your PR:
Please also shortly describe how you tested your change. Bonus points for added tests!