[libvirt] [PATCH] rpcgen fixes

# HG changeset patch # User john.levon@sun.com # Date 1229399266 28800 # Node ID 12c9b283b11f5e20b9dcc91e3d10a862da5d6e81 # Parent 1ba824db09286ebc758e035211da2c533bbd1e0f rpcgen fixes quad_t is not a portable type, but rather than force rpcgen every build, we just patch in the fixes needed. Also, since we ship the rpcgen-derived files in CVS, don't leave Makefile rules that could trigger during a normal build: make a special maintainer target for refreshing the derived sources. Signed-off-by: John Levon <john.levon@sun.com> diff --git a/qemud/Makefile.am b/qemud/Makefile.am --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -32,23 +32,23 @@ EXTRA_DIST = \ $(DAEMON_SOURCES) if RPCGEN -SUFFIXES = .x -.x.c: - rm -f $@ $@-t $@-t2 - rpcgen -c -o $@-t $< +# +# Maintainer-only target for re-generating the derived .c/.h source +# files, which are actually derived from the .x file. +# +rpcgen: + rm -f rp.c-t rp.h-t + rpcgen -h -o rp.h-t @top_srcdir@/qemud/remote_protocol.x + rpcgen -c -o rp.c-t @top_srcdir@/qemud/remote_protocol.x if GLIBC_RPCGEN - perl -w rpcgen_fix.pl $@-t > $@-t2 - chmod 444 $@-t2 - mv $@-t2 $@ -endif - -.x.h: - rm -f $@ $@-t - rpcgen -h -o $@-t $< -if GLIBC_RPCGEN - perl -pi -e 's/\t/ /g' $@-t - chmod 444 $@-t - mv $@-t $@ + perl -w @top_srcdir@/qemud/rpcgen_fix.pl rp.h-t \ + >@top_srcdir@/qemud/remote_protocol.h + perl -w @top_srcdir@/qemud/rpcgen_fix.pl rp.c-t \ + >@top_srcdir@/qemud/remote_protocol.c + rm -f rp.c-t rp.h-t +else + mv rp.h-t @top_srcdir@/remote_protocol.h + mv rp.c-t @top_srcdir@/remote_protocol.c endif endif diff --git a/qemud/rpcgen_fix.pl b/qemud/rpcgen_fix.pl --- a/qemud/rpcgen_fix.pl +++ b/qemud/rpcgen_fix.pl @@ -25,6 +25,15 @@ while (<>) { } s/\t/ /g; + + # Portability for Solaris RPC + s/u_quad_t/uint64_t/g; + s/quad_t/int64_t/g; + s/xdr_u_quad_t/xdr_uint64_t/g; + s/xdr_quad_t/xdr_int64_t/g; + s/IXDR_GET_LONG/IXDR_GET_INT32/g; + s/XDR_INLINE/(int32_t *)XDR_INLINE/g; + s/#include <rpc\/rpc.h>/#include <config.h>\n#include <rpc\/rpc.h>/g; if (m/^}/) { $in_function = 0;

john.levon@sun.com wrote:
# HG changeset patch # User john.levon@sun.com # Date 1229399266 28800 # Node ID 12c9b283b11f5e20b9dcc91e3d10a862da5d6e81 # Parent 1ba824db09286ebc758e035211da2c533bbd1e0f rpcgen fixes
quad_t is not a portable type, but rather than force rpcgen every build, we just patch in the fixes needed.
Also, since we ship the rpcgen-derived files in CVS, don't leave Makefile rules that could trigger during a normal build: make a special maintainer target for refreshing the derived sources.
This sounds like an argument for not putting derived sources in CVS, since derived-for-one-system (linux/gnu) doesn't work for e.g., Solaris.
Signed-off-by: John Levon <john.levon@sun.com>
diff --git a/qemud/Makefile.am b/qemud/Makefile.am --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -32,23 +32,23 @@ EXTRA_DIST = \ $(DAEMON_SOURCES)
if RPCGEN -SUFFIXES = .x -.x.c: - rm -f $@ $@-t $@-t2 - rpcgen -c -o $@-t $< +# +# Maintainer-only target for re-generating the derived .c/.h source +# files, which are actually derived from the .x file. +# +rpcgen: + rm -f rp.c-t rp.h-t + rpcgen -h -o rp.h-t @top_srcdir@/qemud/remote_protocol.x + rpcgen -c -o rp.c-t @top_srcdir@/qemud/remote_protocol.x
Use $(srcdir), in place of @top_srcdir@/qemud (and others below). The latter @var@ syntax is old, if not deprecated, and $(srcdir) is guaranteed to be safe, while $(top_srcdir) may be quite nasty (arbitrary abs. dir name) in the worst case.
if GLIBC_RPCGEN - perl -w rpcgen_fix.pl $@-t > $@-t2 - chmod 444 $@-t2 - mv $@-t2 $@ -endif - -.x.h: - rm -f $@ $@-t - rpcgen -h -o $@-t $< -if GLIBC_RPCGEN - perl -pi -e 's/\t/ /g' $@-t - chmod 444 $@-t - mv $@-t $@ + perl -w @top_srcdir@/qemud/rpcgen_fix.pl rp.h-t \ + >@top_srcdir@/qemud/remote_protocol.h
Please don't redirect directly to something used as a build source. Instead, redirect to a temporary file, and rename that into place upon successful completion. That makes it less likely you'll ever end up with corrupted sources.
+ perl -w @top_srcdir@/qemud/rpcgen_fix.pl rp.c-t \ + >@top_srcdir@/qemud/remote_protocol.c + rm -f rp.c-t rp.h-t +else + mv rp.h-t @top_srcdir@/remote_protocol.h + mv rp.c-t @top_srcdir@/remote_protocol.c endif endif ...

On Wed, Dec 17, 2008 at 02:24:29PM +0100, Jim Meyering wrote:
This sounds like an argument for not putting derived sources in CVS, since derived-for-one-system (linux/gnu) doesn't work for e.g., Solaris.
If you're OK with requiring rpcgen, that's most likely fine by me (though you'd want to tighten up that glibc rpcgen check!)
+ rpcgen -h -o rp.h-t @top_srcdir@/qemud/remote_protocol.x + rpcgen -c -o rp.c-t @top_srcdir@/qemud/remote_protocol.x
Use $(srcdir), in place of @top_srcdir@/qemud (and others below). The latter @var@ syntax is old, if not deprecated,
Coo, been a while - will do.
+ perl -w @top_srcdir@/qemud/rpcgen_fix.pl rp.h-t \ + >@top_srcdir@/qemud/remote_protocol.h
Please don't redirect directly to something used as a build source. Instead, redirect to a temporary file, and rename that into place upon successful completion. That makes it less likely you'll ever end up with corrupted sources.
Sorry I'm missing why this helps any? regards john

John Levon <john.levon@Sun.COM> wrote: ...
+ perl -w @top_srcdir@/qemud/rpcgen_fix.pl rp.h-t \ + >@top_srcdir@/qemud/remote_protocol.h
Please don't redirect directly to something used as a build source. Instead, redirect to a temporary file, and rename that into place upon successful completion. That makes it less likely you'll ever end up with corrupted sources.
Sorry I'm missing why this helps any?
In rules where you're redirecting to $@, it's essential so that you don't end up with a corrupt-yet-up-to-date target. Here, it'd be nice. Otherwise, if the command fails with a disk full or I/O error, you're left with an incomplete file, and you might have missed the failure and/or you might then forget to rerun it, which leaves you to figure out why things don't work. Not a terribly big deal in this case; since it's version-controlled, you'll probably notice the diffs pretty quickly. Oh, also, since you've added the new "rpcgen" target, please declare it phony to GNU make with this: .PHONY: rpcgen otherwise, if you run make -t (or otherwise create a file name rpcgen), that rule will do nothing.

On Tue, Dec 16, 2008 at 06:25:26PM -0800, john.levon@sun.com wrote:
# HG changeset patch # User john.levon@sun.com # Date 1229399266 28800 # Node ID 12c9b283b11f5e20b9dcc91e3d10a862da5d6e81 # Parent 1ba824db09286ebc758e035211da2c533bbd1e0f rpcgen fixes
quad_t is not a portable type, but rather than force rpcgen every build, we just patch in the fixes needed.
IIRC, this will also help on Mac OS-X which lacked this.
Also, since we ship the rpcgen-derived files in CVS, don't leave Makefile rules that could trigger during a normal build: make a special maintainer target for refreshing the derived sources.
ACK, good change. The automatic refresh was broken anyway, because the generated files are needed from ../src before the build even gets to qemud/. These generated files / wire protocol define ABI and should never change for ordinary builds, so its safer to keep generation of them an explicit maintainer task. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (4)
-
Daniel P. Berrange
-
Jim Meyering
-
John Levon
-
john.levon@sun.com