This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC 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: PATCH: other/17466: Testsuites in gcc override LD_LIBRARY_PATH


H. J. Lu wrote:
> 	PR other/17466:
> 	* lib/gcc-defs.exp (${tool}_set_ld_library_path): New procedure.
> 	* g++.dg/compat/compat.exp (ld_library_path): Renamed to
> 	gcc_ld_library_path.
> 	(compat-fix-library-path): Removed.
> 	(compat-use-alt-compiler): Call ${tool}_set_ld_library_path to
> 	switch library path.
> 	(compat-use-tst-compiler): Likewise.
> 	* lib/g++.exp (g++_link_flags): Don't use global
> 	ld_library_path. Call ${tool}_set_ld_library_path to set up
> 	LD_LIBRARY_PATH, SHLIB_PATH, LD_LIBRARYN32_PATH,
> 	LD_LIBRARY64_PATH, LD_LIBRARY_PATH_32, LD_LIBRARY_PATH_64 and
> 	DYLD_LIBRARY_PATH.
> 	* lib/gfortran.exp (gfortran_link_flags): Likewise.
> 	* lib/objc.exp (objc_target_compile): Likewise.
> 	* lib/gcc-dg.exp: Call ${tool}_set_ld_library_path to set up
> 	LD_LIBRARY_PATH, SHLIB_PATH, LD_LIBRARYN32_PATH,
> 	LD_LIBRARY64_PATH, LD_LIBRARY_PATH_32, LD_LIBRARY_PATH_64 and
> 	DYLD_LIBRARY_PATH.

The description says this patch does one thing: preserve the value of
the user's setting of LD_LIBRARY_PATH.  The patch actually does two
things.  The other thing it does is try to include the directory
containing libgcc_s in ld_library_path.  This isn't mentioned in the
patch description, nor in the ChangeLog entry.

This patch was superceded by another patch from you in November.  So
this is actually the wrong patch to be reviewing.  The patch seems to be
    http://gcc.gnu.org/ml/gcc-patches/2004-11/msg01502.html
although looking closer, I see that in turn was superceded by another
patch on a different thread, started by John David Anglin.
    http://gcc.gnu.org/ml/gcc-patches/2004-11/msg02059.html
This one correctly mentions that the patch does two things.

Anyways, this all seems pointless, because the two bugs you are trying
to fix here were already fixed by other patches that have already been
checked in.

The first bug was fixed by a patch from John David Anglin (the new
thread mentioned above).
    http://gcc.gnu.org/ml/gcc-patches/2004-11/msg01538.html
This patch was inspired by yours, and is rather similar.

The second bug was fixed by a patch from Mark Mitchell.
    http://gcc.gnu.org/ml/gcc-patches/2005-03/msg02335.html
This is maybe unrelated to your patch, and I think much better.  You
tried to fix the problem in gcc-dg.exp, but not everyone uses that.
Mark's patch adds a utility function in gcc-defs, and then calls it from
various places which is a better solution.  Otherwise, Mark's solution
is similar to your, e.g. use of gcc --print-multi-lib.

It is possible that there are still some cases fixed by your patch that
weren't properly fixed by the two patches that have already been checked
in.  But in that case, you will have to write a new patch from scratch
that addresses the remaining problems.

I don't believe there is anything useful in this patch as things stand now.

:REVIEWURL http://gcc.gnu.org/ml/gcc-patches/2004-09/msg01626.html:
-- 
Jim Wilson, GNU Tools Support, http://www.specifix.com


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