Skip to content

Conversation

@chrissie-c
Copy link
Contributor

This tool (currently) isn't used in libqb, but may be in the future.
It's here so it can be used by other cluster packages without them having their own private copy. It's is currently used by knet (where this code comes from) and we intend to also use it in corosync.

I'm not claiming this is the ideal place for this utility, but having a totally separate main project for it seems unnecessary.

This (currently) isn't used in libqb, but may be in the future.
It's here so it can be used by other cluster  packages without
them having their own copy.
@fabbione fabbione self-requested a review March 9, 2020 13:42
Copy link
Member

@fabbione fabbione left a comment

Choose a reason for hiding this comment

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

ack from me

@kgaillot
Copy link

kgaillot commented Mar 9, 2020

I don't have an opinion one way or the other, but a separate project would be worthwhile if a different release cadence would be useful, or if this might take off among projects that don't need libqb.

FYI, I've been working on converting Pacemaker (non-API) documentation to sphinx. Sphinx has doxygen-like capabilities for Python, and is able to output python API documentation as man, html, etc. The "hawkmoth" sphinx extension extends this to C, but I haven't looked into it yet. Alternatively the "breathe" project takes doxygen output and converts it for use by sphinx. I like the idea of having one system for API and non-API documentation, so I might look into that, though I don't know when I'll have time for it.

@chrissie-c
Copy link
Contributor Author

We have considered this, but the tool is really in "it works, leave it" mode. Most people wanting documentation from doxygen these days want more than just man pages AFACIT,but we're low enough in the stack that they are our main documentation (for APIs at least).

The resulting package is deliberately not prefixed with 'libqb' just so we can spin it off if it becomes necessary!

@fabbione
Copy link
Member

fabbione commented Mar 9, 2020

retest this please

@wferi
Copy link
Contributor

wferi commented Mar 9, 2020

Please reconsider creating a separate project for doxygen2man. Cross-building is such a pain if you need to run some of your code during the build... See all those *_BUILD* variables in Kronosnet. (It will bite if you ever want to use it in libqb.) Thanks.

@fabbione
Copy link
Member

@chrissie-c I am fine either ways, if we want to make it a separate project or not. We have all the infra in place for it. Just need some autotools magic here and there.

@jfriesse
Copy link
Member

jfriesse commented Mar 10, 2020

+1 for ether way - separate project or libqb (actually, best place would be in doxygen itself where it really belongs to as one of the output options, but that's probably another story).

@wenningerk
Copy link

@wferi: that is where cross-build-environments like scratchbox shine - even useful for basic testing

@fabbione
Copy link
Member

@wferi this version of doxygen2man would replace the one in knet and shared across different projects. To be honest, I understand your concerns about cross-compile, but we have the autotools snippets in knet already (just need to be moved over here) and doxygen2man is too small to make it a project on its own IMHO.

@jfriesse
Copy link
Member

@wferi We discussed this problem a bit on IRC, and there are basically two problems:

  • If doxygen2xml is a separate package and libqb dependency is not removed and if it is used for libqb doc then there is a circular dependency
  • If it is in libqb, then there is problem with cross-compilation

I think having circular dependency is probably worse of two evils, but you may have some idea how to solve it in a better way?

@wenningerk
Copy link

In case of cross-building:

BuildRequires: qemu-user-static

as long as the target isn't too exotic ...

@wferi
Copy link
Contributor

wferi commented Mar 11, 2020

Oh well, I forgot about the libqb dependency of doxygen2xml. As far as I know, it's only the hash implementation, but I've got no idea what it would take to remove the dependency.

  • The circular dependency is rather unfortunate, but not the end of the world, because it affects the documentation generation only, libqb is functional without it. This means bootstrapping would require a (partial) rebuild of libqb after stage1 (or a separate arch-all build to generate the libqb-doc package).
  • Moving the current cross-compilation support from Kronosnet into libqb won't be enough when libqb starts to use doxygen2xml, because double compilation will be needed then. But it's something to build on at least. I'm not sure I'd dare to recommend requiring qemu-user-static in a face-to-face meeting with the cross-bootstrap crew, though. :)

So yeah, neither option is particularly pretty, but both could be made to work. The easiest solution is to not use doxygen2xml in the libqb build, then neither problem arises regardless if it's a part of libqb or a separate project.

@jfriesse
Copy link
Member

jfriesse commented Mar 11, 2020

@wferi: Yup, agree, there is no good way. Both paths are explored already (crosscompilation see openwrt, ... and no qemu is needed, circular dependency see binutils/gcc/make/....). Removal of the map usage is probably the way, but I'm unsure how much work it means - for list we could use sys/queue.h, which is everywhere and maintained by libc maintainers.

@fabbione
Copy link
Member

@wferi I honestly have a hard time worrying too much about cross compilation. How many distros are actually cross building the HA stack? if so why? if an arch cannot build the stack "quickly" I doubt it even has a use case to run the stack itself...

@wferi
Copy link
Contributor

wferi commented Mar 11, 2020

I don't think it's widespread practice, but when we broke cross-compilation in Kronosnet, I got a bug report about it. (I've got another one about Pacemaker, only half of which is fixed, because help2man is inherently incompatible with the concept.) People have strange ideas...

@chrissie-c
Copy link
Contributor Author

Hang on though, if we're worried about cross compilation (and TBH I'm not, very) then surely the current situation is also bad for it. If we move doxygen2man into libqb and don't use it for those man pages then that should actually solve the problem.

@wferi
Copy link
Contributor

wferi commented Mar 12, 2020

Cross compilation is fixed in Kronosnet, but we don't ship doxy2xml from there, only use it internally. Not using doxygen2xml in libqb is what I referred to as the "easiest solution" above, and yes, that would provide doxygen2xml packages without any complication (from libqb or from a separate project depending on it).

@fabbione
Copy link
Member

@wferi it should be easy enough to have 2 doxygen2xml targets in the Makefile, one for local use (as we do for knet, built with the local compiler, say: doxygen2-xml-internal) and one for general shipping (cross compiled normally as any other binary). It´s a matter of enabling it in the Makefile and we should all be happy.

@wferi
Copy link
Contributor

wferi commented Mar 12, 2020

Absolutely. I just meant to point out that this part is not necessary until libqb starts to use doxygen2man (see, I get the name right occasionally) so it's rightfully missing from this PR, but it's coming. On the other hand, using a separate project does not require double compilation (and also decouples the release schedules), but creates a dependency cycle when libqb starts to use doxygen2man, as Honza pointed out. In a different arrangement: moving doxygen2man into

  • libqb
    • pro: less administration (no project creation)
    • con: requires double compilation pixie dust once libqb starts to use doxygen2man
  • separate project:
    • pro: independent release schedules
    • con: dependency cycle once libqb starts to use doxygen2man (unless the hash implementation gets replaced)

... as I see it at the moment (feel free to correct). The choice is yours. :)

@fabbione
Copy link
Member

I vote to keep doxygen2man inside libqb and have double compilation. It´s such a small binary that 2 compilation is much much less overhead than a separate project. Clearly this require fixing this PR to handle what we already discussed.

@fabbione
Copy link
Member

All, I am moving this PR here: #388

@fabbione fabbione closed this Mar 13, 2020
@kgaillot kgaillot mentioned this pull request Jul 13, 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.

6 participants