Closed p5pRT closed 12 years ago
The attached patch adds to the debugger a capability I thought about ages ago and which turned out to be absurdly easy to implement. It adds an optional parameter to the t(race) command\, a maximum number of stack frames to trace below the current one:\, e.g.:
t 3 - turn tracing on\, trace up to 3 levels below current depth\, be silent below that t 2 fnord() - trace up to 2 levels deep in execution of fnord()
Since it is backwards compatible I added it to the legacy command set as well\, but that's certainly debatable.
Preferably this patch should include some regression tests - add them to lib/perl5db.t .
Regards\,
â Shlomi Fish
The RT System itself - Status changed from 'new' to 'open'
On 11/21/2011 7:45 AM\, Shlomi Fish via RT wrote:
Preferably this patch should include some regression tests - add them to lib/perl5db.t .
Thanks\, I took a closer look at that .t and found this process is easier than it used to be. Two patches attached - an incremental one\, and the combined patch. One of those work for you?
On Mon\, 21 Nov 2011 10:27:41 -0800 Peter Scott \Peter@​PSDT\.com wrote:
On 11/21/2011 7:45 AM\, Shlomi Fish via RT wrote:
Preferably this patch should include some regression tests - add them to lib/perl5db.t .
Thanks\, I took a closer look at that .t and found this process is easier than it used to be. Two patches attached - an incremental one\, and the combined patch. One of those work for you?
Well\, patch.combined does not apply correctly against bleadperl (at least not for me):
[SHELL]
shlomif@telaviv1:\~/Download/unpack/perl/p5/git/perl$ patch -p0 \< ~/patch.combined patching file lib/perl5db.pl Hunk #1 succeeded at 2115 (offset 7 lines). Hunk #2 succeeded at 2143 (offset 7 lines). Hunk #3 succeeded at 2343 (offset 7 lines). Hunk #4 succeeded at 3460 (offset 39 lines). Hunk #5 succeeded at 7333 (offset 114 lines). Hunk #6 succeeded at 7474 (offset 114 lines). Hunk #7 succeeded at 7525 (offset 114 lines). Hunk #8 succeeded at 7649 (offset 114 lines). patching file pod/perldebug.pod Hunk #1 succeeded at 269 (offset -1 lines). patching file lib/perl5db.t Hunk #1 FAILED at 27. Hunk #2 succeeded at 210 (offset 43 lines). 1 out of 2 hunks FAILED -- saving rejects to file lib/perl5db.t.rej patching file lib/perl5db/t/rt-104168 shlomif@telaviv1:\~/Download/unpack/perl/p5/git/perl$ git branch * blead perlipc-revamp perlipc-revamp-take2 shlomif-xs-pod-work
[/SHELL]
Otherwise\, here are a few notes about it:
} else { - print_lineinfo($position); + print_lineinfo($position) if $stack_depth \< $trace_to_depth; }
# Scan forward\, stopping at either the end or the next @@ -2136\,7 +2136\,7 @@ "$i:\t$dbline[$i]$after" ); } else { - print_lineinfo($incr_pos); + print_lineinfo($incr_pos) if $stack_depth \< $trace_to_depth; }
There's some duplicate functionality here. You should extract a subroutine or a closure or whatever to encapsulate it:
sub depth_print_lineinfo { my ($contents) = @_; if ($stack_depth \< $trace_to_depth) { print_lineinfo($contents); } return; }
Also\, the variable $trace_to_depth appeared out of the blue. Is it a global package-scope variable? I would suggest declaring it\, even at least with "use vars(...)".
- $cmd =~ /^t$/ && do { + $cmd =~ /^t(?:\s+(\d+))?$/ && do { $trace ^= 1; local $\ = ''; + $trace_to_depth = $1 ? $stack_depth + $1 : 1E9; print $OUT "Trace = " - . ( ( $trace & 1 ) ? "on" : "off" ) . "\n"; + . ( ( $trace & 1 ) + ? ( $1 ? "on (to level $trace_to_depth)" : "on" ) + : "off" ) . "\n"; next CMD; };
The $trace_to_depth is out of indentation. Please check appropriate leading tabs and spaces.
Plus\, you shouldn't use $1 in the "print $OUT" statement. Set a lexical variable flag based on its presence and/or value and then use it.
- $cmd =~ s/^t\s/\$DB::trace |= 1;\n/; + $cmd =~ s/^t\s+(\d+)?/\$DB::trace |= 1;\n/ && do { + $trace_to_depth = $1 ? $stack_depth||0 + $1 : 1E9; + };
Again mixed indentation.
Otherwise looks fine.
Regards\,
Shlomi Fish
--
Shlomi Fish http://www.shlomifish.org/ Freecell Solver - http://fc-solve.shlomifish.org/
In Soviet Russia\, every time you kill a kitten\, God masturbates. â http://linux.slashdot.org/comments.pl?sid=195378&cid=16009070
Please reply to list if it's a mailing list post - http://shlom.in/reply .
Begin forwarded message:
Date: Tue\, 22 Nov 2011 11:01:09 +0200 From: Shlomi Fish \shlomif@​shlomifish\.org To: Peter Scott \Peter@​PSDT\.com Cc: perlbug-followup@perl.org Subject: Re: [perl #104168] Debugger patch - trace to max depth
On Mon\, 21 Nov 2011 10:27:41 -0800 Peter Scott \Peter@​PSDT\.com wrote:
On 11/21/2011 7:45 AM\, Shlomi Fish via RT wrote:
Preferably this patch should include some regression tests - add them to lib/perl5db.t .
Thanks\, I took a closer look at that .t and found this process is easier than it used to be. Two patches attached - an incremental one\, and the combined patch. One of those work for you?
Well\, patch.combined does not apply correctly against bleadperl (at least not for me):
[SHELL]
shlomif@telaviv1:\~/Download/unpack/perl/p5/git/perl$ patch -p0 \< ~/patch.combined patching file lib/perl5db.pl Hunk #1 succeeded at 2115 (offset 7 lines). Hunk #2 succeeded at 2143 (offset 7 lines). Hunk #3 succeeded at 2343 (offset 7 lines). Hunk #4 succeeded at 3460 (offset 39 lines). Hunk #5 succeeded at 7333 (offset 114 lines). Hunk #6 succeeded at 7474 (offset 114 lines). Hunk #7 succeeded at 7525 (offset 114 lines). Hunk #8 succeeded at 7649 (offset 114 lines). patching file pod/perldebug.pod Hunk #1 succeeded at 269 (offset -1 lines). patching file lib/perl5db.t Hunk #1 FAILED at 27. Hunk #2 succeeded at 210 (offset 43 lines). 1 out of 2 hunks FAILED -- saving rejects to file lib/perl5db.t.rej patching file lib/perl5db/t/rt-104168 shlomif@telaviv1:\~/Download/unpack/perl/p5/git/perl$ git branch * blead perlipc-revamp perlipc-revamp-take2 shlomif-xs-pod-work
[/SHELL]
Otherwise\, here are a few notes about it:
} else { - print_lineinfo($position); + print_lineinfo($position) if $stack_depth \< $trace_to_depth; }
# Scan forward\, stopping at either the end or the next @@ -2136\,7 +2136\,7 @@ "$i:\t$dbline[$i]$after" ); } else { - print_lineinfo($incr_pos); + print_lineinfo($incr_pos) if $stack_depth \< $trace_to_depth; }
There's some duplicate functionality here. You should extract a subroutine or a closure or whatever to encapsulate it:
sub depth_print_lineinfo { my ($contents) = @_; if ($stack_depth \< $trace_to_depth) { print_lineinfo($contents); } return; }
Also\, the variable $trace_to_depth appeared out of the blue. Is it a global package-scope variable? I would suggest declaring it\, even at least with "use vars(...)".
- $cmd =~ /^t$/ && do { + $cmd =~ /^t(?:\s+(\d+))?$/ && do { $trace ^= 1; local $\ = ''; + $trace_to_depth = $1 ? $stack_depth + $1 : 1E9; print $OUT "Trace = " - . ( ( $trace & 1 ) ? "on" : "off" ) . "\n"; + . ( ( $trace & 1 ) + ? ( $1 ? "on (to level $trace_to_depth)" : "on" ) + : "off" ) . "\n"; next CMD; };
The $trace_to_depth is out of indentation. Please check appropriate leading tabs and spaces.
Plus\, you shouldn't use $1 in the "print $OUT" statement. Set a lexical variable flag based on its presence and/or value and then use it.
- $cmd =~ s/^t\s/\$DB::trace |= 1;\n/; + $cmd =~ s/^t\s+(\d+)?/\$DB::trace |= 1;\n/ && do { + $trace_to_depth = $1 ? $stack_depth||0 + $1 : 1E9; + };
Again mixed indentation.
Otherwise looks fine.
Regards\,
Shlomi Fish
--
Shlomi Fish http://www.shlomifish.org/ Freecell Solver - http://fc-solve.shlomifish.org/
In Soviet Russia\, every time you kill a kitten\, God masturbates. â http://linux.slashdot.org/comments.pl?sid=195378&cid=16009070
Please reply to list if it's a mailing list post - http://shlom.in/reply .
--
Shlomi Fish http://www.shlomifish.org/ Stop Using MSIE - http://www.shlomifish.org/no-ie/
* Backward compatibility is your worst enemy. * Backward compatibility is your usersâ best friend.
Please reply to list if it's a mailing list post - http://shlom.in/reply .
Forwarding to the list\, as this message was sent to me alone.
Regards\,
Shlomi Fish
Begin forwarded message:
Date: Wed\, 23 Nov 2011 13:43:36 -0800 From: Peter Scott \Peter@​PSDT\.com To: Shlomi Fish \shlomif@​shlomifish\.org Subject: Re: [perl #104168] Debugger patch - trace to max depth
Thanks\, Shlomi.
On 11/22/2011 1:01 AM\, Shlomi Fish wrote:
Well\, patch.combined does not apply correctly against bleadperl (at least not for me): It seems someone's been busy on the debugger recently. I downloaded a fresh blead and attach a diff made from it. [/SHELL]
There's some duplicate functionality here. You should extract a subroutine or a closure or whatever to encapsulate it: Good\, done. Also\, the variable $trace_to_depth appeared out of the blue. Is it a global package-scope variable? I would suggest declaring it\, even at least with "use vars(...)". Normally I would\, of course\, but the debugger is a minefield in that respect. There are no 'use vars' statements and no file-scoped lexicals (with one exception that looks like an accident). So I felt that treating this variable differently from all the others (see\, e.g.\, $laststep) would only invite unwarranted confusion and speculation as to what made it different from the rest. In other words\, when in Rome...
The $trace_to_depth is out of indentation. Please check appropriate leading tabs and spaces. Fixed. Plus\, you shouldn't use $1 in the "print $OUT" statement. Set a lexical variable flag based on its presence and/or value and then use it.
Done.
--
Shlomi Fish http://www.shlomifish.org/ Stop Using MSIE - http://www.shlomifish.org/no-ie/
Q2: Busy people are unproductive. We are very productive and so weâre never busy. â Star Trek\, âWe\, the Living Deadâ by Shlomi Fish
Please reply to list if it's a mailing list post - http://shlom.in/reply .
Hi Peter\,
On Thu\, 24 Nov 2011 09:09:37 +0200 Shlomi Fish \shlomif@​shlomifish\.org wrote:
Thanks\, Shlomi.
On 11/22/2011 1:01 AM\, Shlomi Fish wrote:
Well\, patch.combined does not apply correctly against bleadperl (at least not for me): It seems someone's been busy on the debugger recently. I downloaded a fresh blead and attach a diff made from it.
Well\, next time you can use the "git diff" command.
[/SHELL]
There's some duplicate functionality here. You should extract a subroutine or a closure or whatever to encapsulate it: Good\, done. Also\, the variable $trace_to_depth appeared out of the blue. Is it a global package-scope variable? I would suggest declaring it\, even at least with "use vars(...)". Normally I would\, of course\, but the debugger is a minefield in that respect. There are no 'use vars' statements and no file-scoped lexicals (with one exception that looks like an accident). So I felt that treating this variable differently from all the others (see\, e.g.\, $laststep) would only invite unwarranted confusion and speculation as to what made it different from the rest. In other words\, when in Rome...
OK.
The $trace_to_depth is out of indentation. Please check appropriate leading tabs and spaces. Fixed.
OK.
Plus\, you shouldn't use $1 in the "print $OUT" statement. Set a lexical variable flag based on its presence and/or value and then use it.
Done.
Nice.
Well\, I have tested your patch on a local git branch. All tests pass except for t/porting/manifest.t and after I add the missing file to the "MANIFEST" file\, I'm getting this:
\<\<\< Failed 2 tests out of 2170\, 99.91% okay. porting/filenames.t porting/manifest.t ### Since not all tests were successful\, you may want to run some of ### them individually and examine any diagnostic messages they produce. ### See the INSTALL document's section on "make test". ### You have a good chance to get more information by running ### ./perl harness ### in the 't' directory since most (>=80%) of the tests succeeded. ### You may have to set your dynamic library search path\, ### LD_LIBRARY_PATH\, to point to the build directory: ### setenv LD_LIBRARY_PATH `pwd`; cd t; ./perl harness ### LD_LIBRARY_PATH=`pwd`; export LD_LIBRARY_PATH; cd t; ./perl harness ### export LD_LIBRARY_PATH=`pwd`; cd t; ./perl harness ### for csh-style shells\, like tcsh; or for traditional/modern ### Bourne-style shells\, like bash\, ksh\, and zsh\, respectively. u=4.62 s=1.20 cu=296.52 cs=23.06 scripts=2170 tests=487908 make: *** [test] Error 1
I don't think any of this is your fault\, but I still don't know how to fix it. Is it documented anywhere?
Regards\,
Shlomi Fish
--
Shlomi Fish http://www.shlomifish.org/ http://www.shlomifish.org/humour/ways_to_do_it.html
I invented the term ObjectâOriented\, and I can tell you I did not have C++ in mind. â Alan Kay (Attributed)
Please reply to list if it's a mailing list post - http://shlom.in/reply .
On Wed Nov 23 23:10:19 2011\, shlomif@shlomifish.org wrote:
Date: Wed\, 23 Nov 2011 13:43:36 -0800 From: Peter Scott \Peter@​PSDT\.com To: Shlomi Fish \shlomif@​shlomifish\.org Subject: Re: [perl #104168] Debugger patch - trace to max depth
Thank you. Applied as 611272bb.
--
Father Chrysostomos
@cpansprout - Status changed from 'open' to 'resolved'
On Thu Nov 24 00:01:37 2011\, shlomif@shlomifish.org wrote:
\<\<\< Failed 2 tests out of 2170\, 99.91% okay. porting/filenames.t porting/manifest.t ### Since not all tests were successful\, you may want to run some of ### them individually and examine any diagnostic messages they produce. ### See the INSTALL document's section on "make test". ### You have a good chance to get more information by running ### ./perl harness ### in the 't' directory since most (>=80%) of the tests succeeded. ### You may have to set your dynamic library search path\, ### LD_LIBRARY_PATH\, to point to the build directory: ### setenv LD_LIBRARY_PATH `pwd`; cd t; ./perl harness ### LD_LIBRARY_PATH=`pwd`; export LD_LIBRARY_PATH; cd t; ./perl harness ### export LD_LIBRARY_PATH=`pwd`; cd t; ./perl harness ### for csh-style shells\, like tcsh; or for traditional/modern ### Bourne-style shells\, like bash\, ksh\, and zsh\, respectively. u=4.62 s=1.20 cu=296.52 cs=23.06 scripts=2170 tests=487908 make: *** [test] Error 1
I don't think any of this is your fault\, but I still don't know how to fix it. Is it documented anywhere?
In the test names. :-)
I suspect you used spaces instead of tabs in MANIFEST.
You can get verbose test output with:
cd t; ./perl TEST porting/{filenames\,manifest}.t
It cuts off output on the first failure.
--
Father Chrysostomos
Migrated from rt.perl.org#104168 (status was 'resolved')
Searchable as RT104168$