Re: [PATCH] Add --enable-dynamic-test configure option

["Joseph S. Myers" <joseph at codesourcery dot com>]

The $(cross-compiling) conditionals in the patch are suspect (just about 
any $(cross-compiling) conditional is suspect).  There should be no reason 
not to support the new option properly for cross compiling, which would 
appear to be the effect of the conditionals on the definitions of 
test-program-prefix etc.

Instead, I think you should just use ifeq (yes,$(build-dynamic-test)) 
instead of ifeq (noyes,$(cross-compiling)$(build-dynamic-test)), and all 
of the variables you define in that case other than host-test-program-cmd 
should start with $(test-wrapper), which also helps for any other use 
cases of wrappers.

All four of the new variables also need extended comments explaining their 
precise semantics, similar to those I put on $(run-via-rtld-prefix), 
$(run-program-prefix), $(built-program-cmd) and $(host-built-program-cmd).  
Those comments explaining the precise semantics would also help elucidate 
whether what I said in the previous paragraph about when to use 
$(test-wrapper) is correct.

I'm concerned about the change to elf/Makefile for running "order".  You 
appear to change it unconditionally to run the binary directly rather than 
via ld.so --library-path.  When the new configure option isn't passed, 
won't that try to run it with whatever old dynamic linker might be already 
installed on the system?  Have you tested without the new configure 
option?  Likewise, order2.

If you wish to include any changes that affect how a test is run *without* 
the new configure option, please split such changes out and submit them 
separately, so that the patch adding the new configure option makes no 
difference at all to how anything is built or run when the option isn't 
used.

The additions of -static versions of tests look like another thing that 
should be split out.

Could you clarify what the bug-setlocale1.c change is for and why it is 
safe (in the absence of the new configure option)?

Note that localedata/ has its own ChangeLog.

-- 
Joseph S. Myers
joseph@codesourcery.com