Message ID | alpine.DEB.2.20.1905142118020.18422@tpp.hgst.com |
---|---|
State | Accepted |
Headers | show |
Series | GNAT test suite fixes for build sysroot | expand |
Maciej W. Rozycki wrote: > [...] > --- > gcc/testsuite/lib/gnat.exp | 2 ++ > 1 file changed, 2 insertions(+) > > gcc-test-gnat-options-ada.diff > Index: gcc/gcc/testsuite/lib/gnat.exp > =================================================================== > --- gcc.orig/gcc/testsuite/lib/gnat.exp > +++ gcc/gcc/testsuite/lib/gnat.exp > @@ -167,6 +167,8 @@ proc gnat_target_compile { source dest t > set options [concat "additional_flags=$TOOL_OPTIONS" $options] > } > > + set options [concat "{ada}" $options] > + > return [target_compile $source $dest $type $options] > } > Your Tcl syntax looks suspicious to me. Is there a reason for "ada" to be in both double quotes and braces? Perhaps {lappend options ada} might be simpler? Is placing ada at the beginning of the list important? -- Jacob
On Wed, 15 May 2019, Jacob Bachmeyer wrote: > > Index: gcc/gcc/testsuite/lib/gnat.exp > > =================================================================== > > --- gcc.orig/gcc/testsuite/lib/gnat.exp > > +++ gcc/gcc/testsuite/lib/gnat.exp > > @@ -167,6 +167,8 @@ proc gnat_target_compile { source dest t > > set options [concat "additional_flags=$TOOL_OPTIONS" $options] > > } > > > > + set options [concat "{ada}" $options] > > + > > return [target_compile $source $dest $type $options] > > } > > > Your Tcl syntax looks suspicious to me. Is there a reason for "ada" to > be in both double quotes and braces? Most existing `options' elements are lists, as shown by: clone_output "options: $options\n" placed at the top of `default_target_compile' (leading paths stripped here): options: {ada} {additional_flags=-fno-diagnostics-show-caret -fno-diagnostics-show-line-numbers -fdiagnostics-color=never --sysroot=.../sysroot} {additional_flags=-gnatJ -c -u} {compiler=.../gcc/gnatmake --GCC=.../gcc/xgcc --GNATBIND=.../gcc/gnatbind --GNATLINK=.../gcc/gnatlink -cargs -B.../gcc -largs --GCC=.../gcc/xgcc\ -B.../gcc\ -march=rv64imafdc\ -mabi=lp64d -margs --RTS=.../riscv64-linux-gnu/lib64/lp64d/libada -q -f} timeout=300 so I did this for consistency, although in reality it doesn't matter, not at least for `default_target_compile', and either approach would work. We are not consistent here in `gnat_target_compile' anyway, as you can see from the two existing `concat' invocations, and also the `timeout=300' element. > Perhaps {lappend options ada} might be simpler? Is placing ada at the > beginning of the list important? It can't be last because we override the default compiler otherwise selected by this option in `default_target_compile', and then options passed in may override it further. Overall I felt it to be safer if we placed the compiler type selection first rather than somewhere in the middle. I hope it clears your concerns. Maciej
Maciej W. Rozycki wrote: > On Wed, 15 May 2019, Jacob Bachmeyer wrote: > [...] > We are not consistent here in `gnat_target_compile' anyway, as you can > see from the two existing `concat' invocations, and also the `timeout=300' > element. > That is the GCC testsuite rather than DejaGnu itself, so it is less of a concern to me. >> Perhaps {lappend options ada} might be simpler? Is placing ada at the >> beginning of the list important? >> > > It can't be last because we override the default compiler otherwise > selected by this option in `default_target_compile', and then options > passed in may override it further. Overall I felt it to be safer if we > placed the compiler type selection first rather than somewhere in the > middle. > This is probably a bug in DejaGnu, (those options should set defaults rather than override whatever else has been given) but you will still need to work around it for the installed base. > I hope it clears your concerns. > As far as the patch to GCC goes, I am not worried. -- Jacob
Index: gcc/gcc/testsuite/lib/gnat.exp =================================================================== --- gcc.orig/gcc/testsuite/lib/gnat.exp +++ gcc/gcc/testsuite/lib/gnat.exp @@ -167,6 +167,8 @@ proc gnat_target_compile { source dest t set options [concat "additional_flags=$TOOL_OPTIONS" $options] } + set options [concat "{ada}" $options] + return [target_compile $source $dest $type $options] }