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.

Sign up for GitHub

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

Jump to bottom

Do not always pass --quickjump to haddock #9049

Merged
merged 1 commit into from
Jun 29, 2023
Merged

Do not always pass --quickjump to haddock #9049

merged 1 commit into from
Jun 29, 2023

Conversation

FinleyMcIlwaine
Copy link
Contributor

6d8adf1 caused cabal to always pass the --quickjump flag to haddock. This commit fixes it by requiring that the result of flagToList on argQuickJump is True, instead of anything.

@ulysses4ever ulysses4ever requested a review from coot June 20, 2023 02:00
Kleidukos
Kleidukos approved these changes Jun 20, 2023
View reviewed changes
Copy link
Member

@Kleidukos Kleidukos left a comment

Choose a reason for hiding this comment

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

LGTM but I'd love @coot's opinion on this

@coot
Copy link
Collaborator

coot commented Jun 21, 2023

Wouldn't it be better to just remove --quickjump option. What's the use case to not enable quick jumps?

@FinleyMcIlwaine
Copy link
Contributor Author

Since the quickjump index generation can use so much memory, I think it's okay to keep it optional. No matter what the right default is for Haddock, this is just fixing a bug in Cabal.

@coot coot added the merge me Tell Mergify Bot to merge label Jun 22, 2023
@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jun 24, 2023
@coot coot mentioned this pull request Jun 29, 2023
Closed
4 tasks
@coot
Copy link
Collaborator

coot commented Jun 29, 2023

Could you add a changelog entry?

@fgaz fgaz removed merge me Tell Mergify Bot to merge merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days labels Jun 29, 2023
@ulysses4ever
Copy link
Collaborator

My research on this issue:

This should fix #9060 and improve #8326, i.e. with GHC < 9.4 cabal haddock --enable-doc works in more situations than before. #8326 is not fixed because cabal haddock --enable-doc --haddock-for-hackage still fails and doesn't seem to be fixable without upgrading to a newer GHC (9.4+) and Haddock.

@ulysses4ever
Copy link
Collaborator

@mergify rebase

@mergify
Copy link
Contributor

mergify bot commented Jun 29, 2023

rebase

✅ Branch has been successfully rebased

@ulysses4ever ulysses4ever mentioned this pull request Jun 29, 2023
Open
@ulysses4ever ulysses4ever added the merge me Tell Mergify Bot to merge label Jun 29, 2023
@ulysses4ever
Copy link
Collaborator

I see this as a critical bugfix #9060 to be ported to 3.10, so I took liberty to add a changelog file and put the merge-me label back. If anyone is willing to give it another approval, perhaps we could skip the cool-off period and merge it right away so that we can backport sooner (3.10.2 is any day now). Otherwise, this will seat for the usual 2 days.

geekosaur
geekosaur approved these changes Jun 29, 2023
View reviewed changes
Copy link
Collaborator

@geekosaur geekosaur left a comment

Choose a reason for hiding this comment

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

Otherwise looks good to me.

changelog.d/pr-9049 Outdated Show resolved Hide resolved
@FinleyMcIlwaine @ulysses4ever
Do not always pass --quickjump to haddock
8867b46 
6d8adf1 caused cabal to always pass the
`--quickjump` flag to haddock. This commit fixes it by requiring that the
result of `flagToList` on `argQuickJump` is `True`, instead of anything.
@Kleidukos
Copy link
Member

Yes I'd like to expedite the backporting process on this one.

@Kleidukos Kleidukos merged commit 7b1a693 into haskell:master Jun 29, 2023
41 checks passed
@Kleidukos
Copy link
Member

@Mergifyio backport 3.10

@mergify
Copy link
Contributor

mergify bot commented Jun 29, 2023 •
edited
Loading

backport 3.10

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Jun 29, 2023
@FinleyMcIlwaine @mergify
Do not always pass --quickjump to haddock (#9049)
234708b 
(cherry picked from commit 7b1a693)

# Conflicts:
#	Cabal/src/Distribution/Simple/Haddock.hs
@mergify mergify bot mentioned this pull request Jun 29, 2023
Merged
ulysses4ever pushed a commit that referenced this pull request Jun 30, 2023
@FinleyMcIlwaine @ulysses4ever
Do not always pass --quickjump to haddock (#9049)
3faee44 
(cherry picked from commit 7b1a693)
Kleidukos added a commit that referenced this pull request Jun 30, 2023
@mergify @FinleyMcIlwaine @Kleidukos
Do not always pass --quickjump to haddock (backport #9049) (#9073)
08e94a7 
Co-authored-by: Finley <finleymcilwaine@gmail.com>
Co-authored-by: Hécate Moonlight <Kleidukos@users.noreply.github.com>
@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jul 1, 2023
liskin added a commit to xmonad/X11 that referenced this pull request Nov 12, 2023
@liskin
ci: Only build haddock for hackage with the latest GHC
036f969 
Cabal 3.10.2.0 exposes a bug in Haddock shipped with GHC 9.0 and 9.2, so
we need to work around it by bumping the version of GHC/Haddock we use
for building/uploading docs to Hackage, and to prevent build failures we
don't ever try to build haddocks for Hackage with older versions of
GHC/Haddock.

Related: haskell/haddock#1582 (comment)
Related: haskell/cabal#8326
Related: haskell/cabal#9060
Related: haskell/cabal#9073
Related: haskell/cabal#9049
liskin added a commit to xmonad/X11-xft that referenced this pull request Nov 12, 2023
@liskin
ci: Only build haddock for hackage with the latest GHC
af373db 
Cabal 3.10.2.0 exposes a bug in Haddock shipped with GHC 9.0 and 9.2, so
we need to work around it by bumping the version of GHC/Haddock we use
for building/uploading docs to Hackage, and to prevent build failures we
don't ever try to build haddocks for Hackage with older versions of
GHC/Haddock.

Related: haskell/haddock#1582 (comment)
Related: haskell/cabal#8326
Related: haskell/cabal#9060
Related: haskell/cabal#9073
Related: haskell/cabal#9049
liskin added a commit to xmonad/xmonad that referenced this pull request Nov 12, 2023
@liskin
ci: Only build haddock for hackage with the latest GHC
f2aa84e 
Cabal 3.10.2.0 exposes a bug in Haddock shipped with GHC 9.0 and 9.2, so
we need to work around it by bumping the version of GHC/Haddock we use
for building/uploading docs to Hackage, and to prevent build failures we
don't ever try to build haddocks for Hackage with older versions of
GHC/Haddock.

Related: haskell/haddock#1582 (comment)
Related: haskell/cabal#8326
Related: haskell/cabal#9060
Related: haskell/cabal#9073
Related: haskell/cabal#9049
liskin added a commit to xmonad/xmonad-contrib that referenced this pull request Nov 12, 2023
@liskin
ci: Only build haddock for hackage with the latest GHC
192c0da 
Cabal 3.10.2.0 exposes a bug in Haddock shipped with GHC 9.0 and 9.2, so
we need to work around it by bumping the version of GHC/Haddock we use
for building/uploading docs to Hackage, and to prevent build failures we
don't ever try to build haddocks for Hackage with older versions of
GHC/Haddock.

Related: haskell/haddock#1582 (comment)
Related: haskell/cabal#8326
Related: haskell/cabal#9060
Related: haskell/cabal#9073
Related: haskell/cabal#9049
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ulysses4ever ulysses4ever ulysses4ever approved these changes

@coot coot coot approved these changes

@geekosaur geekosaur geekosaur approved these changes

@Kleidukos Kleidukos Kleidukos approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@FinleyMcIlwaine @coot @ulysses4ever @Kleidukos @geekosaur @Mikolaj @fgaz