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

Fix haddock build #355

Closed
wants to merge 1 commit into from
Closed

Conversation

mightybyte
Copy link
Contributor

Haddock does not allow the caret character here

@phadej
Copy link
Contributor

phadej commented Apr 1, 2019

@mightybyte
Copy link
Contributor Author

I'm running the most recently released version of cabal-install and it still fails. That seems a bit too soon to upgrade something as minor and inconsequential as this. On top of that, this bug makes the problem crushing. Please wait awhile before introducing this. Not everyone is able to upgrade core infrastructure like cabal-install and haddock quickly.

@phadej
Copy link
Contributor

phadej commented Apr 1, 2019

Note that you are implicitly asking for me to have less documentation; though I agree that in this case the comments are not so insightful. I don't consider haddocks to be a part of any versioning contract.

The failure is due haddock; upgrading cabal-install won't make an error go away. Except if newer cabal is more lenient (which it seems to be for local builds?)

I have mixed feelings. I could make a patch release removing the comments, and another patch release adding them back but dropping older GHC (and accidentally haddock) support. I hope that in next release I'll actually document the constructors of GADTs, then the haddocks won't be that obvious anymore.

I co-maintain another package which old haddock cannot handle (and cannot be fixed without actual code changes), and there dropping old GHC support (requires haddock bundled with GHC-8.4) is too early.

@hvr
Copy link

hvr commented Apr 1, 2019

@phadej what about using CPP to conditionally inject those comments? iirc we can test for the haddock version via some CPP macro...

for one there this here in cabal_macros.h for recent enough cabal versions:

/* tool haddock-2.22.0 */
#ifndef TOOL_VERSION_haddock
#define TOOL_VERSION_haddock "2.22.0"
#endif /* TOOL_VERSION_haddock */
#ifndef MIN_TOOL_VERSION_haddock
#define MIN_TOOL_VERSION_haddock(major1,major2,minor) (\
  (major1) <  2 || \
  (major1) == 2 && (major2) <  22 || \
  (major1) == 2 && (major2) == 22 && (minor) <= 0)
#endif /* MIN_TOOL_VERSION_haddock */

and iirc there was also a CPP macro emitted by haddock directly (but that's long-term not a good idea as it probably doesn't work yet w/ Hi-Haddock)

@mightybyte
Copy link
Contributor Author

The failure is due haddock; upgrading cabal-install won't make an error go away. Except if newer cabal is more lenient (which it seems to be for local builds?)

That's even worse! I have no idea where haddock gets brought into the picture. All I did was install the most recent version of cabal-install and try to build my project. Nowhere does a haddock version ever come into view.

This seems like a really obvious choice to me. All it takes to solve the significant user pain is remove three words from the documentation that didn't really add much value in the first place.

@phadej
Copy link
Contributor

phadej commented Apr 1, 2019

It feels obvious as haddocks are short, I blame myself I didn't wrote longer
ones.

I could live with, everything less won't be accepted.

+#if __HADDOCK_VERSION__ >= recentenough
+#define HAS_GADT_HADDOCKS
+endif
-        :: CommandMethod           -- ^ command
+        :: CommandMethod
+#ifdef HAS_GADT_HADDOCKS
+           -- ^ command
+#endif

As said, I planned to write more documentation so the diff would be

+#ifdef HAS_GADT_HADDOCKS
         -- | Commands are not-(read-only)-queries,
         -- i.e. cause some side-effect/write on the GitHub side.
         -- Therefore 'Command' is always 'RW' (read-write).
+#endif
         :: CommandMethod

@phadej
Copy link
Contributor

phadej commented Apr 1, 2019

FYI

% haddock-ghc-8.6.4 --version
Haddock version 2.22.0, (c) Simon Marlow 2006
Ported to use the GHC API by David Waern 2006-2008
% haddock-ghc-8.4.4 --version
Haddock version 2.20.0, (c) Simon Marlow 2006
Ported to use the GHC API by David Waern 2006-2008

@phadej
Copy link
Contributor

phadej commented May 30, 2019

I won't merge this until haskell/cabal#5977 is resolved.

EDIT: currently we do build haddocks with all GHCs on CI, and CI is green. So, out of principle, I won't fix this haddock issue, until CI actually verifies there are no others.

@phadej phadej added the blocked label May 30, 2019
@ghost
Copy link

ghost commented Nov 2, 2021

It seems the cabal issue has been resolved at the time of this writing?

@andreasabel
Copy link
Member

@Mergifyio rebase

Haddock does not allow the caret character here
@mergify
Copy link

mergify bot commented Apr 19, 2022

rebase

✅ Branch has been successfully rebased

@andreasabel
Copy link
Member

This PR is obsolete, since haddock for the code base works from GHC 8.6 on:

haddock: >=8.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants