Re: PING [testsuite]: Tidy up testsuite handling of LD_LIBRARY_PATH

[Richard Sandiford <rdsandiford at googlemail dot com>]

"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Fri, Jul 10, 2009 at 2:52 AM, Richard
> Sandiford<rdsandiford@googlemail.com> wrote:
>> 2009/7/10 H.J. Lu <hjl.tools@gmail.com>:
>>> On Thu, Jul 9, 2009 at 7:49 PM, H.J. Lu<hjl.tools@gmail.com> wrote:
>>>> On Tue, Jun 30, 2009 at 3:20 PM, H.J. Lu<hjl.tools@gmail.com> wrote:
>>>>> On Tue, Jun 30, 2009 at 11:38 AM, Richard
>>>>> Sandiford<rdsandiford@googlemail.com> wrote:
>>>>>> Janis Johnson <janis187@us.ibm.com> writes:
>>>>>>> On Sat, 2009-06-27 at 09:42 +0100, Richard Sandiford wrote:
>>>>>>>> Ping for this patch:
>>>>>>>>
>>>>>>>> ? ? http://gcc.gnu.org/ml/gcc-patches/2009-06/msg00151.html
>>>>>>>>
>>>>>>>> (which should probably have had [testsuite] in the header, sorry).
>>>>>>>
>>>>>>> Good grief, it already had "testsuite" in the header twice! ?I don't
>>>>>>> know how I missed it.
>>>>>>>
>>>>>>> This is an area with which I'm not comfortable, but the patch looks
>>>>>>> much better thought-out than what was there before, so OK.
>>>>>>
>>>>>> Thanks, applied. ?I agree this is a bit of a tender area, so please
>>>>>> shout if I've broken something.
>>>>>>
>>>>>
>>>>> It may have caused:
>>>>>
>>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40601
>>>>>
>>>>
>>>> It also caused:
>>>>
>>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40707
>>>>
>>>
>>> It also caused:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40709
>>
>> OK, it looks like the cure is worse than the disease. ?I'll revert
>> these patches when I get home.
>> Sorry once again for all the hassle.
>>
>
> I posted a patch for PR 40707:
>
> http://gcc.gnu.org/ml/gcc-patches/2009-07/msg00602.html
>
> and one for 40709:
>
> http://gcc.gnu.org/ml/gcc-patches/2009-07/msg00603.html

Thanks HJ.

TBH, I'm not sure I really follow what's going wrong in 40707.
How did you stop the pre-patch harness from trampling over
LD_LIBRARY_PATH?  It could set the variable too, and it also
used "[is_remote target]".

I'm not too comfortable with the idea of using "isnative" here.
It's OK to test an x86_64-linux-gnu->i686-pc-linux-gnu "cross"
on the same machine, and you'd want the current LD_LIBRARY_PATH
handling there.

As far as 40709 goes: the reason for the difference is that I removed:

global orig_environment_saved

# This file may be sourced, so don't override environment settings
# that have been previously setup.
if { $orig_environment_saved == 0 } {
    append ld_library_path [gcc-set-multilib-library-path $GCC_UNDER_TEST]
    set_ld_library_path_env_vars
}

from the top-level of c-torture.exp and gcc-dg.exp and instead used:

    if { "$ld_library_path_multilib"
	 != "[board_info target multilib_flags]" } {
	set ld_library_path [find_libgcc_s $GCC_UNDER_TEST]
	set_ld_library_path_env_vars
    }

in c-torture-execute and gcc-dg-test-1.  But the PR makes we wonder
if we shouldn't be doing this in gcc_init instead.  struct-layout-1.exp
is very similar for C and C++, but this works for C++ because g++_init
is responsible for setting up the path.

Unfortunately, that could be another can of worms.

I think silently defaulting to "" is likely to paper over real bugs.
It looks like my patch has regressed the library path chosen for
C struct-layout-1; it no longer contains libgcc_s.so.  I think that
needs to be fixed.

My instinct is still to revert the patch.  It also caused PR40699,
although from Rainer's last message (thanks) it seems like it might
be tickling an underlying problem.

If we do revert, I could always come back to this later if I'm feeling
brave enough.  Hopefully I'll avoid making a hash of my testing the
second time round...

Thanks again for helping out with the patches.

Richard