[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] Remove all generated RPC files from GIT



On 05/06/2011 07:11 AM, Daniel P. Berrange wrote:
> commit 7a2fd256cb85dd9a9d6640d9faed0b66ca617411
> Author: Daniel P. Berrange <berrange redhat com>
> Date:   Fri May 6 13:36:21 2011 +0100
> 
>     Remove all generated files for remote protocol
>     
>     Stop storing the generated files for the remote protocol client
>     and server in source control. The generated files will still be
>     included in the result of 'make dist' to avoid end-users needing
>     to generate the files
>     
>     * daemon/Makefile.am: Removed generated files with
>       maintainer-clean target
>     * src/Makefile.am: Removed generated files with
>       maintainer-clean target. Always run 'rpcgen' if
>       generated files are missing
> 
> 
> NB: hand edited diff to remove the huge deleted files

Thanks for doing that.  However, it means that 'git am' didn't like this
patch, so I hope I tested it correctly.  At any rate, after applying
your patch, removing the generated files, then running 'make dist', and
comparing that to a 'make dist' pre-patch, the minor differences were
all expected (in the Makefiles, not in the generated files).

> -remote.h: \
> -	remote_dispatch_args.h \
> -	remote_dispatch_ret.h \
> -	qemu_dispatch_args.h \
> -	qemu_dispatch_ret.h
> +remote.c: $(DAEMON_GENERATED)
> +remote.h: $(DAEMON_GENERATED)

More than strictly necessary, but I suppose it doesn't hurt, since all
the generated files will have similar timestamps anyways.  Certainly
more compact :)

>  
>  REMOTE_PROTOCOL = $(top_srcdir)/src/remote/remote_protocol.x
>  QEMU_PROTOCOL = $(top_srcdir)/src/remote/qemu_protocol.x
> @@ -350,4 +344,4 @@ endif
>  
>  CLEANFILES += $(BUILT_SOURCES) $(man8_MANS)
>  CLEANFILES += *.cov *.gcov .libs/*.gcda .libs/*.gcno *.gcno *.gcda
> -MAINTAINERCLEANFILES = $(srcdir)/libvirtd.8.in
> +MAINTAINERCLEANFILES = $(srcdir)/libvirtd.8.in $(DAEMON_GENERATED)

Yep, that's the right approach for something that gets generated into
$(srcdir) for use in the distribution, but which does not need to be
rebuilt by normal users.

> @@ -528,68 +535,17 @@ libvirt_driver_remote_la_LDFLAGS += -module -avoid-version
>  endif
>  libvirt_driver_remote_la_SOURCES = $(REMOTE_DRIVER_SOURCES)
>  
> -if HAVE_RPCGEN

> -rpcgen: rpcgen-normal rpcgen-qemu

> +remote/remote_driver.c: $(REMOTE_DRIVER_GENERATED)
>  
> -endif
> +remote/%_protocol.c: remote/%_protocol.x remote/%_protocol.h
> +	$(AM_V_GEN)perl -w $(srcdir)/remote/rpcgen_fix.pl $(RPCGEN) -c \
> +               $< $@
> +
> +remote/%_protocol.h: remote/%_protocol.x
> +	$(AM_V_GEN)perl -w $(srcdir)/remote/rpcgen_fix.pl $(RPCGEN) -h \
> +               $< $@

Let's make bootstrap.conf mention that rpcgen is now a bootstrap
prerequisite.

> +++ b/src/remote/rpcgen_fix.pl
> @@ -1,4 +1,6 @@
> -# Fix XDR code (generated by rpcgen) so that it compiles
> +# 
> +# Generate code for an XDR protocol, optionally applying
> +# fixups to the glibc rpcgen code so that it compiles
>  # with warnings turned on.
>  #
>  # This code is evil.  Arguably better would be just to compile
> @@ -17,10 +19,35 @@ use strict;
>  my $in_function = 0;
>  my @function = ();
>  
> -while (<>) {
> +my $rpcgen = shift;
> +my $mode = shift;
> +my $xdrdef = shift;
> +my $target = shift;
> +
> +unlink $target;
> +
> +open RPCGEN, "-|", $rpcgen, $mode, $xdrdef
> +    or die "cannot run $rpcgen $mode $xdrdef: $!";
> +open TARGET, ">$target"
> +    or die "cannot create $target: $!";
> +
> +my $fixup = $^O eq "linux";

I guess I'll see what happens when I try this on Cygwin, but if there
are any followups needed, it should be separate patches.  As is, cygwin
already can't use -Werror because of repetitive  declarations in the
tirpc headers that trigger one of our default set of gcc warnings.

> +
> +chmod 0444, $target
> +    or die "cannot set $target readonly: $!";

Nice touch, to help us remember that it is generated.

ACK with this squashed in:

diff --git i/bootstrap.conf w/bootstrap.conf
index 09e8218..2d27846 100644
--- i/bootstrap.conf
+++ w/bootstrap.conf
@@ -169,6 +169,7 @@ gzip       -
 libtool    -
 perl       5.5
 pkg-config -
+rpcgen	   -
 tar        -
 "

diff --git i/src/remote/rpcgen_fix.pl w/src/remote/rpcgen_fix.pl
index d11bbd4..4edba98 100644
--- i/src/remote/rpcgen_fix.pl
+++ w/src/remote/rpcgen_fix.pl
@@ -1,4 +1,4 @@
-#
+#
 # Generate code for an XDR protocol, optionally applying
 # fixups to the glibc rpcgen code so that it compiles
 # with warnings turned on.
@@ -8,7 +8,7 @@
 # actually fixes for 64 bit, so this file is necessary.  Arguably
 # so is the type-punning fix.
 #
-# Copyright (C) 2007 Red Hat, Inc.
+# Copyright (C) 2007, 2011 Red Hat, Inc.
 #
 # See COPYING for the license of this software.
 #
diff --git i/.gitignore w/.gitignore
index 803f2a3..86afa8d 100644
--- i/.gitignore
+++ w/.gitignore
@@ -34,6 +34,7 @@
 /config.sub
 /configure
 /configure.lineno
+/daemon/*_dispatch_*.h
 /gnulib/
 /libtool
 /libvirt-*.tar.gz
@@ -49,6 +50,7 @@
 /po/*
 /proxy/
 /src/libvirt_iohelper
+/src/remote/*_protocol.[ch]
 /tests/*.log
 /tests/cputest
 /tests/hashtest


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

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]