4:28 a.m.
On Wed, Dec 05, 2007 at 12:33:24AM +0100, Jim Meyering wrote:
Recently, I heard of two tricky portability problems in libvirt that
are easy to solve with gnulib. Of course, gnulib provides a lot more,
and is not exactly lightweight if you count "lines of code imported", but
once the framework (this patch) is installed, adding an additional module
is as easy as adding the module name to a list. Keep in mind that what
matters with a portability library is that it stay out of your way -- and
of course do its job well. To that end, gnulib has many per-module unit
[...]
The first portability problem was to determine the total physical
memory
available on the current system. Currently the code works only on
Linux-like systems that have /proc/meminfo of an expected form. However,
the gnulib physmem module handles 13 distinct types of systems and is
well tested:
http://www.gnu.org/software/gnulib/MODULES.html#module=physmem
Right a portability solution for this is badly needed
The second portability problem was to find a robust and
LGPL-compatible
getaddrinfo function to be used on systems lacking it. Here's the
gnulib module:
http://www.gnu.org/software/gnulib/MODULES.html#module=getaddrinfo
yes that's needed too for Windows now.
Once the few gnulib hooks are in your configure.ac and Makefile.am
files, there is very little extra work required to use the new
functions. In the case of physmem, just use the function and
include "physmem.h". For getaddrinfo, merely include
"getaddrinfo.h"
from the two files that use the function.
This change brings in a lot of code, but many of the lib/.[ch] files
are used only on systems that lack some required functionality. For
example, the getaddrinfo.c file isn't even compiled when it's not
needed.
My main concerns were over:
- an 'invasion' of a lot of different code in libvirt own code base
and the associated maintainance duties
- Licencing problems, we are LGPLv2 and should be very careful to
not import in the library code which would break it even inadvertently
In the patch below, I've included a new script called bootstrap.
It is a wrapper around gnulib-tool that pulls into libvirt the
Like Dan I don't like this too much, we should have the files in CVS
and not require dynamic fetching. Moreover this just fails for me here
after applying the patch, the final patch should include everything
needed to
./autogen.sh ; make ; make check
both from a CVS checkout and from a tarball without assuming network
connection.
paphio:~/libvirt -> sh bootstrap
bootstrap: getting gnulib files...
bootstrap: line 50: git: command not found
bootstrap: line 51: cleanup_gnulib: command not found
bootstrap: line 59: gnulib/gnulib-tool: No such file or directory
paphio:~/libvirt ->
files selected by the (currently two) modules in use. Those new
files go in three places:
m4/*.m4
lib/*.[ch] and a few template .h.in files
Hum, if lib/ ends up containing only files coming from gnulib it's
probably better to name it after it, that way it will be clear where
this originates from
gl-tests/ for unit test C programs and Bourne shell scripts
should be in tests/gnulib/ or something similar, I concur with Dan
on this,
However, note that gettextize and libtoolize (run by autogen.sh)
also deposit many *.m4 files in m4. I compared and found that 8
of the files that are already pulled in by the *ize programs are
also pulled in (potentially newer versions) from gnulib. But currently,
using gettext-0.16.1 or gettext-0.17, there is no difference in any
of the overlapping files.
We need to find a way, Dan suggested a mechanism,
Re Licenses: the two modules (and all of their dependent modules)
are LGPL-compatible. This is enforced by running gnulib-tool
with the --lgpl option. If you were to request a module with
an incompatible license (say GPL or LGPLv3), it would fail.
Can we make qa static check at commit time ? We really need everything
in CVS and checking individually all added files before commit sounds the
right way to make sure there is no problem.
----------------------
Here's the patch that shows what existing parts of libvirt have to
be modified to use these two new modules. To try it out, just apply
the patch and then run this:
./autogen.sh && ./bootstrap && make && make check
Running bootstrap creates the new lib/ and gl-tests/ directories.
well it fails for me, but the patch applies fine.
Personally, I prefer not to add generated files to version control
systems, because it can lead to problems with version skew if all
developers don't use the same releases of the tools that do the
generating. Perhaps more importantly, when there are massive diffs in
the generated files, that can obscure real changes in non-generated parts
of the code. That already happens to me whenever the .po files change.
But if people prefer to add all of these imported files to CVS, just
say the word and I'll prepare the patch. If so, do you guys want the
gettextize- and libtoolize-added files to be version-controlled, now, too?
Basically everything needed for autogen/build/check/install whould be in CVS.
I will provide a finer grained review of the patch, hopefully it doesn't
really change existing code, it's more glue at the autogen/configure.in and
Makefiles level, trying to review the C files of gnulib may not be very
productive (except for the Licence check which really need to be done
individually).
Thanks a lot for looking at this, I was initially rather dubious because of
the size of the addition for the relatively few things being fixed a priori.
It looks safe though (once all files are indvidually checked for licencing)
and can probably help further as libvirt get ported to other things.
One thing I really wonder though is suppot of that code when using
Microsoft compilers on the various Windows platforms. Are those part of
the target for gnulib. That's in my experience with libxml2 some of the
place where the most tricky problems could be reported, so if this
could help there theis would be a big argument for gnulib inclusion,
thanks !
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/