Skip to content

Conversation

@aherrmann
Copy link
Member

This is to make generated Haddocks available in the runfiles tree of targets that have a data dependency on a haskell_doc target.

Adds a regression test to //tests/haddock.

Comment on lines +65 to +66
# Fails in profiling mode due to missing haddocks for Deep.
"requires_dynamic",
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not a regression introduced with this PR but already the case on master. I.e. the following fails on master:

$ nix-shell --pure --run 'bazel test //tests/haddock/... -c dbg'
...
ERROR: /.../rules_haskell/tests/haddock/BUILD.bazel:18:1: HaskellHaddock //tests/haddock:haddock-lib-a failed (Exit 1) haddock_wrapper-haddock-lib-a failed: error executing command bazel-out/k8-dbg/bin/tests/haddock/haddock_wrapper-haddock-lib-a '--package-name=testsZShaddockZShaddock-lib-a' '--package-version=0' -D ... (remaining 96 argument(s) skipped)
...
tests/haddock/LibA.hs:10:1: error:
    Could not find module ‘Deep’
    There are files missing in the ‘haddock-lib-deep’ package,
    try running 'ghc-pkg check'.
    Use -v to see a list of the files searched for.
   |
10 | import Deep (deep_lib)
   | ^^^^^^^^^^^^^^^^^^^^^^
INFO: Elapsed time: 1.354s, Critical Path: 0.57s
INFO: 0 processes.
FAILED: Build did NOT complete successfully

This does not cause a CI failure on master, because we pass the flag --build_tests_only in profiling mode. This PR introduces a test case that depends on the haddock targets, which means they are now included despite --build_tests_only. We set the tag to disable the test on profiling builds.


return [DefaultInfo(
files = depset(html_dict_copied.values() + [index_root]),
runfiles = ctx.runfiles(html_dict_copied.values() + [index_root]),
Copy link
Contributor

Choose a reason for hiding this comment

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

The html_dict is huge and here we are building two temporaries copies of it.

I do wonder what can be the impact on performance. Something such as:

    l = html_dict_copied.values() + [index_root]
    return [DefaultInfo(
        files = depset(l),
        runfiles = ctx.runfiles(l),

may be better.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. It's tracking directories, not individual files, so one entry per target, but you're right, better not copy it unnecessarily.

@aherrmann aherrmann added the merge-queue merge on green CI label Apr 16, 2020
This is to make generated Haddocks avaible in the runfiles
tree of targets that have a `data` dependency on a
`haskell_doc` target.
@mergify mergify bot merged commit cf6beb4 into master Apr 16, 2020
@mergify mergify bot deleted the haddock_data branch April 16, 2020 14:49
@mergify mergify bot removed the merge-queue merge on green CI label Apr 16, 2020
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.

3 participants