[V2] Testsuite: Ensure changing directory does not break the log file
Commit Message
[ Reposting this as a V2 with all the changes, as wasn't sure if I had a full
ok on it ]
get_compiler_info switches to a new log file before checking the compiler
to ensure the checks are not logged. Afterwards it restores back to using
the original log file. However, the logfile uses a relative path name -
if the current test has changed the current directory then all further
output for the test will be lost. This can confuse the code that collates
the main gdb.log file at the end of a FORCE_PARALLEL run.
fullpath-expand.exp calls gdb_compile after changing the current directory.
The "Ensure stack protection is off for GCC" patch added a call to
get_compiler_info from inside of gdb_compile, causing log file collection
to break for FORCE_PARALLEL runs.
The ideal solution would be to ensure the log file is always created using
an absolute path name. However, this is set at multiple points in
Makefile.in and in some instances just relies on dejagnu common code to set
the log file directory to "."
The simpler and safer solution is to override the builtin cd function. The
new function checks the current log file and if the path is relative, then
it resets the logging using an absolute path. Finally it calls the builtin
cd. This ensures get_compiler_info (and any other code) can correctly
backup and restore the current log file.
gdb/testsuite/ChangeLog:
2019-02-26 Alan Hayward <alan.hayward@arm.com>
* lib/gdb.exp (builtin_cd): rename of cd.
(cd): Override builtin.
---
gdb/testsuite/lib/gdb.exp | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
Comments
Ping. I think this got lost in all the 8.3 chatter.
It’d probably be useful to get this into 8.3 too.
Alan.
> On 26 Feb 2019, at 09:59, Alan Hayward <Alan.Hayward@arm.com> wrote:
>
> [ Reposting this as a V2 with all the changes, as wasn't sure if I had a full
> ok on it ]
>
> get_compiler_info switches to a new log file before checking the compiler
> to ensure the checks are not logged. Afterwards it restores back to using
> the original log file. However, the logfile uses a relative path name -
> if the current test has changed the current directory then all further
> output for the test will be lost. This can confuse the code that collates
> the main gdb.log file at the end of a FORCE_PARALLEL run.
>
> fullpath-expand.exp calls gdb_compile after changing the current directory.
>
> The "Ensure stack protection is off for GCC" patch added a call to
> get_compiler_info from inside of gdb_compile, causing log file collection
> to break for FORCE_PARALLEL runs.
>
> The ideal solution would be to ensure the log file is always created using
> an absolute path name. However, this is set at multiple points in
> Makefile.in and in some instances just relies on dejagnu common code to set
> the log file directory to "."
>
> The simpler and safer solution is to override the builtin cd function. The
> new function checks the current log file and if the path is relative, then
> it resets the logging using an absolute path. Finally it calls the builtin
> cd. This ensures get_compiler_info (and any other code) can correctly
> backup and restore the current log file.
>
> gdb/testsuite/ChangeLog:
>
> 2019-02-26 Alan Hayward <alan.hayward@arm.com>
>
> * lib/gdb.exp (builtin_cd): rename of cd.
> (cd): Override builtin.
> ---
> gdb/testsuite/lib/gdb.exp | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index d05854329d..e429dcaf19 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -6284,5 +6284,41 @@ proc gdb_define_cmd {command command_list} {
> }
> }
>
> +# Override the 'cd' builtin with a version that ensures that the
> +# log file keeps pointing at the same file. We need this because
> +# unfortunately the path to the log file is recorded using an
> +# relative path name, and, we sometimes need to close/reopen the log
> +# after changing the current directory. See get_compiler_info.
> +
> +rename cd builtin_cd
> +
> +proc cd { dir } {
> +
> + # Get the existing log file flags.
> + set log_file_info [log_file -info]
> +
> + # Split the flags into args and file name.
> + set log_file_flags ""
> + set log_file_file ""
> + foreach arg [ split "$log_file_info" " "] {
> + if [string match "-*" $arg] {
> + lappend log_file_flags $arg
> + } else {
> + lappend log_file_file $arg
> + }
> + }
> +
> + # If there was an existing file, ensure it is an absolute path, and then
> + # reset logging.
> + if { $log_file_file != "" } {
> + set log_file_file [file normalize $log_file_file]
> + log_file
> + log_file $log_file_flags "$log_file_file"
> + }
> +
> + # Call the builtin version of cd.
> + builtin_cd $dir
> +}
> +
> # Always load compatibility stuff.
> load_lib future.exp
> --
> 2.17.2 (Apple Git-113)
>
>>>>> "Alan" == Alan Hayward <Alan.Hayward@arm.com> writes:
Alan> Ping. I think this got lost in all the 8.3 chatter.
Alan> It’d probably be useful to get this into 8.3 too.
Thanks, this is ok.
I agree it should go on the 8.3 branch as well.
thanks,
Tom
@@ -6284,5 +6284,41 @@ proc gdb_define_cmd {command command_list} {
}
}
+# Override the 'cd' builtin with a version that ensures that the
+# log file keeps pointing at the same file. We need this because
+# unfortunately the path to the log file is recorded using an
+# relative path name, and, we sometimes need to close/reopen the log
+# after changing the current directory. See get_compiler_info.
+
+rename cd builtin_cd
+
+proc cd { dir } {
+
+ # Get the existing log file flags.
+ set log_file_info [log_file -info]
+
+ # Split the flags into args and file name.
+ set log_file_flags ""
+ set log_file_file ""
+ foreach arg [ split "$log_file_info" " "] {
+ if [string match "-*" $arg] {
+ lappend log_file_flags $arg
+ } else {
+ lappend log_file_file $arg
+ }
+ }
+
+ # If there was an existing file, ensure it is an absolute path, and then
+ # reset logging.
+ if { $log_file_file != "" } {
+ set log_file_file [file normalize $log_file_file]
+ log_file
+ log_file $log_file_flags "$log_file_file"
+ }
+
+ # Call the builtin version of cd.
+ builtin_cd $dir
+}
+
# Always load compatibility stuff.
load_lib future.exp