[Libvir] using gnulib: starting with the physmem and getaddrinfo modules

[Daniel Veillard]

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 at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/