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

Module listings in the presence of Backpack #577

Open
ezyang opened this issue Mar 21, 2017 · 9 comments
Open

Module listings in the presence of Backpack #577

ezyang opened this issue Mar 21, 2017 · 9 comments

Comments

@ezyang
Copy link
Contributor

ezyang commented Mar 21, 2017

Let's talk about package index pages; you know, the thing you see when you navigate to a package page like https://hackage.haskell.org/package/time

Currently, Hackage duplicates the algorithm for rendering a module tree with Haddock. Here is Hackage's copy of ModuleForest: https://github.com/haskell/hackage-server/blob/master/Distribution/Server/Packages/ModuleForest.hs and here is Haddock's copy https://github.com/haskell/haddock/blob/master/haddock-api/src/Haddock/ModuleTree.hs Haddock generated its index from the list of modules that would have been passed to GHC to compile; Hackage generates it from a flattened package description. For the longest time, this didn't really cause any problems because the module tree format didn't ever really change (just read it off of exposed-modules).

Unfortunately, I recently added two new features to Cabal which have broken this assumption:

  • Reexported modules means that exposed modules are not limited to exposed-modules; they may also be defined in reexported-modules. This has meant that Hackage does not actually display reexported modules. To make matters worse, if reexported modules are implemented as links to the reexported module in another package (a reasonable strategy) this logic must be duplicated in both Haddock and Hackage. And the cherry on top: Hackage doesn't even have enough information to actually compute this correctly. More context at Support reexported-modules haddock#563

  • Signatures can be inherited from packages, and we don't make it mandatory for a user to specify all signatures in their Cabal file (we can infer it when we are configuring the package.) But there will be Haddock documentation for every real signature in the package. Once again, Hackage has no way of figuring this out when it is generating its index.

I want to argue that it is morally wrong for Hackage to be in the business of generating the module tree that connects to the documentation in question. Not only does Hackage not have enough information to do it properly, Haddock is already in the business of computing it.

So, I think the correct model for Haddock to generate the HTML for the module tree, and then Hackage to transclude it into the index page, in much the same way Hackage transcludes the README into the index page.

Here's the annoying thing though: current Haddock, when invoked by Cabal to build documentation for Hackage, doesn't actually generate the index, because it is passed the --use-contents= flag which stops Haddock from generating the index. And even if it did generate the index, the HTML we are interested in is embedded inside a div inside the index HTML proper.

So, it would seem, to properly fix this, we would have to modify Haddock to generate a module tree which Hackage can transclude in. This is not great, since Haddock is on the same release cycle as GHC. We can always fall back on the old code if Haddock doesn't support this, but this means we can't really delete anything.

An alternative, pragmatic solution is to continue limping along with some less-than-perfect-but-good-enough module tree generation code in Hackage. For example, to get accurate signature listing, you'd simply be forced to list every signature explicitly in your Cabal file to help Cabal out. reexported-modules would remain a disaster for the forseeable future, or perhaps we can levy a Haddock level fix to copy in the documentation.

What do people think?

@gbaz
Copy link
Contributor

gbaz commented Mar 21, 2017

"Haddock generated its index from the list of modules that would have been passed to GHC to compile; Hackage generates it from "

I think you accidentally a phrase.

@ezyang
Copy link
Contributor Author

ezyang commented Mar 21, 2017

Fixed thanks.

@hvr
Copy link
Member

hvr commented Mar 21, 2017

So, I think the correct model for Haddock to generate the HTML for the module tree, and then Hackage to transclude it into the index page, in much the same way Hackage transcludes the README into the index page.

as already mentioned on IRC, here I disagree; I think Hackage should have full control about how to render the HTML for the documentation tree; it should instead get all it needs to know about the module tree structure from Haddock, so it can generate the HTML. Same goes for README's btw, which we render ourselves, so we have full control over CSS and what HTML features we support, and can make sure things seamlessly integrate with the rest of the package cover page styling.

PS: We also want to know an approximate module tree for when Haddock hasn't been able to build the docs yet (or fails to build them)

@ezyang
Copy link
Contributor Author

ezyang commented Mar 21, 2017

An alternate possibility which solves the signatures problem is for us to probe the TarIndex of the documentation tarball to find all files which "look like" modules; in practice, files which are capitalized, to find the complete signatures list.

@gbaz
Copy link
Contributor

gbaz commented Mar 21, 2017

In any case, a major concern is that the builder is in better shape than it used to be but should never be relied on to always work -- packages may be unbuildable for a variety of reasons that the builder can never help with.

So it would be wrong to not be able to even generate a module tree when the builder fails.

On the reexports thing I vote to just fix it in hackage. (see also: #63)

On the backpack inferred signature thing I can't follow the exact problem or solution being described.

@ezyang
Copy link
Contributor Author

ezyang commented Mar 21, 2017

So it would be wrong to not be able to even generate a module tree when the builder fails.

Yes, we would have to fall back, so we wouldn't really get to delete any code.

@ezyang
Copy link
Contributor Author

ezyang commented Mar 21, 2017

OK, here is my proposed patch: 389d9a4

hvr pushed a commit that referenced this issue Jul 16, 2017
This is a partial patch which improves Hackage package
description rendering of modules.  Now, instead of
ignoring reexported-modules and signatures, they get
displayed.

This is incomplete because:

    - reexported-module links will always be broken, because
      we'll be testing for HTML files which will never exist
      (Haddock doesn't currently generate HTML for reexported
      modules, although it may in the future.)

    - Signatures might be inherited from dependencies (and
      even get Haddock documentation for them), but we won't
      display them unless they are explicitly listed in
      signatures.

Signed-off-by: Edward Z. Yang <ezyang@mit.edu>
(cherry picked from commit 389d9a4; relates to #577)
@hvr
Copy link
Member

hvr commented Jul 16, 2017

@ezyang I've merged your proposed patch, as to me it represented an incremental improvement over the current state. If a better approach/patch comes along we can upgrade... :-)

@adamgundry
Copy link
Member

This is an issue for the optics package as we rely heavily on reexported-modules (see well-typed/optics#237). Looking at the corresponding pages on Hackage and Stackage I noticed that the latter omits reexported-modules entirely. While that isn't ideal either, it does avoid the long list of links with no docs.

Perhaps a better interim solution would be for Hackage to divide the "Modules" list into "Modules" and "Re-exported Modules"? Then the former would have docs and the latter would not.

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

No branches or pull requests

4 participants