Skip to content

haddock-project support public sublibraries #9821

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

Merged
merged 8 commits into from
Jun 30, 2024
Merged

Conversation

coot
Copy link
Collaborator

@coot coot commented Mar 19, 2024

  • Added argLibraryName to HaddockArgs
  • haddock-project: fixed main library name
  • haddock-project support for sublibraries

QA Notes

Running cabal haddock-project on a package with multiple public sublibraries should create ./haddocks directory which contains the package:sublibrary haddocks in ./haddocks/package-sublibrary directory. All cross-references will be resolved to files inside ./haddocks directory. ./haddocks directory should also contain both indexes: html and quick-jump index. Both indexes should contain references to identifiers from all public sublibraries.

When using cabal haddock-project --hackage the cross-references between sublibraries will be resolved to hackage (as described in #9852).

@coot
Copy link
Collaborator Author

coot commented Mar 21, 2024

I am getting a lot of test failures of this sort:

+Warning: Both <ROOT>/cabal.dist/home/.cabal and /home/coot/.config/cabal/config exist - ignoring the former.
+It is advisable to remove one of them. In that case, we will use the remaining one by default (unless '$CABAL_DIR' is explicitly set).

on my dev machine, I use cabal's support for xdg directory layout; is this related?

@coot coot force-pushed the coot/haddock-project-sublibs branch 2 times, most recently from 31b34fa to 0bd944e Compare March 22, 2024 09:15
@ulysses4ever
Copy link
Collaborator

Try removing ~/.cabal maybe.

@coot coot force-pushed the coot/haddock-project-sublibs branch 2 times, most recently from 291e98b to 223839d Compare March 22, 2024 16:49
@coot
Copy link
Collaborator Author

coot commented Mar 22, 2024

I think the ~/.cabal file comes from the test suite (I suspect it sets HOME variable in some particular way). We probably need to set some XDG env vars in the tests so cabal doesn't find the user's configuration.

@coot
Copy link
Collaborator Author

coot commented Mar 22, 2024

btw, I don't have ~/.cabal file in my HOME directory.

@coot coot marked this pull request as ready for review March 22, 2024 17:27
@coot coot force-pushed the coot/haddock-project-sublibs branch 4 times, most recently from 7ed8b05 to f13915c Compare March 23, 2024 12:40
@coot coot requested a review from andreabedini March 30, 2024 10:53
@coot coot changed the title coot/haddock project sublibs haddock project sublibs Apr 1, 2024
@coot coot changed the title haddock project sublibs haddock-project support public sublibraries Apr 1, 2024
@coot coot requested a review from Kleidukos April 2, 2024 19:23
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.

I'd appreciate if you wrote QA notes to ensure that this PR does for others what you expect it to.

Otherwise I don't see anything blocking. :)

LSubLibName sublib_name ->
prettyShow (packageName pkg_descr) ++ "-" ++ prettyShow sublib_name
_ -> prettyShow (packageName pkg_descr)
-- TODO: how Hackage should deal with sublibraries?
Copy link
Member

Choose a reason for hiding this comment

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

How do we resolve this question? Do you want to ask gbaz?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this needs to be considered.

I see two choices:

  • each sublibrary is available from it's own page;
  • All public sublibraries are available on the main library page

The former might be cumbersome (since making a revision of each sublibrary will need to make a revision of all of them).

For the latter one might be able to use cabal haddock-project --hackage maybe with some modifications to write a directory containing haddocks of all sublibraries which can be served by hackage. This might be not only more ergonomic but also the simplest way to support public sublibraries on hackage.

desclaimer: I have never contributed to hackage so please take my word with a grain of salt 😄.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

^ ping @gbaz

Copy link
Collaborator

Choose a reason for hiding this comment

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

The former might be cumbersome (since making a revision of each sublibrary will need to make a revision of all of them).

What do you mean by "revision" in this sentence? A cabal file revision on Hackage is a package-level thing so it would affect all package's libraries.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I mean package revisions Hackage. I was considering if a sublibrary is exposed on its own, e.g. hackage.com/package/mypackage-sublibrary while the main library is served from hackage.com/package/mypackage.

But if we publish sublibraries as part of the package, e.g. hackage.com/package/mypackage/sublibrary then it will be more consistent. Also finding sublibraries of a package will be simpler if links to them will be rendered on the package page.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think mypackage/sublibrary is the correct option here.

Copy link
Member

Choose a reason for hiding this comment

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

@coot what's the final word on this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I haven't changed it from mypackage-sublibrary to mypackage/sublibrary. We'll need to pass a different --base-url for sublibraries, but I already do that for test components and executables, so it's doable.

Copy link
Member

Choose a reason for hiding this comment

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

@coot cool, I'd love this to be documented somewhere so that we don't forget it exists

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I adjusted the paths, but there's more work either on cabal side or hackage side. If we have a LIB with a sublibrary SUBLIB, then LIB docs will be placed under LIB-0.1.0.0/doc/html/LIB and the documentation of the sublibrary will be placed under LIB-0.1.0.0/l/SUBLIB/doc/html/LIB/SUBLIB/.

There are various options from here to get sublibrary support on hackage:

  • use haddock-project on hackage - this might require some changes to haddock-project;
  • copy the sublibrary documentation to the right place;
  • as above, but let cabal do that. This might require changing the directory layout of dist-newstyle to be consistent (e.g. drop the l/t subdirs).

I think the first option is the best one - but this is outside of this PR anyway.

@coot
Copy link
Collaborator Author

coot commented Apr 2, 2024

@Kleidukos I added the QA Notes: and checked that it works as expected 😄. Do you know who else could review this PR?

@coot
Copy link
Collaborator Author

coot commented Apr 2, 2024

It might be worth noting that in an earlier version of this PR I experimented using package:sublibrary as path in ./haddocks, but there are two problems with it:

  • : is an invalid character on Windows
  • : when used in URLs, requires either relative or absolute paths (which can be done, even without haddock changes)

Maybe it's worth to use the : syntax as we are used to do cabal build package:sublibrary, and letting Windows users use - instead, even though it slightly complicates things. Anyway, for now, I left it outside of this PR. The drawback is that one cannot have some-package:sublib and some:package-sublib in a single project.

@coot coot force-pushed the coot/haddock-project-sublibs branch 2 times, most recently from 9a5b6a7 to 320b408 Compare April 6, 2024 19:48
@andreabedini
Copy link
Collaborator

As always I am late to the party. Looking at HaddockProjectFlags, how come the concept of project leaks into Cabal? A project is exclusively a cabal-install concept. Are we using the word "project" with different meanings?

@Kleidukos
Copy link
Member

As always I am late to the party. Looking at HaddockProjectFlags, how come the concept of project leaks into Cabal? A project is exclusively a cabal-install concept. Are we using the word "project" with different meanings?

@andreabedini it's the whole point of this experimental command: To generate the haddocks of a whole cabal project (1 or more local packages) to a single documentation artifact

@andreabedini
Copy link
Collaborator

Of course, but it's a cabal-install command, not a Setup.hs command. While HaddockProjectFlags is defined in Cabal.

@coot coot force-pushed the coot/haddock-project-sublibs branch 4 times, most recently from 7008c7d to 2be7de9 Compare June 28, 2024 15:18
@coot
Copy link
Collaborator Author

coot commented Jun 28, 2024

@Kleidukos I addressed your comments regarding hackage.

@Kleidukos Kleidukos self-requested a review June 28, 2024 16:10
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.

Alright, let's get this merged. :)

I'm excited to use it and see which bugs we've missed. ❤️

Thanks for the work @coot

coot added 7 commits June 28, 2024 19:53
The main library must use the package name rather than `UnitId`,
otherwise the links from other sublibraries will not work.
This commit makes haddock-project handle sublibraries.

The commit changes how `cabal haddock` works, changing the layout
in the `dist-newstyle` folder.  With this change `haddock` subcommand
will install `package:sublib` component in a directory `package-sublib`
under `l/sublib/doc/html/`.
When running haddock-project `extra-doc-files` should be copied to
a subdirectories of the `argOutputDir` corresponding to the component.
This doesn't affect `haddock` command.
Haddock should know the sublibrary name rather than just the package.
For sublibraries `package_name:sublib_name` is used. The name is
recorded in the `.haddock` file and used when haddock creates the
`index.html` file, e.g. when called by `haddock-project` command.
`--use-unicode` is added to `cabal haddock` and `cabal haddock-project`.
One cannot simply use `--haddock-option="--use-unicode"` because it
makes haddock to fail when building indexes.
Link test suites & benchmarks from the index.html; Provide the right
`--base-url` flag for them.
@coot coot force-pushed the coot/haddock-project-sublibs branch from 2be7de9 to fb68bc4 Compare June 28, 2024 18:10
@coot
Copy link
Collaborator Author

coot commented Jun 28, 2024

I cleaned up the commit messages and updated the changelog entry.

@coot coot added the merge me Tell Mergify Bot to merge label Jun 28, 2024
@coot coot force-pushed the coot/haddock-project-sublibs branch from fb68bc4 to b118edb Compare June 30, 2024 15:51
@coot coot merged commit 8466a8f into master Jun 30, 2024
57 checks passed
@coot coot deleted the coot/haddock-project-sublibs branch June 30, 2024 20:39
@mergify mergify bot added the merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days label Jul 2, 2024
@geekosaur
Copy link
Collaborator

Should this have closed #9577?

@ulysses4ever
Copy link
Collaborator

@geekosaur I think so

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
attention: needs-manual-qa PR is destined for manual QA attention: needs-review cabal-install: cmd/haddock cabal-install: cmd/haddock-project 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
Status: To Test
Development

Successfully merging this pull request may close these issues.

7 participants