* [RFC/commmit] [testsuite/Ada] stop using project files when building test programs
@ 2015-12-22 15:33 Joel Brobecker
2015-12-23 13:21 ` Pedro Alves
0 siblings, 1 reply; 7+ messages in thread
From: Joel Brobecker @ 2015-12-22 15:33 UTC (permalink / raw)
To: gdb-patches
Hello,
Below is a fairly straightforward change, except maybe for the use
of try / finally, which requires TCL version 8.6, released Dec 2012.
I think it's a reasonable requirement; any objection?
Thanks!
-------------------------------------------------------------
The current approach when building Ada programs for testing is
based on the use of a project file (testsuite/gdb.ada/gnat_ada.gpr).
To do that, we pass a number of additional arguments to target_compile,
one of them being the project file (via "-P/path/to/gnat_ada.gpr").
This used to work well-enough, but AdaCore is currently working towards
removing project-file support from gnatmake (the prefered tool for
using project files is gprbuild). So, we need to either switch
the compilation to gprbuild, or stop using project files.
First, using gprbuild is not always what users will be using to
build their applications. So having the option of using gnatmake
provides more flexibility towards exactly reproducing past bugs.
If we ever need a testcase that requires the use of gprbuild, then
I believe support for a new target needs to be added to dejagnu's
target_compile.
Also, the only real reason behind using a project file in the first
place is that we wanted to make it easy to specify the directory
where all compilation artifacts get stored. This is a consequence
of the organization choice we made for gdb.ada to keep each testcase
well organized. It is very easy to achieve that goal without using
project files.
This is therefore what this patch does: It change gdb_compile_ada
to build any program using gnatmake without using a project file
(by temporarily changing the current working directory).
There is a small (beneficial) side-effect; in the situation where
GDB is built in-tree, gnatmake is called as...
% gnatmake [...] unit.adb
... which means that the debugging info in unit.o will say contain
a filename whose name is 'unit.adb', rather than '/path/to/unit.adb'.
This also better matches what users might typically do. But the side-
effect is that the unit name in the GDB output is not always a full
path. This patch tweaks a couple of testcases to make the path part
optional.
Note that this patch requires TCL version 8.6, which was released
Dec 2012.
gdb/testsuite:
* lib/ada.exp (gdb_compile_ada): Reimplement avoiding the use
of project files.
* gdb.ada/gnat_ada.gpr: Delete.
* gdb.ada/cond_lang.exp: Adjust test to make path before
filename optional.
* gdb.ada/small_reg_param.exp: Likewise.
Tested on x86_64-linux, with both in-tree and out-of-tree builds.
---
gdb/testsuite/gdb.ada/cond_lang.exp | 2 +-
gdb/testsuite/gdb.ada/gnat_ada.gpr | 25 -------------------------
gdb/testsuite/gdb.ada/small_reg_param.exp | 2 +-
gdb/testsuite/lib/ada.exp | 20 ++++++++++++++++----
4 files changed, 18 insertions(+), 31 deletions(-)
delete mode 100644 gdb/testsuite/gdb.ada/gnat_ada.gpr
diff --git a/gdb/testsuite/gdb.ada/cond_lang.exp b/gdb/testsuite/gdb.ada/cond_lang.exp
index 0dfb9e3..7c3ad6e 100644
--- a/gdb/testsuite/gdb.ada/cond_lang.exp
+++ b/gdb/testsuite/gdb.ada/cond_lang.exp
@@ -39,7 +39,7 @@ gdb_test "show lang" \
# current language mode is auto, and the breakpoint is inside Ada code.
set bp_location [gdb_get_line_number "STOP" ${testdir}/mixed.adb]
gdb_test "break mixed.adb:${bp_location} if light = green" \
- "Breakpoint \[0-9\]* at .*: file .*/mixed.adb, line \[0-9\]*\\."
+ "Breakpoint \[0-9\]* at .*: file (.*/)?mixed.adb, line \[0-9\]*\\."
# Now, continue until we hit the breakpoint. If the condition is
# evaluated correctly, the first hit will be ignored, and the debugger
diff --git a/gdb/testsuite/gdb.ada/gnat_ada.gpr b/gdb/testsuite/gdb.ada/gnat_ada.gpr
deleted file mode 100644
index 2736206..0000000
--- a/gdb/testsuite/gdb.ada/gnat_ada.gpr
+++ /dev/null
@@ -1,25 +0,0 @@
--- Copyright 2004-2015 Free Software Foundation, Inc.
---
--- This program is free software; you can redistribute it and/or modify
--- it under the terms of the GNU General Public License as published by
--- the Free Software Foundation; either version 3 of the License, or
--- (at your option) any later version.
---
--- This program is distributed in the hope that it will be useful,
--- but WITHOUT ANY WARRANTY; without even the implied warranty of
--- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
--- GNU General Public License for more details.
---
--- You should have received a copy of the GNU General Public License
--- along with this program. If not, see <http://www.gnu.org/licenses/>.
-
--- This project file allows us to control the location where the
--- compilation artifacts produced when building the Ada examples
--- are stored.
-
-project Gnat_Ada is
-
- for Source_Dirs use (external ("SRC"));
- for Object_Dir use external ("OBJ");
-
-end Gnat_Ada;
diff --git a/gdb/testsuite/gdb.ada/small_reg_param.exp b/gdb/testsuite/gdb.ada/small_reg_param.exp
index bd5cfd6..0a3b972 100644
--- a/gdb/testsuite/gdb.ada/small_reg_param.exp
+++ b/gdb/testsuite/gdb.ada/small_reg_param.exp
@@ -33,7 +33,7 @@ gdb_breakpoint "call_me"
# Continue until we hit the breakpoint inside `Call_Me'. We verify
# that the parameter value is correct.
gdb_test "continue" \
- "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at .*/pck.adb:.*" \
+ "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at (.*)?/pck.adb:.*" \
"continue to call_me"
# And just to make sure, we also verify that the parameter value
diff --git a/gdb/testsuite/lib/ada.exp b/gdb/testsuite/lib/ada.exp
index 6a1e192..b8a1724 100644
--- a/gdb/testsuite/lib/ada.exp
+++ b/gdb/testsuite/lib/ada.exp
@@ -21,12 +21,24 @@ proc gdb_compile_ada {source dest type options} {
set gprdir [file dirname $srcdir]
set objdir [file dirname $dest]
+ # Although strictly not necessary, we force the recompilation
+ # of all units (additional_flags=-f). This is what is done
+ # when using GCC to build programs in the other languages,
+ # and it avoids using a stray objfile file from a long-past
+ # run, for instance.
append options " ada"
- append options " additional_flags=-P$gprdir/gnat_ada"
- append options " additional_flags=-XSRC=[file tail $srcdir]"
- append options " additional_flags=-XOBJ=$objdir"
+ append options " additional_flags=-f"
+ append options " additional_flags=-I$srcdir"
- set result [target_compile [file tail $source] $dest $type $options]
+ # Run target_compile from the directory where we want the object
+ # files and the executable to be written.
+ set saved_cwd [pwd]
+ try {
+ cd $objdir
+ set result [target_compile [file tail $source] $dest $type $options]
+ } finally {
+ cd $saved_cwd
+ }
# The Ada build always produces some output, even when the build
# succeeds. Thus, we can not use the output the same way we do in
--
2.1.4
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [RFC/commmit] [testsuite/Ada] stop using project files when building test programs 2015-12-22 15:33 [RFC/commmit] [testsuite/Ada] stop using project files when building test programs Joel Brobecker @ 2015-12-23 13:21 ` Pedro Alves 2015-12-23 13:35 ` Joel Brobecker 0 siblings, 1 reply; 7+ messages in thread From: Pedro Alves @ 2015-12-23 13:21 UTC (permalink / raw) To: Joel Brobecker, gdb-patches On 12/22/2015 03:33 PM, Joel Brobecker wrote: > Hello, > > Below is a fairly straightforward change, except maybe for the use > of try / finally, which requires TCL version 8.6, released Dec 2012. > I think it's a reasonable requirement; any objection? I'm still on F20, and that shipped with 8.5 too. I'll have to upgrade eventually, so this may the trigger. Note that requiring newer expect/tcl may mean that fewer people will do the occasional gcc compile farm testing on some older hosts. From a quick look at some machines there, I see: gcc20 (debian wheezy)'s expect links with 8.5. gcc111 (AIX 7.1)'s expect links with 8.4. gcc110 (F18 POWER7/ppc64)'s expect links with 8.5. How hard would it be to avoid try/finally? Wouldn't you just have to use catch instead? Are there other nice 8.6 features that it'd be really nice to make use of too? Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC/commmit] [testsuite/Ada] stop using project files when building test programs 2015-12-23 13:21 ` Pedro Alves @ 2015-12-23 13:35 ` Joel Brobecker 2015-12-23 14:43 ` Joel Brobecker 0 siblings, 1 reply; 7+ messages in thread From: Joel Brobecker @ 2015-12-23 13:35 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches > > Below is a fairly straightforward change, except maybe for the use > > of try / finally, which requires TCL version 8.6, released Dec 2012. > > I think it's a reasonable requirement; any objection? > > I'm still on F20, and that shipped with 8.5 too. I'll have to upgrade > eventually, so this may the trigger. > > Note that requiring newer expect/tcl may mean that fewer people will do > the occasional gcc compile farm testing on some older hosts. From a quick > look at some machines there, I see: > > gcc20 (debian wheezy)'s expect links with 8.5. > gcc111 (AIX 7.1)'s expect links with 8.4. > gcc110 (F18 POWER7/ppc64)'s expect links with 8.5. Humpf, that's a fair number of reasons showing that assuming 8.6 may not be reasonable. Bouh... > How hard would it be to avoid try/finally? Wouldn't you just have to > use catch instead? I don't think it would be very hard. I think catch will work, but will be a little more convoluted. I'll give it a try... > Are there other nice 8.6 features that it'd be really nice to > make use of too? This is what the 8.6 release notes say: (https://www.tcl.tk/software/tcltk/8.6.html) Highlights of Tcl 8.6 * Object Oriented Programming [prob not very intesting for us, but see URL if interested] * Stackless Evaluation: The evaluation of many levels of nested proc calls are no longer implemented as a stack of nested C routine calls. This revision in the internal implementation of Tcl evaluation makes deep recursion in Tcl scripts safe to do. But there's more... This new implementation enables a collection of new commands, coroutine, tailcall, yield, and yieldto that provide profound new capabilities and models of concurrency to Tcl scripts. * Enhanced Exceptions: New commands try and throw and a wealth of new -errorcode values enable far more precise trapping and handling of exceptions using a familiar construct. * Batteries Included: Tcl delivers in the pkgs subdirectory a bundled collection of third-party packages built and installed along with Tcl. * Thread-enabled Operations: A thread-enabled default build, a bundled Thread package, and new command interp cancel make Tcl 8.6 ready for your multi-threaded programming tasks. * SQL Database Powered: [prob not very intesting for us, but see URL if interested] * IPv6 Networking: Both client and server sockets support IPv6 where platform support exists. * Built-in Zlib Compression: New command zlib provides utilities to handle compression of data and streams. * List Processing: New commands lmap and dict map enable the elegant expression of transformations over Tcl containers. * Stacked Channels by Script: New commands chan push and chan pop expose the power of stacked channels without the need to write C code. * Additional New Features: Temporary file creation, enhancements to list sorting and setting, dict filtering, half-close of bidirectional channels, encoding and decoding of binary sequences, finer control over load, and many many more. Maybe some interesting stuff, but I wouldn't want compilation of Ada code be the trigger that breaks people's testing. Upgrading tcl/expect is not always a trivial task... Thanks for the feedback, -- Joel ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC/commmit] [testsuite/Ada] stop using project files when building test programs 2015-12-23 13:35 ` Joel Brobecker @ 2015-12-23 14:43 ` Joel Brobecker 2015-12-23 15:21 ` Pedro Alves 0 siblings, 1 reply; 7+ messages in thread From: Joel Brobecker @ 2015-12-23 14:43 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 944 bytes --] > Humpf, that's a fair number of reasons showing that assuming 8.6 > may not be reasonable. Bouh... > > > How hard would it be to avoid try/finally? Wouldn't you just have to > > use catch instead? > > I don't think it would be very hard. I think catch will work, but > will be a little more convoluted. I'll give it a try... Still, for a complete dummy like myself, that took nearly 45 minutes, just to make sure I understand TCL semantics better, and find a machine that has TCL 8.5... I would appreciate a review of the attached patch, because TCL and I are not regular friends. gdb/testsuite: * lib/ada.exp (target_compile_ada_from_dir): New function. (gdb_compile_ada): Reimplement avoiding the use of project files. * gdb.ada/gnat_ada.gpr: Delete. * gdb.ada/cond_lang.exp: Adjust test to make path before filename optional. * gdb.ada/small_reg_param.exp: Likewise. Thanks! -- Joel [-- Attachment #2: 0001-testsuite-Ada-stop-using-project-files-when-building.patch --] [-- Type: text/x-diff, Size: 7802 bytes --] From 672d8857a125959b51e5f760cfc7f1c5d5a851cc Mon Sep 17 00:00:00 2001 From: Joel Brobecker <brobecker@adacore.com> Date: Tue, 22 Dec 2015 13:28:41 +0400 Subject: [PATCH] [testsuite/Ada] stop using project files when building test programs The current approach when building Ada programs for testing is based on the use of a project file (testsuite/gdb.ada/gnat_ada.gpr). To do that, we pass a number of additional arguments to target_compile, one of them being the project file (via "-P/path/to/gnat_ada.gpr"). This used to work well-enough, but AdaCore is currently working towards removing project-file support from gnatmake (the prefered tool for using project files is gprbuild). So, we need to either switch the compilation to gprbuild, or stop using project files. First, using gprbuild is not always what users will be using to build their applications. So having the option of using gnatmake provides more flexibility towards exactly reproducing past bugs. If we ever need a testcase that requires the use of gprbuild, then I believe support for a new target needs to be added to dejagnu's target_compile. Also, the only real reason behind using a project file in the first place is that we wanted to make it easy to specify the directory where all compilation artifacts get stored. This is a consequence of the organization choice we made for gdb.ada to keep each testcase well organized. It is very easy to achieve that goal without using project files. This is therefore what this patch does: It change gdb_compile_ada to build any program using gnatmake without using a project file (by temporarily changing the current working directory). There is a small (beneficial) side-effect; in the situation where GDB is built in-tree, gnatmake is called as... % gnatmake [...] unit.adb ... which means that the debugging info in unit.o will say contain a filename whose name is 'unit.adb', rather than '/path/to/unit.adb'. This also better matches what users might typically do. But the side- effect is that the unit name in the GDB output is not always a full path. This patch tweaks a couple of testcases to make the path part optional. gdb/testsuite: * lib/ada.exp (target_compile_ada_from_dir): New function. (gdb_compile_ada): Reimplement avoiding the use of project files. * gdb.ada/gnat_ada.gpr: Delete. * gdb.ada/cond_lang.exp: Adjust test to make path before filename optional. * gdb.ada/small_reg_param.exp: Likewise. Tested on x86_64-linux, with both in-tree and out-of-tree builds. --- gdb/testsuite/gdb.ada/cond_lang.exp | 2 +- gdb/testsuite/gdb.ada/gnat_ada.gpr | 25 ----------------------- gdb/testsuite/gdb.ada/small_reg_param.exp | 2 +- gdb/testsuite/lib/ada.exp | 33 +++++++++++++++++++++++++++---- 4 files changed, 31 insertions(+), 31 deletions(-) delete mode 100644 gdb/testsuite/gdb.ada/gnat_ada.gpr diff --git a/gdb/testsuite/gdb.ada/cond_lang.exp b/gdb/testsuite/gdb.ada/cond_lang.exp index 0dfb9e3..7c3ad6e 100644 --- a/gdb/testsuite/gdb.ada/cond_lang.exp +++ b/gdb/testsuite/gdb.ada/cond_lang.exp @@ -39,7 +39,7 @@ gdb_test "show lang" \ # current language mode is auto, and the breakpoint is inside Ada code. set bp_location [gdb_get_line_number "STOP" ${testdir}/mixed.adb] gdb_test "break mixed.adb:${bp_location} if light = green" \ - "Breakpoint \[0-9\]* at .*: file .*/mixed.adb, line \[0-9\]*\\." + "Breakpoint \[0-9\]* at .*: file (.*/)?mixed.adb, line \[0-9\]*\\." # Now, continue until we hit the breakpoint. If the condition is # evaluated correctly, the first hit will be ignored, and the debugger diff --git a/gdb/testsuite/gdb.ada/gnat_ada.gpr b/gdb/testsuite/gdb.ada/gnat_ada.gpr deleted file mode 100644 index 2736206..0000000 --- a/gdb/testsuite/gdb.ada/gnat_ada.gpr +++ /dev/null @@ -1,25 +0,0 @@ --- Copyright 2004-2015 Free Software Foundation, Inc. --- --- This program is free software; you can redistribute it and/or modify --- it under the terms of the GNU General Public License as published by --- the Free Software Foundation; either version 3 of the License, or --- (at your option) any later version. --- --- This program is distributed in the hope that it will be useful, --- but WITHOUT ANY WARRANTY; without even the implied warranty of --- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the --- GNU General Public License for more details. --- --- You should have received a copy of the GNU General Public License --- along with this program. If not, see <http://www.gnu.org/licenses/>. - --- This project file allows us to control the location where the --- compilation artifacts produced when building the Ada examples --- are stored. - -project Gnat_Ada is - - for Source_Dirs use (external ("SRC")); - for Object_Dir use external ("OBJ"); - -end Gnat_Ada; diff --git a/gdb/testsuite/gdb.ada/small_reg_param.exp b/gdb/testsuite/gdb.ada/small_reg_param.exp index bd5cfd6..0a3b972 100644 --- a/gdb/testsuite/gdb.ada/small_reg_param.exp +++ b/gdb/testsuite/gdb.ada/small_reg_param.exp @@ -33,7 +33,7 @@ gdb_breakpoint "call_me" # Continue until we hit the breakpoint inside `Call_Me'. We verify # that the parameter value is correct. gdb_test "continue" \ - "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at .*/pck.adb:.*" \ + "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at (.*)?/pck.adb:.*" \ "continue to call_me" # And just to make sure, we also verify that the parameter value diff --git a/gdb/testsuite/lib/ada.exp b/gdb/testsuite/lib/ada.exp index 6a1e192..28284ff 100644 --- a/gdb/testsuite/lib/ada.exp +++ b/gdb/testsuite/lib/ada.exp @@ -13,6 +13,26 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. +# Call target_compile with SOURCE DEST TYPE and OPTIONS as argument, +# after having temporarily changed the current working directory to +# BUILDDIR. +# +# FIXME: brobecke/2015-12-23: We can get rid of this function entirely +# when we are able to migrate to TCL 8.6, which added support for +# try { ... } finally {...} constructs. Despite being released on +# December 2012, that version is still far from being widely used +# (in particular in the more exotic systems of the GCC compile farm). + +proc target_compile_ada_from_dir {builddir source dest type options} { + set saved_cwd [pwd] + catch { + cd $builddir + return [target_compile $source $dest $type $options] + } result options + cd $saved_cwd + return -options $options $result +} + # Compile some Ada code. proc gdb_compile_ada {source dest type options} { @@ -21,12 +41,17 @@ proc gdb_compile_ada {source dest type options} { set gprdir [file dirname $srcdir] set objdir [file dirname $dest] + # Although strictly not necessary, we force the recompilation + # of all units (additional_flags=-f). This is what is done + # when using GCC to build programs in the other languages, + # and it avoids using a stray objfile file from a long-past + # run, for instance. append options " ada" - append options " additional_flags=-P$gprdir/gnat_ada" - append options " additional_flags=-XSRC=[file tail $srcdir]" - append options " additional_flags=-XOBJ=$objdir" + append options " additional_flags=-f" + append options " additional_flags=-I$srcdir" - set result [target_compile [file tail $source] $dest $type $options] + set result [target_compile_ada_from_dir \ + $objdir [file tail $source] $dest $type $options]] # The Ada build always produces some output, even when the build # succeeds. Thus, we can not use the output the same way we do in -- 2.1.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC/commmit] [testsuite/Ada] stop using project files when building test programs 2015-12-23 14:43 ` Joel Brobecker @ 2015-12-23 15:21 ` Pedro Alves 2015-12-23 15:30 ` Pedro Alves 0 siblings, 1 reply; 7+ messages in thread From: Pedro Alves @ 2015-12-23 15:21 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches On 12/23/2015 02:43 PM, Joel Brobecker wrote: >> > Humpf, that's a fair number of reasons showing that assuming 8.6 >> > may not be reasonable. Bouh... >> > >>> > > How hard would it be to avoid try/finally? Wouldn't you just have to >>> > > use catch instead? >> > >> > I don't think it would be very hard. I think catch will work, but >> > will be a little more convoluted. I'll give it a try... > Still, for a complete dummy like myself, that took nearly 45 minutes, > just to make sure I understand TCL semantics better, and find a machine > that has TCL 8.5... > Thanks for doing this. > I would appreciate a review of the attached patch, because TCL and > I are not regular friends. :-) Looks generally good to me. A couple minor comments below. > --- a/gdb/testsuite/gdb.ada/cond_lang.exp > +++ b/gdb/testsuite/gdb.ada/cond_lang.exp > @@ -39,7 +39,7 @@ gdb_test "show lang" \ > # current language mode is auto, and the breakpoint is inside Ada code. > set bp_location [gdb_get_line_number "STOP" ${testdir}/mixed.adb] > gdb_test "break mixed.adb:${bp_location} if light = green" \ > - "Breakpoint \[0-9\]* at .*: file .*/mixed.adb, line \[0-9\]*\\." > + "Breakpoint \[0-9\]* at .*: file (.*/)?mixed.adb, line \[0-9\]*\\." Isn't that the same as just: - "Breakpoint \[0-9\]* at .*: file .*/mixed.adb, line \[0-9\]*\\." + "Breakpoint \[0-9\]* at .*: file .*mixed.adb, line \[0-9\]*\\." ? > gdb_test "continue" \ > - "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at .*/pck.adb:.*" \ > + "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at (.*)?/pck.adb:.*" \ > "continue to call_me" Likewise, I think this is a no-op. Did you mean to put the / inside the parens like in the other case? If so I'd suggest: - "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at .*/pck.adb:.*" \ + "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at .*pck.adb:.*" \ > > # And just to make sure, we also verify that the parameter value > diff --git a/gdb/testsuite/lib/ada.exp b/gdb/testsuite/lib/ada.exp > index 6a1e192..28284ff 100644 > --- a/gdb/testsuite/lib/ada.exp > +++ b/gdb/testsuite/lib/ada.exp > @@ -13,6 +13,26 @@ > # You should have received a copy of the GNU General Public License > # along with this program. If not, see <http://www.gnu.org/licenses/>. > > +# Call target_compile with SOURCE DEST TYPE and OPTIONS as argument, > +# after having temporarily changed the current working directory to > +# BUILDDIR. > +# > +# FIXME: brobecke/2015-12-23: We can get rid of this function entirely > +# when we are able to migrate to TCL 8.6, which added support for > +# try { ... } finally {...} constructs. Despite being released on > +# December 2012, that version is still far from being widely used > +# (in particular in the more exotic systems of the GCC compile farm). Not sure this FIXME comment here turns out useful going forward. We have many other instances of catch in the harness. And, IMO, having a wrapper function looks nicer than open coding try/finally at the caller. Once we rely on 8.6 and thus try/finally, I would think we'd keep the wrapper function, but reimplement its body with try/finally instead. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC/commmit] [testsuite/Ada] stop using project files when building test programs 2015-12-23 15:21 ` Pedro Alves @ 2015-12-23 15:30 ` Pedro Alves 2015-12-24 5:30 ` Joel Brobecker 0 siblings, 1 reply; 7+ messages in thread From: Pedro Alves @ 2015-12-23 15:30 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches On 12/23/2015 03:21 PM, Pedro Alves wrote: >> > --- a/gdb/testsuite/gdb.ada/cond_lang.exp >> > +++ b/gdb/testsuite/gdb.ada/cond_lang.exp >> > @@ -39,7 +39,7 @@ gdb_test "show lang" \ >> > # current language mode is auto, and the breakpoint is inside Ada code. >> > set bp_location [gdb_get_line_number "STOP" ${testdir}/mixed.adb] >> > gdb_test "break mixed.adb:${bp_location} if light = green" \ >> > - "Breakpoint \[0-9\]* at .*: file .*/mixed.adb, line \[0-9\]*\\." >> > + "Breakpoint \[0-9\]* at .*: file (.*/)?mixed.adb, line \[0-9\]*\\." > Isn't that the same as just: > > - "Breakpoint \[0-9\]* at .*: file .*/mixed.adb, line \[0-9\]*\\." > + "Breakpoint \[0-9\]* at .*: file .*mixed.adb, line \[0-9\]*\\." > > ? > Huh, sorry, brain malfunction. My version would give out a spurious PASS with foomixed.adb, yours wouldn't. >> > gdb_test "continue" \ >> > - "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at .*/pck.adb:.*" \ >> > + "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at (.*)?/pck.adb:.*" \ >> > "continue to call_me" > Likewise, I think this is a no-op. Did you mean to put the / inside the > parens like in the other case? If so I'd suggest: This one still looks odd to me, though. Thanks, Pedro Alves ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC/commmit] [testsuite/Ada] stop using project files when building test programs 2015-12-23 15:30 ` Pedro Alves @ 2015-12-24 5:30 ` Joel Brobecker 0 siblings, 0 replies; 7+ messages in thread From: Joel Brobecker @ 2015-12-24 5:30 UTC (permalink / raw) To: Pedro Alves; +Cc: gdb-patches [-- Attachment #1: Type: text/plain, Size: 1941 bytes --] > >> > gdb_test "break mixed.adb:${bp_location} if light = green" \ > >> > - "Breakpoint \[0-9\]* at .*: file .*/mixed.adb, line \[0-9\]*\\." > >> > + "Breakpoint \[0-9\]* at .*: file (.*/)?mixed.adb, line \[0-9\]*\\." > > Isn't that the same as just: > > > > - "Breakpoint \[0-9\]* at .*: file .*/mixed.adb, line \[0-9\]*\\." > > + "Breakpoint \[0-9\]* at .*: file .*mixed.adb, line \[0-9\]*\\." > > > > ? > > > > Huh, sorry, brain malfunction. My version would give out a spurious > PASS with foomixed.adb, yours wouldn't. That's the idea, yes. And to be more complete, you would get a full path in the case where you built GDB out-of-tree, which you would get a simple filename if when you build GDB in-tree. > >> > gdb_test "continue" \ > >> > - "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at .*/pck.adb:.*" \ > >> > + "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at (.*)?/pck.adb:.*" \ > >> > "continue to call_me" > > Likewise, I think this is a no-op. Did you mean to put the / inside the > > parens like in the other case? If so I'd suggest: > > This one still looks odd to me, though. ... and it is wrong. I don't understand how it could have slipped, since I am supposed to have tested it in both in-tree and out-of-tree, and this should have caused a failure in the in-tree case. Thanks for spotting it! I fixed this, removed the FIXME, and retested both in-tree and out-of-tree, getting clean and identical results in both cases. gdb/testsuite: * lib/ada.exp (target_compile_ada_from_dir): New function. (gdb_compile_ada): Reimplement avoiding the use of project files. * gdb.ada/gnat_ada.gpr: Delete. * gdb.ada/cond_lang.exp: Adjust test to make path before filename optional. * gdb.ada/small_reg_param.exp: Likewise. Pushed to master. Thanks again for the help, Pedro, and Merry Christmas :) -- Joel [-- Attachment #2: 0001-testsuite-Ada-stop-using-project-files-when-building.patch --] [-- Type: text/x-diff, Size: 8135 bytes --] From ab8314b3d99625c9a2125d39f4f3e74bf9e49cce Mon Sep 17 00:00:00 2001 From: Joel Brobecker <brobecker@adacore.com> Date: Tue, 22 Dec 2015 13:28:41 +0400 Subject: [PATCH] [testsuite/Ada] stop using project files when building test programs The current approach when building Ada programs for testing is based on the use of a project file (testsuite/gdb.ada/gnat_ada.gpr). To do that, we pass a number of additional arguments to target_compile, one of them being the project file (via "-P/path/to/gnat_ada.gpr"). This used to work well-enough, but AdaCore is currently working towards removing project-file support from gnatmake (the prefered tool for using project files is gprbuild). So, we need to either switch the compilation to gprbuild, or stop using project files. First, using gprbuild is not always what users will be using to build their applications. So having the option of using gnatmake provides more flexibility towards exactly reproducing past bugs. If we ever need a testcase that requires the use of gprbuild, then I believe support for a new target needs to be added to dejagnu's target_compile. Also, the only real reason behind using a project file in the first place is that we wanted to make it easy to specify the directory where all compilation artifacts get stored. This is a consequence of the organization choice we made for gdb.ada to keep each testcase well organized. It is very easy to achieve that goal without using project files. This is therefore what this patch does: It change gdb_compile_ada to build any program using gnatmake without using a project file (by temporarily changing the current working directory). There is a small (beneficial) side-effect; in the situation where GDB is built in-tree, gnatmake is called as... % gnatmake [...] unit.adb ... which means that the debugging info in unit.o will say contain a filename whose name is 'unit.adb', rather than '/path/to/unit.adb'. This also better matches what users might typically do. But the side- effect is that the unit name in the GDB output is not always a full path. This patch tweaks a couple of testcases to make the path part optional. gdb/testsuite: * lib/ada.exp (target_compile_ada_from_dir): New function. (gdb_compile_ada): Reimplement avoiding the use of project files. * gdb.ada/gnat_ada.gpr: Delete. * gdb.ada/cond_lang.exp: Adjust test to make path before filename optional. * gdb.ada/small_reg_param.exp: Likewise. Tested on x86_64-linux, with both in-tree and out-of-tree builds. --- gdb/testsuite/ChangeLog | 9 +++++++++ gdb/testsuite/gdb.ada/cond_lang.exp | 2 +- gdb/testsuite/gdb.ada/gnat_ada.gpr | 25 ------------------------- gdb/testsuite/gdb.ada/small_reg_param.exp | 2 +- gdb/testsuite/lib/ada.exp | 27 +++++++++++++++++++++++---- 5 files changed, 34 insertions(+), 31 deletions(-) delete mode 100644 gdb/testsuite/gdb.ada/gnat_ada.gpr diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 2d4e795..eb355b9 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2015-12-24 Joel Brobecker <brobecker@adacore.com> + + * lib/ada.exp (target_compile_ada_from_dir): New function. + (gdb_compile_ada): Reimplement avoiding the use of project files. + * gdb.ada/gnat_ada.gpr: Delete. + * gdb.ada/cond_lang.exp: Adjust test to make path before + filename optional. + * gdb.ada/small_reg_param.exp: Likewise. + 2015-12-22 Simon Marchi <simon.marchi@ericsson.com> * gdb.base/foll-vork.exp: Remove HP-UX special case. diff --git a/gdb/testsuite/gdb.ada/cond_lang.exp b/gdb/testsuite/gdb.ada/cond_lang.exp index 0dfb9e3..7c3ad6e 100644 --- a/gdb/testsuite/gdb.ada/cond_lang.exp +++ b/gdb/testsuite/gdb.ada/cond_lang.exp @@ -39,7 +39,7 @@ gdb_test "show lang" \ # current language mode is auto, and the breakpoint is inside Ada code. set bp_location [gdb_get_line_number "STOP" ${testdir}/mixed.adb] gdb_test "break mixed.adb:${bp_location} if light = green" \ - "Breakpoint \[0-9\]* at .*: file .*/mixed.adb, line \[0-9\]*\\." + "Breakpoint \[0-9\]* at .*: file (.*/)?mixed.adb, line \[0-9\]*\\." # Now, continue until we hit the breakpoint. If the condition is # evaluated correctly, the first hit will be ignored, and the debugger diff --git a/gdb/testsuite/gdb.ada/gnat_ada.gpr b/gdb/testsuite/gdb.ada/gnat_ada.gpr deleted file mode 100644 index 2736206..0000000 --- a/gdb/testsuite/gdb.ada/gnat_ada.gpr +++ /dev/null @@ -1,25 +0,0 @@ --- Copyright 2004-2015 Free Software Foundation, Inc. --- --- This program is free software; you can redistribute it and/or modify --- it under the terms of the GNU General Public License as published by --- the Free Software Foundation; either version 3 of the License, or --- (at your option) any later version. --- --- This program is distributed in the hope that it will be useful, --- but WITHOUT ANY WARRANTY; without even the implied warranty of --- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the --- GNU General Public License for more details. --- --- You should have received a copy of the GNU General Public License --- along with this program. If not, see <http://www.gnu.org/licenses/>. - --- This project file allows us to control the location where the --- compilation artifacts produced when building the Ada examples --- are stored. - -project Gnat_Ada is - - for Source_Dirs use (external ("SRC")); - for Object_Dir use external ("OBJ"); - -end Gnat_Ada; diff --git a/gdb/testsuite/gdb.ada/small_reg_param.exp b/gdb/testsuite/gdb.ada/small_reg_param.exp index bd5cfd6..8212e50 100644 --- a/gdb/testsuite/gdb.ada/small_reg_param.exp +++ b/gdb/testsuite/gdb.ada/small_reg_param.exp @@ -33,7 +33,7 @@ gdb_breakpoint "call_me" # Continue until we hit the breakpoint inside `Call_Me'. We verify # that the parameter value is correct. gdb_test "continue" \ - "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at .*/pck.adb:.*" \ + "Breakpoint .*, pck\\.call_me \\(w=(w@entry=)?50\\) at (.*/)?pck.adb:.*" \ "continue to call_me" # And just to make sure, we also verify that the parameter value diff --git a/gdb/testsuite/lib/ada.exp b/gdb/testsuite/lib/ada.exp index 6a1e192..1c243ad 100644 --- a/gdb/testsuite/lib/ada.exp +++ b/gdb/testsuite/lib/ada.exp @@ -13,6 +13,20 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. +# Call target_compile with SOURCE DEST TYPE and OPTIONS as argument, +# after having temporarily changed the current working directory to +# BUILDDIR. + +proc target_compile_ada_from_dir {builddir source dest type options} { + set saved_cwd [pwd] + catch { + cd $builddir + return [target_compile $source $dest $type $options] + } result options + cd $saved_cwd + return -options $options $result +} + # Compile some Ada code. proc gdb_compile_ada {source dest type options} { @@ -21,12 +35,17 @@ proc gdb_compile_ada {source dest type options} { set gprdir [file dirname $srcdir] set objdir [file dirname $dest] + # Although strictly not necessary, we force the recompilation + # of all units (additional_flags=-f). This is what is done + # when using GCC to build programs in the other languages, + # and it avoids using a stray objfile file from a long-past + # run, for instance. append options " ada" - append options " additional_flags=-P$gprdir/gnat_ada" - append options " additional_flags=-XSRC=[file tail $srcdir]" - append options " additional_flags=-XOBJ=$objdir" + append options " additional_flags=-f" + append options " additional_flags=-I$srcdir" - set result [target_compile [file tail $source] $dest $type $options] + set result [target_compile_ada_from_dir \ + $objdir [file tail $source] $dest $type $options]] # The Ada build always produces some output, even when the build # succeeds. Thus, we can not use the output the same way we do in -- 2.1.4 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-12-24 5:30 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-22 15:33 [RFC/commmit] [testsuite/Ada] stop using project files when building test programs Joel Brobecker 2015-12-23 13:21 ` Pedro Alves 2015-12-23 13:35 ` Joel Brobecker 2015-12-23 14:43 ` Joel Brobecker 2015-12-23 15:21 ` Pedro Alves 2015-12-23 15:30 ` Pedro Alves 2015-12-24 5:30 ` Joel Brobecker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox