Re: Cross-rpcgen patch, version 6

[Roland McGrath <roland at hack dot frob dot com>]

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.

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).

>  # 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.

> +#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.


Thanks,
Roland