Re: [libvirt] [PATCH] rpc: Fix VPATH building issue

On 03/06/2012 10:07 AM, Guannan Ren wrote: > The XDR routine .c file generated by rpcgen includes the corresponding > Header file. however, the path in include directive of the header file > is picked up based on the path of .x file. In the situation of VPATH Builds > it will include full path of the header file rather than relative path. Hence, > the error happens when compiling time > > For example: The libvirt source code resides in /home/testuser, > I make dist in /tmp/buildvpath, the XDR routine .c file will > include full path of the header file like: > > #include "/home/testuser/src/rpc/virnetprotocol.h" > #include "internal.h" > #include<arpa/inet.h> > > If we distribute the tarball to another machine to compile, > it will report error as follows: > > rpc/virnetprotocol.c:7:59: fatal error: > /home/testuser/src/rpc/virnetprotocol.h: No such file or directory Previously, we've fixed this in genprotocol.pl, via lines like this: s,#include ".*remote/remote_protocol\.h",#include "remote_protocol.h",; I'm wondering whether it is better to fix it there for all protocol files. > --- > src/Makefile.am | 14 ++++++++++---- > 1 files changed, 10 insertions(+), 4 deletions(-) > > diff --git a/src/Makefile.am b/src/Makefile.am > index e57eca2..6c9f598 100644 > --- a/src/Makefile.am > +++ b/src/Makefile.am > @@ -662,12 +662,18 @@ $(srcdir)/remote/remote_driver.c: $(REMOTE_DRIVER_GENERATED) > endif WITH_REMOTE > > %protocol.c: %protocol.x %protocol.h $(srcdir)/rpc/genprotocol.pl > - $(AM_V_GEN)perl -w $(srcdir)/rpc/genprotocol.pl $(RPCGEN) -c \ > - $< $@ > + $(AM_V_GEN)protocolx='$<'; \ > + protocolc='$@'; \ > + cd $(srcdir); \ > + perl -w $(srcdir)/rpc/genprotocol.pl $(RPCGEN) -c \ > + $${protocolx/'$(srcdir)/'/''} $${protocolc/'$(srcdir)/'/''} Alas, ${foo/pat/sub} is not POSIX, so we cannot use it (if /bin/sh is dash, as is the case on Debian, then this is broken). But no fear, we _do_ require GNU make, so we can use $(patsubst pat,subst,foo) instead of $${foo/pat/sub}, if we still like the approach of changing Makefile.am rather than fixing genprotocol.pl.

So, given that background, what do you think of this alternative patch? From 0ec1071294c3b02ceeee6c77df4c61cb6f039bba Mon Sep 17 00:00:00 2001 From: Eric Blake&lt;eblake(a)redhat.com&gt; Date: Tue, 6 Mar 2012 13:49:53 -0700 Subject: [PATCH] rpc: generalize solution for VPATH builds Commit 5d4b0c4c80 tried to fix certain classes of VPATH builds, but was too limited. In particular, Guannan Ren reported: > For example: The libvirt source code resides in /home/testuser, > I make dist in /tmp/buildvpath, the XDR routine .c file will > include full path of the header file like: > > #include "/home/testuser/src/rpc/virnetprotocol.h" > #include "internal.h" > #include<arpa/inet.h> > > If we distribute the tarball to another machine to compile, > it will report error as follows: > > rpc/virnetprotocol.c:7:59: fatal error: > /home/testuser/src/rpc/virnetprotocol.h: No such file or directory * src/rpc/genprotocol.pl: Fix more include lines. --- src/rpc/genprotocol.pl | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/rpc/genprotocol.pl b/src/rpc/genprotocol.pl index 4838325..f8e68f5 100755 --- a/src/rpc/genprotocol.pl +++ b/src/rpc/genprotocol.pl @@ -8,7 +8,7 @@ # actually fixes for 64 bit, so this file is necessary. Arguably # so is the type-punning fix. # -# Copyright (C) 2007, 2011 Red Hat, Inc. +# Copyright (C) 2007, 2011-2012 Red Hat, Inc. # # See COPYING for the license of this software. # @@ -53,13 +53,15 @@ while (<RPCGEN>) { s/\t/ /g; + # Fix VPATH builds + s,#include ".*/([^/]+)protocol\.h",#include "${1}protocol.h",; + # 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_INT32 )IXDR_GET_LONG/IXDR_GET_INT32/g; - s,#include ".*remote/remote_protocol\.h",#include "remote_protocol.h",; if (m/^}/) { $in_function = 0;