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

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



On 03/07/2012 04:56 AM, Eric Blake wrote:
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.

     Get it, thanks.


So, given that background, what do you think of this alternative patch?

 From 0ec1071294c3b02ceeee6c77df4c61cb6f039bba Mon Sep 17 00:00:00 2001
From: Eric Blake<eblake redhat com>
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;

    It works great, so ACK here.


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