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

Ignore missing .haddock files when generating documentation #362

Merged
merged 2 commits into from
Jul 24, 2018
Merged

Ignore missing .haddock files when generating documentation #362

merged 2 commits into from
Jul 24, 2018

Conversation

Profpatsch
Copy link
Contributor

@Profpatsch Profpatsch commented Jul 23, 2018 •
edited
Loading

Fixes #335.

For now only a test reproducing the issue:

$ bazel test //tests/haddock
INFO: Analysed target //tests/haddock:haddock (0 packages loaded).
INFO: Found 1 target and 0 test targets...
ERROR: /home/philip/code/bazel/rules_haskell/tests/haddock/BUILD:46:1: HaskellHaddock //tests/haddock:haddock-lib-b failed (Exit 1)
haddock: internal error: /nix/store/9ab4cswajxzxgy8s6khkr99sdp96xm9k-libc-0.1.0.0/share/doc/x86_64-linux-ghc-8.2.2/libc-0.1.0.0/html/libc.haddock: openBinaryFile: does not exist (No such file or directory)
Target //tests/haddock:haddock failed to build

Profpatsch
Profpatsch commented Jul 23, 2018
View reviewed changes
tests/ghc.nix Outdated
@@ -6,6 +6,7 @@ let haskellPackages = pkgs.haskell.packages.ghc822.override {
overrides = with pkgs.haskell.lib; self: super: rec {
lens-labels = super.lens-labels_0_2_0_1;
proto-lens = super.proto-lens_0_3_1_0;
libc = import ./haddock/libC.nix self pkgs;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This needs to be solved differently; my suggestion is creating a different ghcWithPackages for the tests.

@Profpatsch
Copy link
Contributor Author

Profpatsch commented Jul 23, 2018 •
edited
Loading

yay

screenshot

$ bazel test //tests/haddock
INFO: Analysed target //tests/haddock:haddock (0 packages loaded).
INFO: Found 1 target and 0 test targets...
INFO: From HaskellHaddock //tests/haddock:haddock-lib-b:
Warning: LibB: could not find link destinations for:
    LibCType
Warning: haddock missing for package libc
Target //tests/haddock:haddock up-to-date:
  bazel-bin/tests/haddock/haddock/testsZShaddockZShaddock-lib-deep-1.0.0
  bazel-bin/tests/haddock/haddock/testsZShaddockZShaddock-lib-a-1.0.0
  bazel-bin/tests/haddock/haddock/testsZShaddockZShaddock-lib-b-1.0.0
  bazel-bin/tests/haddock/haddock/index
INFO: Elapsed time: 2.078s, Critical Path: 1.95s
INFO: 5 processes: 5 processwrapper-sandbox.
INFO: Build completed successfully, 6 total actions

@Profpatsch Profpatsch requested review from mboes and guibou July 23, 2018 11:01
@Profpatsch
Copy link
Contributor Author

This is ready for review, but it still needs a solution for how to handle the ghc.nix issue I commented on above.

@Profpatsch Profpatsch changed the title WIP Ignore missing .haddock files when generating documentation Ignore missing .haddock files when generating documentation Jul 23, 2018
@mboes
Copy link
Member

mboes commented Jul 23, 2018

@guibou do you have a good solution to @Profpatsch's question above?

@Profpatsch
Copy link
Contributor Author

I’d use a superset of packages for the tests, where we can add more stuff when we need it.

@guibou
Copy link
Contributor

guibou commented Jul 23, 2018

Well done. I'm OK with your "hack" in the ghc.nix, that's part of the testing environment.

However I'll had more documentation inside ghc.nix and I'll do something simpler. You can have a look at the original PR I did for this issue (with a test similar to yours), where I only put the mtl packages available with dontHaddock flag.

df74df5#diff-8c75769c4d497fd08fde7536e40fa727

@Profpatsch
Copy link
Contributor Author

I'm OK with your "hack" in the ghc.nix, that's part of the testing environment.

Yes, after studying the source code a bit more it is now clear to me that the ghc.nix file is only used locally, for the protobuf library and the tests.

and I'll do something simpler. You can have a look at the original PR I did for this issue (with a test similar to yours), where I only put the mtl packages available with dontHaddock flag.

We can switch to an existing haskellPackages library for the example, but it won’t be as self-contained and reusable for other tests. Also it might give a compilation overhead (libC.nix is basically the most minimal cabal package input one can give to haskellPackages.callPackage).

@Profpatsch
Copy link
Contributor Author

Added two small comments to ghc.nix.

@guibou
Copy link
Contributor

guibou commented Jul 23, 2018

LGTM. You convinced me with the "small" example (which is, in fact, bigger than a oneliner pulling a package from hackage ;)

@Profpatsch
Copy link
Contributor Author

@mboes If you have no further corrections, we can merge.

@Profpatsch
Copy link
Contributor Author

Added a changelog entry, being a good citizen. :)

mboes
mboes approved these changes Jul 23, 2018
View reviewed changes
Copy link
Member

@mboes mboes 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 CI needs to go green.

@mboes mboes force-pushed the master branch 3 times, most recently from cb2b7ef to a65d39a Compare July 23, 2018 22:05
@Profpatsch
tests/haddock: Failing test for non-haddocked nix lib
2eec099 
When referencing a nix `haskellPackages` library that uses `dontHaddock` to
ignore broken upstream documentation generation, the `.haddock` file is not
generated, so haddock fails for our bazel build.

The `libc` import in `tests/ghc.nix` is temporary, it needs a better solution.
@Profpatsch
haskell/haddock: Skip haddock inputs with missing .haddock
7ec4986 
This adds a simple skip if the `.haddock` file is missing.

```
$ bazel test //tests/haddock
INFO: Analysed target //tests/haddock:haddock (0 packages loaded).
INFO: Found 1 target and 0 test targets...
INFO: From HaskellHaddock //tests/haddock:haddock-lib-b:
Warning: LibB: could not find link destinations for:
    LibCType
Warning: haddock missing for package libc
Target //tests/haddock:haddock up-to-date:
  bazel-bin/tests/haddock/haddock/testsZShaddockZShaddock-lib-deep-1.0.0
  bazel-bin/tests/haddock/haddock/testsZShaddockZShaddock-lib-a-1.0.0
  bazel-bin/tests/haddock/haddock/testsZShaddockZShaddock-lib-b-1.0.0
  bazel-bin/tests/haddock/haddock/index
INFO: Elapsed time: 1.970s, Critical Path: 1.82s
```
@Profpatsch Profpatsch merged commit cf0e639 into tweag:master Jul 24, 2018
@Profpatsch Profpatsch deleted the haddock-ignore-missing branch January 14, 2019 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@guibou guibou guibou approved these changes

@mboes mboes mboes approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Profpatsch @mboes @guibou