This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [trunk][patch] Install headers and plugin-version.o
- From: Paolo Bonzini <bonzini at gnu dot org>
- To: GCC Patches <gcc-patches at gcc dot gnu dot org>, Rafael Espindola <espindola at google dot com>
- Date: Tue, 05 May 2009 16:06:00 +0200
- Subject: Re: [trunk][patch] Install headers and plugin-version.o
- References: <38a0d8450904210614v407ef154v17927b6c9bcb0c92@mail.gmail.com> <Pine.LNX.4.64.0904211547271.16159@digraph.polyomino.org.uk> <38a0d8450904210949g3c5d4e9do3042f3047053fcea@mail.gmail.com> <Pine.LNX.4.64.0904211824350.20450@digraph.polyomino.org.uk> <38a0d8450904220509o3d650df1w601198913a3bcafd@mail.gmail.com> <38a0d8450905050624s1de6f10bxe3719c9ffabc1d74@mail.gmail.com> <38a0d8450905050638x7e301d16t5f8113cae7a1172c@mail.gmail.com>
Rafael Espindola wrote:
> Just got a message from Joseph saying he is away from his email. Could
> you please review this patch.
Without a ChangeLog I cannot understand the config.gcc parts.
This:
> + if [ "$$path" != "" ]; then \
can use a leftover $$path from the previous iteration; reset it in the
preceding "else", or use "continue".
This:
> + config=gcc/config/; \
is wrong when building with srcdir=.
and in general, this:
> + echo $$path | grep "$$config" > /dev/null; \
> + isconfig=$$?; \
> + echo $$path | grep "\.def$$" > /dev/null; \
> + isdef=$$?; \
> + echo $$path | grep "^$(srcdir)" > /dev/null; \
> + is_source=$$?; \
> + if [ $$isdef = 0 ] || [ $$isconfig = 0 ]; then \
> + if [ $$is_source = 0 ]; then \
may fail depending on set -e, it seems easier to use case and $$file
like this:
case $$path in \
"$(srcdir)"*/config/* | "$(srcdir)*.def ) base=$$file;; \
*) base=`basename $$path` ;; \
esac; \
dest=$(plugin_includedir)/$$base; \
dir=`dirname $$dest`; \
$(mkinstalldirs) $$dir; \
$(INSTALL_DATA) $$path $(DESTDIR)/$$dest; \
or something like that; also notice that I used mkinstalldirs instead of
> mkdir -p $$dir
which is exactly what mkinstalldirs replaces, though portably.
Finally, regarding
> +ifeq ($(enable_plugin),yes)
> +INSTALL_PLUGIN=install-plugin
> +else
> +INSTALL_PLUGIN=
> +endif
you can just move it down and say
ifeq ($(enable_plugin),yes)
install: install-plugin
endif
Paolo