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 assertion failure when combining build-tool-depends and --enable-documentation #9446

Merged
merged 2 commits into from
Nov 23, 2023

Conversation

mpickering
Copy link
Collaborator

@mpickering mpickering commented Nov 14, 2023

Fix assertion failure when combining build-tool-depends and --enable-documentation

The setDocumentation function was modifying the elaborated package
after the hash was computed. This led to the assertion failing as the
computed hash was different to what was computed in the initial install
plan.

Therefore in order to fix this we either needed to:

. Set elabBuildHaddocks = False at the point where the hash is
initially computed.
2 Verify that elabBuildHaddocks = True will not lead to unexpected
results.

The latter has been implemented.

The elabBuildHaddocks option is only consulted in
hasValidHaddockTargets, at which point documentation building the
executable component is disabled because elabHaddockExecutables is
False.

In the added test we ensure this by checking that we didn't build
documentation for the executable which is built because of
build-tool-depends.

Fixes #6006 #8313

Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

Thank you!

@Mikolaj
Copy link
Member

Mikolaj commented Nov 17, 2023

BTW, the CI failure looks like a server filesystem or network issue, so let me rebase to restart it...

@Mikolaj
Copy link
Member

Mikolaj commented Nov 17, 2023

@mergify rebase

Copy link
Contributor

mergify bot commented Nov 17, 2023

rebase

✅ Branch has been successfully rebased

@mpickering
Copy link
Collaborator Author

I accidentally force pushed to the wrong branch, should be fixed now.

@Mikolaj
Copy link
Member

Mikolaj commented Nov 20, 2023

Time for the merge label?

@mpickering mpickering added the merge me Tell Mergify Bot to merge label Nov 20, 2023
@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Nov 23, 2023
@Mikolaj
Copy link
Member

Mikolaj commented Nov 23, 2023

This got stuck due to changes CI mandatory tests rule. Rebasing...

@Mikolaj
Copy link
Member

Mikolaj commented Nov 23, 2023

@mergify rebase

Copy link
Contributor

mergify bot commented Nov 23, 2023

rebase

✅ Branch has been successfully rebased

…documentation

The `setDocumentation` function was modifying the elaborated package
after the hash was computed. This led to the assertion failing as the
computed hash was different to what was computed in the initial install
plan.

Therefore in order to fix this we either needed to:

1. Set elabBuildHaddocks = False at the point where the hash is
   initially computed.
2. Verify that elabBuildHaddocks = True will not lead to unexpected
   results.

The latter has been implemented.

The elabBuildHaddocks option is only consulted in
`hasValidHaddockTargets`, at which point documentation building the
executable component is disabled because elabHaddockExecutables is
False.

In the added test we ensure this by checking that we didn't build
documentation for the executable which is built because of
build-tool-depends.

Fixes haskell#6006 haskell#8313
@mergify mergify bot merged commit b3e024a into haskell:master Nov 23, 2023
45 checks passed
@andreasabel andreasabel added re: build-tool Concerning `build-tools` and `build-tool-depends` re: enable-documentation Concerning option/field {en,dis}able-documentation labels Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 re: build-tool Concerning `build-tools` and `build-tool-depends` re: enable-documentation Concerning option/field {en,dis}able-documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion failure at Distribution/Client/ProjectPlanning.hs:242:5 with --enable-documentation
4 participants