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

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Jul 10, 2009 at 11:36 AM, Richard
Sandiford<rdsandiford@googlemail.com> wrote:
> "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]".

We set up ld_library_path, not LD_LIBRARY_PATH. It is up
to board to set up LD_LIBRARY_PATH. See unix_load
in /usr/share/dejagnu/config/unix.exp.

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

In that case, I think you do want to set up ld_library_path, not simply
return. "isnative" is appropriate. You can give it a try.

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

I have no objections to revert the whole thing.

Thanks.


-- 
H.J.