ros-infrastructure / rosdoc2

Command-line tool for generating documentation for ROS 2 packages.
Apache License 2.0
29 stars 9 forks source link

wrapping of exhale is not valid #44

Open svenevs opened 2 years ago

svenevs commented 2 years ago

Setting breathe_default_project has been a hard requirement for exhale since its inception since it was only designed with consideration of single-project builds. This is a core design flaw, but at it's root in the absence of breathe_default_project you must specify :project: <some key in breathe_projects> to a given .. doxygen*:: directive, and in all honesty I didn't know how to deal with it.

https://github.com/ros-infrastructure/rosdoc2/blob/40a07e0ff34a94f5524c5f63e6efd22703f3aba8/rosdoc2/verbs/build/builders/sphinx_builder.py#L64-L66

I've tested against rcl_lifecycle but it seems likely that it's only working as a fluke.

https://github.com/svenevs/exhale/pull/148 prevents the wrapping byproduct but I'm not confident it is a full resolution -- since right now next(iter(breathe_projects.keys())) may result in the wrong entry. That said, I think the issue is (hopefully) better solved here.

  1. Probably the correct solution is to actually configure breathe_default_project correctly in your wrapper, rather than set an arbitrary value.
  2. If you make your custom specifications mapping include :project: the right thing, this would also bypass this as an underhanded implementation detail of that PR.
  3. If more than one breathe_project is going to get executed in a single run of sphinx-build, then this is where things get tricky.

Right now there's some band-aids for multiproject support that are currently functional via a monkeypatch hack which was only recently revived (and consequently broke rosdoc2). https://github.com/svenevs/exhale/issues/27

If you are building more than one breathe project in a single execution of sphinx-build, you may be best off vendoring the monkeypatch. The underlying problem is that (again, core design flaw), exhale.configs is a module. It is unclear to me if you need to support multiple breathe projects, if you do right now all I can do is make the monkeypatch work again and suggest subscribing to 27 over there since resolving it will include additional breaking changes.

rkent commented 1 month ago

I'm trying to understand this issue, and what if anything it would take to resolve it.

If I might reword, the issue is that rosdoc2 sort of implies that multi project support exists, but only takes the first project. But AFAICT there is no multi-project support in rosdoc2. breathe_default_project = next(iter(breathe_projects.keys())) may be an odd way to get the one project, but it does seem to work.

I don't think there is any action required here.