Patchwork Avoid messages about non-existent gnatls command (PR bootstrap/51201)

login
register
mail settings
Submitter Jakub Jelinek
Date Nov. 30, 2011, 5:41 p.m.
Message ID <20111130174131.GJ27242@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/128542/
State New
Headers show

Comments

Jakub Jelinek - Nov. 30, 2011, 5:41 p.m.
Hi!

:= assigned variable gets evaluated right away, including the case when
host doesn't have any Ada compiler installed.  In that case we remove ada
from enabled languages, but still RTS_DIR is sometimes computed.

With this patch we could call gnatls 4 times (or even 6 times) instead of
just once, but as gnatls isn't very expensive command, I think it is worth
doing that.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2011-11-30  Jakub Jelinek  <jakub@redhat.com>

	PR bootstrap/51201
	* gcc-interface/Make-lang.in: Initialize RTS_DIR with = instead of :=.


	Jakub
Arnaud Charlet - Dec. 1, 2011, 8:08 a.m.
> := assigned variable gets evaluated right away, including the case when
> host doesn't have any Ada compiler installed.  In that case we remove ada
> from enabled languages, but still RTS_DIR is sometimes computed.

Can you elaborate here? When is RTS_DIR computed if Ada is not enabled?
That seems surprising to me, so I'd like to understand more before OKing
this patch.

Arno
Jakub Jelinek - Dec. 1, 2011, 8:38 a.m.
On Thu, Dec 01, 2011 at 09:08:27AM +0100, Arnaud Charlet wrote:
> > := assigned variable gets evaluated right away, including the case when
> > host doesn't have any Ada compiler installed.  In that case we remove ada
> > from enabled languages, but still RTS_DIR is sometimes computed.
> 
> Can you elaborate here? When is RTS_DIR computed if Ada is not enabled?
> That seems surprising to me, so I'd like to understand more before OKing
> this patch.

As written in the PR, even if you on say x86_64-linux
../configure --enable-languages=c --target=i686-pc-linux-gnu
you end up with:
CONFIG_LANGUAGES =  c++ lto
LANGUAGES = c gcov$(exeext) gcov-dump$(exeext) $(CONFIG_LANGUAGES)
...
LANG_MAKEFRAGS =  $(srcdir)/ada/gcc-interface/Make-lang.in $(srcdir)/cp/Make-lang.in $(srcdir)/fortran/Make-lang.in $(srcdir)/go/Make-lang.in $(srcdir)/java/Make-lang.in $(srcdir)/lto/Make-lang.in $(srcdir)/objc/Make-lang.in $(srcdir)/objcp/Make-lang.in
...
# per-language makefile fragments
ifneq ($(LANG_MAKEFRAGS),)
include $(LANG_MAKEFRAGS)
endif

in gcc/Makefile.  When Ada isn't installed on the host, you end
up with something like:
...
checking whether g++ accepts -g... yes
checking for alloca... yes
checking for x86_64-unknown-linux-gnu-gnatbind... no
checking for x86_64-unknown-linux-gnu-gnatmake... no
checking whether compiler driver understands Ada... no
checking how to run the C preprocessor... yes
gcc -E
checking for ANSI C header files... (cached) yes
...
config.status: creating ada/gcc-interface/Makefile
config.status: creating ada/Makefile
config.status: creating auto-host.h
config.status: executing default commands
/bin/sh: gnatls: command not found
make[2]: Entering directory `/usr/src/gcc/obj3/gcc'
/bin/sh ../../gcc/../mkinstalldirs po
/bin/sh ../../gcc/../mkinstalldirs po
/bin/sh ../../gcc/../mkinstalldirs po
...
in the output.  That is because ada/gcc-interface/Make-lang.in is sourced,
even when no goals from it are actually made.  But already mere sourcing
of that makefile results in
ifeq ($(build), $(host))
  ifeq ($(host), $(target))
...
  else
...
    RTS_DIR:=$(strip $(subst \,/,$(shell gnatls -v | grep adalib )))
...
  endif
else
...
endif

$(build) and $(host) is equal here, $(host) and $(target) aren't equal
and thus RTS_DIR variable is initialized from the command, irrespective
from host GNAT not being detected or present.
By changing that variable to deferred:
    RTS_DIR=$(strip $(subst \,/,$(shell gnatls -v | grep adalib )))
no command is run when the makefile fragment is sourced,
instead the command is executed when the vars are actually used:
    ADA_TOOLS_FLAGS_TO_PASS=\
        CC="$(CC)" \
        $(COMMON_FLAGS_TO_PASS) $(ADA_FLAGS_TO_PASS) \
        ADA_INCLUDES="-I$(RTS_DIR)../adainclude -I$(RTS_DIR)" \
        GNATMAKE="gnatmake" \
        GNATBIND="gnatbind" \
        GNATLINK="gnatlink" \
        LIBGNAT=""
...
gnattools: $(GCC_PARTS) $(CONFIG_H) prefix.o force
        $(MAKE) -C ada $(ADA_TOOLS_FLAGS_TO_PASS) gnattools1
        $(MAKE) -C ada $(ADA_TOOLS_FLAGS_TO_PASS) gnattools2

For gnattools target, this means that gnatls is with the patch
invoked 4 times, twice per gnattools1 subcommand and twice
per gnattools2 subcommand, but it is evaluated before spawning
the submake (i.e. we don't run gnatls per command line).

	Jakub
Arnaud Charlet - Dec. 1, 2011, 8:50 a.m.
> As written in the PR, even if you on say x86_64-linux
> ../configure --enable-languages=c --target=i686-pc-linux-gnu
> you end up with:
> CONFIG_LANGUAGES =  c++ lto
> LANGUAGES = c gcov$(exeext) gcov-dump$(exeext) $(CONFIG_LANGUAGES)
> ...
> LANG_MAKEFRAGS =  $(srcdir)/ada/gcc-interface/Make-lang.in
> $(srcdir)/cp/Make-lang.in $(srcdir)/fortran/Make-lang.in
> $(srcdir)/go/Make-lang.in $(srcdir)/java/Make-lang.in
> $(srcdir)/lto/Make-lang.in $(srcdir)/objc/Make-lang.in
> $(srcdir)/objcp/Make-lang.in

Do we know why LANG_MAKEFRAGS contains all the disabled languages?
If that a feature/requirement or a bug/limitation?

If the latter, I'd rather fix the issue there if possible.

Arno
Jakub Jelinek - Dec. 1, 2011, 8:55 a.m.
On Thu, Dec 01, 2011 at 09:50:15AM +0100, Arnaud Charlet wrote:
> > As written in the PR, even if you on say x86_64-linux
> > ../configure --enable-languages=c --target=i686-pc-linux-gnu
> > you end up with:
> > CONFIG_LANGUAGES =  c++ lto
> > LANGUAGES = c gcov$(exeext) gcov-dump$(exeext) $(CONFIG_LANGUAGES)
> > ...
> > LANG_MAKEFRAGS =  $(srcdir)/ada/gcc-interface/Make-lang.in
> > $(srcdir)/cp/Make-lang.in $(srcdir)/fortran/Make-lang.in
> > $(srcdir)/go/Make-lang.in $(srcdir)/java/Make-lang.in
> > $(srcdir)/lto/Make-lang.in $(srcdir)/objc/Make-lang.in
> > $(srcdir)/objcp/Make-lang.in
> 
> Do we know why LANG_MAKEFRAGS contains all the disabled languages?
> If that a feature/requirement or a bug/limitation?
> 
> If the latter, I'd rather fix the issue there if possible.

I believe because we want
cd obj/gcc
make f951
work even when --enable-languages=c,c++ , it is quite handy
not having to reconfigure gcc because you forgot one language, especially
when you have lots of cross-compilers around.
If LANG_MAKEFRAGS would be limited only to the chosen languages,
this would suddenly not work at all.

	Jakub
Jakub Jelinek - Dec. 1, 2011, 9:10 a.m.
On Thu, Dec 01, 2011 at 09:55:04AM +0100, Jakub Jelinek wrote:
> I believe because we want
> cd obj/gcc
> make f951
> work even when --enable-languages=c,c++ , it is quite handy
> not having to reconfigure gcc because you forgot one language, especially
> when you have lots of cross-compilers around.
> If LANG_MAKEFRAGS would be limited only to the chosen languages,
> this would suddenly not work at all.

http://gcc.gnu.org/ml/gcc-patches/2006-01/msg02100.html
is the change that went into 4.2.

	Jakub
Arnaud Charlet - Dec. 1, 2011, 9:29 a.m.
> > when you have lots of cross-compilers around.
> > If LANG_MAKEFRAGS would be limited only to the chosen languages,
> > this would suddenly not work at all.
> 
> http://gcc.gnu.org/ml/gcc-patches/2006-01/msg02100.html
> is the change that went into 4.2.

OK well, maybe the above patch is partly OBE because c++ is now required,
but I guess other people might argue similarly for other languages.

I personally don't think that's such a good idea, and reconfiguring gcc
is needed frequently anyway, and as we can see in this discussion, this does
have unwanted side effects.

This specific patch is "OK", but if we end up finding out that more of such
hacks are needed, then we'll have to revisit the whole issue and find
another solution.

Same thing if we find out that your patch is causing unexpected side
effects, we'll have to revert the patch.

Arno

Patch

--- gcc/ada/gcc-interface/Make-lang.in.jj	2011-11-28 17:58:01.000000000 +0100
+++ gcc/ada/gcc-interface/Make-lang.in	2011-11-30 11:47:18.723657889 +0100
@@ -118,7 +118,7 @@  ifeq ($(build), $(host))
 
     # put the host RTS dir first in the PATH to hide the default runtime
     # files that are among the sources
-    RTS_DIR:=$(strip $(subst \,/,$(shell gnatls -v | grep adalib )))
+    RTS_DIR=$(strip $(subst \,/,$(shell gnatls -v | grep adalib )))
 
     ADA_TOOLS_FLAGS_TO_PASS=\
         CC="$(CC)" \
@@ -153,7 +153,7 @@  else
   else
     # This is a canadian cross. We should use a toolchain running on the
     # build platform and targeting the host platform.
-    RTS_DIR:=$(strip $(subst \,/,$(shell $(GNATLS_FOR_HOST) -v | grep adalib )))
+    RTS_DIR=$(strip $(subst \,/,$(shell $(GNATLS_FOR_HOST) -v | grep adalib )))
     ADA_TOOLS_FLAGS_TO_PASS=\
         CC="$(CC)" \
         $(COMMON_FLAGS_TO_PASS) $(ADA_FLAGS_TO_PASS)  \