[libvirt] [PATCH] Generate HACKING from docs/hacking.html.in

[Eric Blake]

On 11/11/2010 01:39 PM, Matthias Bolte wrote:
> Tweak pre tags in docs/hacking.html.in to achieve proper
> indentation of their plaintext representation.
> 
> Also use more b/i/code tags in docs/hacking.html.in.
> ---
>  HACKING              |  602 +++++++++++++++++++++++++++++---------------------

A lot of this diff is whitespace; the rest looks like it is pulling in
missed bits.  Overall, it looks pretty decent; and it's certainly more
maintainable!

>  Makefile.am          |    8 +
>  docs/hacking.html.in |  351 ++++++++++++++++--------------

Would you mind splitting this into two patches?  One that cleans up just
hacking.html.in, and the rest that covers going from a clean source file
into the new generated HACKING file.

> +++ b/Makefile.am
> @@ -48,6 +48,8 @@ EXTRA_DIST = \
>  pkgconfigdir = $(libdir)/pkgconfig
>  pkgconfig_DATA = libvirt.pc
>  
> +all: NEWS HACKING

Hmm.  This means that everyone will attempt to build NEWS and HACKING,
even if they don't have the prerequisite tools.

> +
>  NEWS: $(top_srcdir)/docs/news.xsl $(top_srcdir)/docs/news.html.in
>  	-@(if [ -x $(XSLTPROC) ] ; then				\

This says that rebuilding NEWS is not fatal if you lack the prereq
tools, but that means the timestamp is never updated, which means every
run of 'make' will be that much slower.

It might be better to have _only_ 'make syntax-check' depend on these
two generated files, or even to leave things like they are for NEWS,
where you have to do a manual 'make NEWS' when it is really time to do
the update.

>  	  $(XSLTPROC) --nonet $(top_srcdir)/docs/news.xsl	\
> @@ -56,6 +58,12 @@ NEWS: $(top_srcdir)/docs/news.xsl $(top_srcdir)/docs/news.html.in
>  	   | perl -pe 's/[ \t]+$$//'				\
>  	   > $@-t && mv $@-t $@ ; fi );
>  
> +HACKING: $(top_srcdir)/docs/hacking1.xsl $(top_srcdir)/docs/hacking2.xsl \

Hmm.  NEWS is distributed (by virtue of automake), HACKING is not
(automake doesn't auto-distribute it, and we didn't list it in Makefile.am).

NEWS is not stored in git, HACKING currently is.  Should HACKING be
removed from git?

With NEWS, the most likely audience is the end user from a tarball, at
which point NEWS already exists.  And that means we have a pre-existing
bug - since NEWS is distributed, it should be built in $(srcdir) not
$(builddir).  As long as you do an in-tree build, they are one and the
same, but a developer doing a VPATH build gets NEWS generated in the
wrong directory given the rule above.

With HACKING, the most likely audience is a developer, working from git
(in fact, the reason we don't distribute it is that we assume someone
working from a tarball rather than from git probably won't care about
the contents of HACKING).

On the other hand, a developer that does a fresh clone of libvirt.git
will want instructions on how to bootstrap, and if those instructions
are hidden in docs/hacking.html.in until after the first 'autogen.sh &&
make', then they aren't too helpful.  That argues for keeping HACKING
still in git.

If HACKING stays in git, then it must also reside in $(srcdir) rather
than $(builddir), which means this rule needs a bit of tweaking to be
VPATH-friendly.

> +         $(top_srcdir)/docs/wrapstring.xsl $(top_srcdir)/docs/hacking.html.in
> +	-@(if [ -x $(XSLTPROC) ] ; then \
> +	   $(XSLTPROC) --nonet $(top_srcdir)/docs/hacking1.xsl $(top_srcdir)/docs/hacking.html.in | \
> +	   $(XSLTPROC) --nonet $(top_srcdir)/docs/hacking2.xsl - \
> +	   > $@-t && mv $@-t $@ ; fi );
>  
>  rpm: clean
>  	@(unset CDPATH ; $(MAKE) dist && rpmbuild -ta $(distdir).tar.gz)
> diff --git a/docs/hacking.html.in b/docs/hacking.html.in
> index bd8b443..88db01a 100644
> --- a/docs/hacking.html.in
>        <li><p>Post patches in unified diff format.  A command similar to this
>          should work:</p>
> -        <pre>
> -  diff -urp libvirt.orig/ libvirt.modified/ > libvirt-myfeature.patch</pre>
> +<pre>
> +  diff -urp libvirt.orig/ libvirt.modified/ > libvirt-myfeature.patch
> +</pre>

Changes like this make sense, since whitespace is significant within a
<pre> block.

> --- /dev/null
> +++ b/docs/hacking1.xsl
> @@ -0,0 +1,28 @@
> +<?xml version="1.0"?>
> +<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform">
> +
> +<xsl:output method="xml" encoding="UTF-8" indent="no"/>

Here, I'll have to take your word that this works; of course, I plan to
test it by applying the patch and running 'make HACKING', but I haven't
got that far yet (wanted to get this email sent first).

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20101111/060d3984/attachment-0001.sig>