GNOME Bugzilla – Bug 743182
Automatically support PACKAGE variables as XML entities
Last modified: 2015-07-22 19:25:44 UTC
Here’s a series of WIP (unfinished, please don’t commit them!) patches to generate an XML file defining some entities to represent the PACKAGE variables set by configure. The idea is that when bootstrapping documentation, users rarely actually replace the ‘[VERSION]’ placeholders in the generated docs.xml file. If they do, the build system faff to generate and include a version.xml file is a pain. So if gtk-doc can automate it, that would be great. I’ve spent a little while on these patches. However, I’ve hit some problems with XInclude relative paths which I cannot work out. Specifically, the path the gtkdocentities.ent file is loaded from is resolved relative to two different directories when xsltproc processes docs.xml and the files in xml/. I can’t work out how to fix that.
Created attachment 294885 [details] [review] make: Use SH status variables not automake special variables When checking the status code of an executed process, use the SH variable $$?, rather than the automake $(?) special variable (which evaluates to the recipe prerequisites).
Created attachment 294886 [details] [review] make: Generate an entities file on build containing PACKAGE variables Users frequently want to substitute PACKAGE_VERSION into the build documentation, but end up having to do entity substitutions and automake integration for it themselves, which is tricky to get right. Generate an entity file automatically on build, containing all the PACKAGE_* variables. Use it in the default generated main XML file, so that new modules using gtk-doc don’t have to worry about this any more.
Created attachment 294887 [details] [review] mkhtml: Fix quoted path handling Support passing the --path and its value separately, which allows SH to do its quotation mark handling correctly on the value.
Created attachment 294888 [details] [review] make: Run gtkdoc-mkhtml in the top-level documentation directory Rather than in the HTML directory. This makes path management a little simpler. Also pass the xml/ directory path to xsltproc as an entity lookup path. This apparently doesn’t work.
It would be easier to review this, if it were multiple bugs. The "make: Use SH status variables not automake special variables " patch is already submitted. I have no idea how to make this accordingly in the new bugzilla :/ The "mkhtml: Fix quoted path handling" cannot be changed as it would break backwards compatibility. Can you give me an example what is not working? For "make: Run gtkdoc-mkhtml in the top-level documentation directory" it does not look simpler to me. Finally the "make: Generate an entities file on build containing PACKAGE variables" is actually a nice idea. Can you ensure it still applies and also cover it in the docs? Maybe you can move the other patches to separate tickets.
Comment on attachment 294885 [details] [review] make: Use SH status variables not automake special variables (To mark a patch as committed, click the ‘details’ link on the attachment, then ‘edit details’ at the top. It’s not great. :-( )
(In reply to Stefan Sauer (gstreamer, gtkdoc dev) from comment #5) > The "mkhtml: Fix quoted path handling" cannot be changed as it would break > backwards compatibility. Can you give me an example what is not working? I can’t remember, but I suspect it was a path with spaces in. > For "make: Run gtkdoc-mkhtml in the top-level documentation directory" it > does not look simpler to me. It means there’s no `cd`-ing happening, which means all paths can be specified as relative to the $(top_builddir). > Finally the "make: Generate an entities file on build containing PACKAGE > variables" is actually a nice idea. Can you ensure it still applies and also > cover it in the docs? Maybe you can move the other patches to separate > tickets. It’s not a finished patch — see comment #0. The other patches were needed to get it to (almost) work, as they were steps towards fixing the path handling for XML includes, which is needed for the approach I took to adding new XML entities. However, as I said in comment #0, I couldn’t quite get the patch to work, and don’t know enough about xsltproc to work out what’s wrong. I was hoping someone with more xsltproc experience would see the problem.
For buzztrax I am doing this: https://github.com/Buzztrax/buzztrax/blob/master/docs/reference/bt-cmd/buzztrax-cmd-docs.xml#L4-5 but I also copy the file around - probably the same issue that you see. https://github.com/Buzztrax/buzztrax/blob/master/Makefile.am#L94-95 Now I also understand why you'd like to try to avoid the "cd html" for gtkdoc-mkhtml.
(In reply to Stefan Sauer (gstreamer, gtkdoc dev) from comment #8) > For buzztrax I am doing this: > https://github.com/Buzztrax/buzztrax/blob/master/docs/reference/bt-cmd/ > buzztrax-cmd-docs.xml#L4-5 > > but I also copy the file around - probably the same issue that you see. > https://github.com/Buzztrax/buzztrax/blob/master/Makefile.am#L94-95 Ah, so you solved the problem by having two copies of the entities file? One in docs/, one in docs/reference/, so that the ../../version.entities reference which xsltproc tries to resolve from docs/reference/bt-cmd/buzztrax-cmd-docs.xml and from docs/reference/bt-cmd/xml/blah.xml always resolves? That’s a bit of a hack, but I would be fine with it in the patch.
Created attachment 306791 [details] [review] make: Generate an entities file on build containing PACKAGE variables Users frequently want to substitute PACKAGE_VERSION into the build documentation, but end up having to do entity substitutions and automake integration for it themselves, which is tricky to get right. Generate an entity file automatically on build, containing all the PACKAGE_* variables. Use it in the default generated main XML file, so that new modules using gtk-doc don’t have to worry about this any more.
Created attachment 306792 [details] [review] mkhtml: Fix quoted path handling Support passing the --path and its value separately, which allows SH to do its quotation mark handling correctly on the value.
Created attachment 306793 [details] [review] make: Run gtkdoc-mkhtml in the top-level documentation directory Rather than in the HTML directory. This makes path management a little simpler. Also pass the xml/ directory path to xsltproc as an entity lookup path. This apparently doesn’t work.
Here’s an updated patch which *does* work! I used the same approach as you: have two copies of the entities file. Patch #306792 and patch #306793 have been updated, but are no longer necessary, so I’ve marked them as obsolete. These are the latest versions, uploaded for posterity.
Review of attachment 306791 [details] [review]: ::: gtkdoc-mkdb.in @@ +386,3 @@ <!ENTITY % local.common.attrib "xmlns:xi CDATA #FIXED 'http://www.w3.org/2003/XInclude'"> + <!ENTITY % gtkdocentities SYSTEM "gtkdocentities.ent"> + %gtkdocentities; There is now a function called MakeDocHeader() in gtkdoc-mkdb. Could you try to enhance this to rewrite the entity-path. For a start you could only do this for this specific line, that is replace: <!ENTITY % gtkdocentities SYSTEM "gtkdocentities.ent"> by <!ENTITY % gtkdocentities SYSTEM "../gtkdocentities.ent"> if $tag != "book". WDYT?
Created attachment 307006 [details] [review] make: Generate an entities file on build containing PACKAGE variables Users frequently want to substitute PACKAGE_VERSION into the build documentation, but end up having to do entity substitutions and automake integration for it themselves, which is tricky to get right. Generate an entity file automatically on build, containing all the PACKAGE_* variables. Use it in the default generated main XML file, so that new modules using gtk-doc don’t have to worry about this any more. --- Updated patch which modifies MakeDocHeader. Much neater, thanks.
Review of attachment 307006 [details] [review]: Almost there! ::: gtkdoc-mkdb.in @@ +3088,3 @@ + $header .= "<!ENTITY % gtkdocentities SYSTEM \"xml/gtkdocentities.ent\">\n"; + $header .= "%gtkdocentities;\n"; + } Shouldn't this be just removing the '../' from the header? if ($tag eq "book") { $doctype_header =~ s#<!ENTITY % gtkdocentities SYSTEM \"../([a-zA-Z./]+)\">#<!ENTITY % gtkdocentities SYSTEM \"$1\">#g; }
(In reply to Stefan Sauer (gstreamer, gtkdoc dev) from comment #16) > Review of attachment 307006 [details] [review] [review]: > > Almost there! > > ::: gtkdoc-mkdb.in > @@ +3088,3 @@ > + $header .= "<!ENTITY % gtkdocentities SYSTEM > \"xml/gtkdocentities.ent\">\n"; > + $header .= "%gtkdocentities;\n"; > + } > > Shouldn't this be just removing the '../' from the header? I don’t think so. MakeDocHeader() is called to generate the initial main DocBook file if it doesn’t already exist, and we want to include the gtkdocentities stuff in that. I guess it would be equivalent to include it in the initial setting of $doctype_header in run(). Which would you prefer? Probably easiest if you just make the changes to neaten it up yourself, since you know where you would prefer the code to be. :-)
(In reply to Philip Withnall from comment #17) > (In reply to Stefan Sauer (gstreamer, gtkdoc dev) from comment #16) > > Review of attachment 307006 [details] [review] [review] [review]: > > > > Almost there! > > > > ::: gtkdoc-mkdb.in > > @@ +3088,3 @@ > > + $header .= "<!ENTITY % gtkdocentities SYSTEM > > \"xml/gtkdocentities.ent\">\n"; > > + $header .= "%gtkdocentities;\n"; > > + } > > > > Shouldn't this be just removing the '../' from the header? > > I don’t think so. MakeDocHeader() is called to generate the initial main > DocBook file if it doesn’t already exist, and we want to include the > gtkdocentities stuff in that. > > I guess it would be equivalent to include it in the initial setting of > $doctype_header in run(). Which would you prefer? Probably easiest if you > just make the changes to neaten it up yourself, since you know where you > would prefer the code to be. :-) Yes please add the default in line 384 ff. And since that one would be for refentry, it would contain a "../" path and that's why I suggested to do the search and replace in MakeDocHeader(). If its any easier feel free to change the $doctype_header to contain the header for book without a path on the entity and then patch it in MakeDocHeader() for ($tag ne "book").
Oh, and it would be nice to: * also patch the makefile under tests and update tests/*/docs/*-docs.xml * update help/manual/C/index.docbook, especially add a section "modernizing-gtk-doc-1.25" that tells people how to migrate. These can be done as separate patches though :)
Oki, let me take over from here. Thanks for the idea and initial patches!
commit ca106ece4dacff17dab105dd65f5c1046a03e391 Author: Stefan Sauer <ensonic@users.sf.net> Date: Wed Jul 15 10:28:43 2015 +0200 help: document how to use the new entities feature commit df6146ec50c0e8a48309251a2174ea4abe75fddb Author: Stefan Sauer <ensonic@users.sf.net> Date: Wed Jul 15 10:17:31 2015 +0200 tests: use the generated entities in the tests commit 98b88db275738b7235248a503e8fa1cb4c42e01a Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Tue Dec 16 13:38:02 2014 +0000 make: Generate an entities file on build containing PACKAGE variables Users frequently want to substitute PACKAGE_VERSION into the build documentation, but end up having to do entity substitutions and automake integration for it themselves, which is tricky to get right. Generate an entity file automatically on build, containing all the PACKAGE_* variables. Use it in the default generated main XML file, so that new modules using gtk-doc don’t have to worry about this any more. https://bugzilla.gnome.org/show_bug.cgi?id=743182
Great! Do you think this is worth blogging about? (Sorry I didn’t get time to fix up and push the patches before I went on holiday in the last week. Thanks for sorting them out.)
Sure, some publicity can't hurt.