This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Cross-rpcgen patch, version 6


On Wed, 9 May 2012, Roland McGrath wrote:

> I don't like the idea of touching Makerules for something that is
> neither really general-purpose nor used in more than one place.
> This change can be entirely confined to sunrpc/, so please do that.

OK, here's a version that is confined to the sunrpc directory again
(much like the earliest versions of this patch, but with the build
rule as similar as possible to that used outside that directory).

> As I said before, I think we should have first-class machinery for
> compiling nontrivial build-host programs.  Please file a bug for
> that, and give sunrpc/Makefile comments that refer to that bug's URL
> (or just BZ# if you prefer, though IMHO a URL is more handy).

Bug 14087 filed.  I've added a comment referring to it.

> >  # Tell rpcgen where to find the C preprocessor.
> > -rpcgen-cmd = CPP='$(CC) -E -x c-header' $(built-program-cmd) -Y ../scripts
> > +rpcgen-cmd = CPP='$(CC) -E -x c-header' $(built-program-file) -Y ../scripts
> 
> As Carlos mentioned, the meaning of built-program-file is not obvious.
> It also reads as odd that the sole comment for the line is about the
> CPP=... portion.  (Not that either of these is really the fault of your
> change.)  Please amend the comment to something like:
> 
> # How we run rpcgen to generate sources and headers in the rules below.
> # Setting CPP tells it how to run the C preprocessor correctly.  Note
> # that $(built-program-file) requires that the just-built cross-rpcgen
> # binary be the second dependency listed in each rule using rpcgen-cmd.

Suggested comment wording added.

> > +#ifdef IS_IN_build
> > +
> > +#define _(X) (X)
> > +#define _libc_intl_domainname "libc"
> > +
> > +#endif
> 
> Please give these macros some comments explaining what they're doing.
> Since the _ definition means that gettext will never be used, I think it
> would be more clear and clean (and incidentally trivially more
> efficient) to define away textdomain rather than _libc_intl_domainname.
> If it's really significantly easier to leave the textdomain call intact
> for some reason, then I think the string should be something like
> "***UNUSED***" that makes it more obvious that this is just a cruft
> workaround rather than something meaningful for correct operation.

More explanation added.  textdomain is now defined away, but since
_libc_intl_domainname is also used for --version output I kept the
definition of that as well (with a comment).

Tested x86_64.

2012-05-09  Maxim Kuvyrkov  <maxim@codesourcery.com>
	    Joseph Myers  <joseph@codesourcery.com>
	    Paul Pluzhnikov  <ppluzhnikov@google.com>

	[BZ #14012]
	* sunrpc/Makefile [cross-compiling] (headers): Enable additions
	requiring rpcgen.
	[cross-compiling] (extra-libs): Likewise.
	[cross-compiling] (extra-libs-others): Likewise.
	[cross-compiling] (librpcsvc-routines): Likewise.
	[cross-compiling] (librpcsvc-inhibit-o): Likewise.
	[cross-compiling] (omit-deps): Likewise.
	(sunrpc-CPPFLAGS): New variable.
	(CPPFLAGS): Define using $(sunrpc-CPPFLAGS).
	(BUILD_CPPFLAGS): Append $(sunrpc-CPPFLAGS).
	(cross-rpcgen-objs): New variable.
	(extra-objs): Append $(cross-rpcgen-objs).
	($(cross-rpcgen-objs)): New rule.
	($(objpfx)cross-rpcgen): Likewise.
	(rpcgen-cmd): Define to use $(built-program-file).  Expand
	comment.
	($(objpfx)rpcsvc/%.stmp): Depend on cross-rpcgen.
	($(objpfx)x%.stmp): Likewise.
	* sunrpc/proto.h [IS_IN_build] (_): Define.
	[IS_IN_build] (_libc_intl_domainname): Likewise.

diff --git a/sunrpc/Makefile b/sunrpc/Makefile
index 48790f4..1c1049c 100644
--- a/sunrpc/Makefile
+++ b/sunrpc/Makefile
@@ -97,15 +97,12 @@ ifeq ($(have-thread-library),yes)
 xtests += thrsvc
 endif
 
-ifeq (no,$(cross-compiling))
-# We can only build this library if we can run the rpcgen we build.
 headers += $(rpcsvc:%.x=rpcsvc/%.h)
 extra-libs := librpcsvc
 extra-libs-others := librpcsvc # Make it in `others' pass, not `lib' pass.
 librpcsvc-routines = $(rpcsvc:%.x=x%)
 librpcsvc-inhibit-o = .os # Build no shared rpcsvc library.
 omit-deps = $(librpcsvc-routines)
-endif
 
 include ../Rules
 
@@ -139,7 +136,9 @@ CFLAGS-pmap_rmt.c = -fexceptions
 CFLAGS-clnt_perr.c = -fexceptions
 CFLAGS-openchild.c = -fexceptions
 
-CPPFLAGS += -D_RPC_THREAD_SAFE_
+sunrpc-CPPFLAGS = -D_RPC_THREAD_SAFE_
+CPPFLAGS += $(sunrpc-CPPFLAGS)
+BUILD_CPPFLAGS += $(sunrpc-CPPFLAGS)
 
 $(objpfx)tst-getmyaddr: $(common-objpfx)linkobj/libc.so
 $(objpfx)tst-xdrmem: $(common-objpfx)linkobj/libc.so
@@ -147,13 +146,30 @@ $(objpfx)tst-xdrmem2: $(common-objpfx)linkobj/libc.so
 
 $(objpfx)rpcgen: $(addprefix $(objpfx),$(rpcgen-objs))
 
+cross-rpcgen-objs := $(addprefix $(objpfx)cross-,$(rpcgen-objs))
+extra-objs += $(cross-rpcgen-objs)
+
+# When generic makefile support for build system programs is
+# available, it should replace this code.  See
+# <http://sourceware.org/bugzilla/show_bug.cgi?id=14087>.
+$(cross-rpcgen-objs): $(objpfx)cross-%.o: %.c
+	$(make-target-directory)
+	$(BUILD_CC) $($(basename $(<F))-CFLAGS) $(ALL_BUILD_CFLAGS) $< \
+		$(OUTPUT_OPTION) $(compile-mkdep-flags) -c
+
+$(objpfx)cross-rpcgen: $(cross-rpcgen-objs)
+	$(BUILD_CC) $^ $(BUILD_LDFLAGS) -o $@
+
 # This makes sure -DNOT_IN_libc is passed for all these modules.
 cpp-srcs-left := $(rpcgen-objs:.o=.c)
 lib := nonlib
 include $(patsubst %,$(..)cppflags-iterator.mk,$(cpp-srcs-left))
 
-# Tell rpcgen where to find the C preprocessor.
-rpcgen-cmd = CPP='$(CC) -E -x c-header' $(built-program-cmd) -Y ../scripts
+# How we run rpcgen to generate sources and headers in the rules below.
+# Setting CPP tells it how to run the C preprocessor correctly.  Note
+# that $(built-program-file) requires that the just-built cross-rpcgen
+# binary be the second dependency listed in each rule using rpcgen-cmd.
+rpcgen-cmd = CPP='$(CC) -E -x c-header' $(built-program-file) -Y ../scripts
 
 # Install the rpc data base file.
 $(inst_sysconfdir)/rpc: etc.rpc $(+force)
@@ -164,7 +180,7 @@ $(inst_sysconfdir)/rpc: etc.rpc $(+force)
 # relinked.
 $(rpcsvc:%.x=$(objpfx)rpcsvc/%.h): $(objpfx)rpcsvc/%.h: $(objpfx)rpcsvc/%.stmp
 	@:
-$(objpfx)rpcsvc/%.stmp: rpcsvc/%.x $(objpfx)rpcgen
+$(objpfx)rpcsvc/%.stmp: rpcsvc/%.x $(objpfx)cross-rpcgen
 	$(make-target-directory)
 	-@rm -f ${@:stmp=T} $@
 	$(rpcgen-cmd) -h $< -o ${@:stmp=T}
@@ -174,7 +190,7 @@ $(objpfx)rpcsvc/%.stmp: rpcsvc/%.x $(objpfx)rpcgen
 # Generate the rpcsvc XDR functions with rpcgen.
 $(rpcsvc:%.x=$(objpfx)x%.c): $(objpfx)x%.c: $(objpfx)x%.stmp
 	@:
-$(objpfx)x%.stmp: rpcsvc/%.x $(objpfx)rpcgen
+$(objpfx)x%.stmp: rpcsvc/%.x $(objpfx)cross-rpcgen
 	-@rm -f ${@:stmp=T} $@
 	$(rpcgen-cmd) -c $< -o ${@:stmp=T}
 	$(move-if-change) $(@:stmp=T) $(@:stmp=c)
diff --git a/sunrpc/proto.h b/sunrpc/proto.h
index 3e1ecd1..0ba9cd6 100644
--- a/sunrpc/proto.h
+++ b/sunrpc/proto.h
@@ -50,3 +50,19 @@ void crash(void) __attribute__ ((noreturn));
 void tabify(FILE *f, int tab);
 char *make_argname(const char *pname, const char *vname);
 void add_type(int len, const char *type);
+
+/* This header is the last one included in all rpc_*.c files,
+   so we define stuff for cross-rpcgen here to avoid conflicts with
+   $build's C library and $host's glibc.  */
+
+#ifdef IS_IN_build
+
+/* Disable translated messages when built for $build and used in
+   building glibc.  */
+#define _(X) (X)
+#define textdomain(X) ((void) 0)
+
+/* This is used in the definition of PACKAGE for --version output.  */
+#define _libc_intl_domainname "libc"
+
+#endif

-- 
Joseph S. Myers
joseph@codesourcery.com


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]