You are viewing a plain text version of this content. The canonical link for it is here.
Posted to modperl@perl.apache.org by Dominique Quatravaux <do...@idealx.com> on 2005/03/14 16:12:02 UTC

[BUG] line numbering off-by-one with Apache::DB and RegistryCooker

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Because ModPerl::RegistryCooker->rewrite_shebang() adds a line on top
of the file in order to honor e.g. the "-w" in "#!/usr/bin/perl -w",
the debugger loses count.

The (trivial) patch below against SVN trunk fixes that (the "use
warnings; " statement now gets added on the same line).

And yes, it means that I succeeded in running Apache::DB with mod_perl
2, even without -X! (It was just a stupid mistake on my part, actually
Apache::DB works like a charm out-of-the-box with a preforked,
multiprocess Apache).

===================================================================
- --- ModPerl-Registry/lib/ModPerl/RegistryCooker.pm      (revision 157426)
+++ ModPerl-Registry/lib/ModPerl/RegistryCooker.pm      (working copy)
@@ -556,7 +556,9 @@
~ # func: rewrite_shebang
~ # dflt: rewrite_shebang
~ # desc: parse the shebang line and convert command line switches
- -#       (defined in %switches) into a perl code.
+#       (defined in %switches) into a perl code. This routine must
+#       not shange the number of lines in the script, lest the #line
+#       statement be off-by-one in convert_script_to_compiled_handler().
~ # args: $self - registry blessed object
~ # rtrn: nothing
~ # efct: the CODE field gets adjusted
@@ -569,7 +571,7 @@
~              unless ${^TAINT};
~        "";
~    },
- -   'w' => sub { "use warnings;\n" },
+   'w' => sub { "use warnings; " },
~ );

~ sub rewrite_shebang {

- --
Dominique QUATRAVAUX                           Ingénieur senior
01 44 42 00 08                                 IDEALX

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.5 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFCNanCMJAKAU3mjcsRAkRkAKCTJTdaS4zlHEX28fjDLmJaUwFygQCfeB4F
k3Sew3r3ne1iHfwxt7/YFhk=
=2YHl
-----END PGP SIGNATURE-----



Re: [BUG] line numbering off-by-one with Apache::DB and RegistryCooker

Posted by Stas Bekman <st...@stason.org>.
Dominique Quatravaux wrote:
> I wrote:
> 
> 
>>Stop the press! My patch was incomplete:
> 
> 
> ... and this one was incomplete too, I forgot about the POD. It looks
> like I am doomed to make a fool of myself in every conceivable way on
> this list :-( Apologies yet again.
[...]

That's cool, it wasn't committed yet :) Thanks for the complete patch, 
Dominique!

> -sub rewrite_shebang {
> +sub shebang_to_perl {
>      my $self = shift;
>      my($line) = ${ $self->{CODE} } =~ /^(.*)$/m;
>      my @cmdline = split /\s+/, $line;

also needed to add:

-    return unless @cmdline;
-    return unless shift(@cmdline) =~ /^\#!/;
+    return "" unless @cmdline;
+    return "" unless shift(@cmdline) =~ /^\#!/;

and please avoid using tabs, I've removed those (we use ident 4) ;)

It's now comitted.

Thanks again, Dominique!

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

Re: [BUG] line numbering off-by-one with Apache::DB and RegistryCooker

Posted by Dominique Quatravaux <do...@idealx.com>.
I wrote:

> Stop the press! My patch was incomplete:

... and this one was incomplete too, I forgot about the POD. It looks
like I am doomed to make a fool of myself in every conceivable way on
this list :-( Apologies yet again.

Index: ModPerl-Registry/lib/ModPerl/PerlRun.pm
===================================================================
--- ModPerl-Registry/lib/ModPerl/PerlRun.pm	(revision 158046)
+++ ModPerl-Registry/lib/ModPerl/PerlRun.pm	(working copy)
@@ -50,7 +50,7 @@
     cache_table     => 'cache_table_common',
     cache_it        => 'NOP',
     read_script     => 'read_script',
-    rewrite_shebang => 'rewrite_shebang',
+    shebang_to_perl => 'shebang_to_perl',
     get_script_name => 'get_script_name',
     chdir_file      => 'chdir_file_normal',
     get_mark_line   => 'get_mark_line',
Index: ModPerl-Registry/lib/ModPerl/Registry.pm
===================================================================
--- ModPerl-Registry/lib/ModPerl/Registry.pm	(revision 158046)
+++ ModPerl-Registry/lib/ModPerl/Registry.pm	(working copy)
@@ -50,7 +50,7 @@
     cache_table     => 'cache_table_common',
     cache_it        => 'cache_it',
     read_script     => 'read_script',
-    rewrite_shebang => 'rewrite_shebang',
+    shebang_to_perl => 'shebang_to_perl',
     get_script_name => 'get_script_name',
     chdir_file      => 'chdir_file_normal',
     get_mark_line   => 'get_mark_line',
Index: ModPerl-Registry/lib/ModPerl/RegistryCooker.pm
===================================================================
--- ModPerl-Registry/lib/ModPerl/RegistryCooker.pm	(revision 158046)
+++ ModPerl-Registry/lib/ModPerl/RegistryCooker.pm	(working copy)
@@ -371,7 +371,7 @@
     return $rc unless $rc == Apache::OK;
 
     # convert the shebang line opts into perl code
-    $self->rewrite_shebang;
+	my $shebang = $self->shebang_to_perl;
 
     # mod_cgi compat, should compile the code while in its dir, so
     # relative require/open will work.
@@ -397,6 +397,7 @@
                     "sub handler {",
                     "local \$0 = '$script_name';",
                     $nph,
+                    $shebang,
                     $line,
                     ${ $self->{CODE} },
                     "\n}"; # last line comment without newline?
@@ -553,13 +554,13 @@
 }
 
 #########################################################################
-# func: rewrite_shebang
-# dflt: rewrite_shebang
+# func: shebang_to_perl
+# dflt: shebang_to_perl
 # desc: parse the shebang line and convert command line switches
 #       (defined in %switches) into a perl code.
 # args: $self - registry blessed object
-# rtrn: nothing
-# efct: the CODE field gets adjusted
+# rtrn: a Perl snippet to be put at the beginning of the CODE field
+#       by caller
 #########################################################################
 
 my %switches = (
@@ -572,7 +573,7 @@
    'w' => sub { "use warnings;\n" },
 );
 
-sub rewrite_shebang {
+sub shebang_to_perl {
     my $self = shift;
     my($line) = ${ $self->{CODE} } =~ /^(.*)$/m;
     my @cmdline = split /\s+/, $line;
@@ -588,7 +589,8 @@
             $prepend .= $switches{$_}->();
         }
     }
-    ${ $self->{CODE} } =~ s/^/$prepend/ if $prepend;
+
+	return $prepend;
 }
 
 #########################################################################
Index: docs/api/ModPerl/RegistryCooker.pod
===================================================================
--- docs/api/ModPerl/RegistryCooker.pod	(revision 158046)
+++ docs/api/ModPerl/RegistryCooker.pod	(working copy)
@@ -120,9 +120,9 @@
 
 default: read_script()
 
-=item * rewrite_shebang()
+=item * shebang_to_perl()
 
-default: rewrite_shebang()
+default: shebang_to_perl()
 
 =item * get_script_name()
 


-- 
Dominique QUATRAVAUX                           Ingénieur senior
01 44 42 00 08                                 IDEALX


Re: [BUG] line numbering off-by-one with Apache::DB and RegistryCooker

Posted by Dominique Quatravaux <do...@idealx.com>.
I wrote:

>The other problem is that I cannot Do The Right Thing while keeping
>the current API intact. I propose the attached patch as an alternative,

Stop the press! My patch was incomplete: I forgot to rename
"rewrite_shebang()" into "shebang_to_perl()" in subclasses
ModPerl::Registry and ModPerl::PerlRun. Here is an updated patch.

Index: ModPerl-Registry/lib/ModPerl/PerlRun.pm
===================================================================
--- ModPerl-Registry/lib/ModPerl/PerlRun.pm	(revision 158046)
+++ ModPerl-Registry/lib/ModPerl/PerlRun.pm	(working copy)
@@ -50,7 +50,7 @@
     cache_table     => 'cache_table_common',
     cache_it        => 'NOP',
     read_script     => 'read_script',
-    rewrite_shebang => 'rewrite_shebang',
+    shebang_to_perl => 'shebang_to_perl',
     get_script_name => 'get_script_name',
     chdir_file      => 'chdir_file_normal',
     get_mark_line   => 'get_mark_line',
Index: ModPerl-Registry/lib/ModPerl/Registry.pm
===================================================================
--- ModPerl-Registry/lib/ModPerl/Registry.pm	(revision 158046)
+++ ModPerl-Registry/lib/ModPerl/Registry.pm	(working copy)
@@ -50,7 +50,7 @@
     cache_table     => 'cache_table_common',
     cache_it        => 'cache_it',
     read_script     => 'read_script',
-    rewrite_shebang => 'rewrite_shebang',
+    shebang_to_perl => 'shebang_to_perl',
     get_script_name => 'get_script_name',
     chdir_file      => 'chdir_file_normal',
     get_mark_line   => 'get_mark_line',
Index: ModPerl-Registry/lib/ModPerl/RegistryCooker.pm
===================================================================
--- ModPerl-Registry/lib/ModPerl/RegistryCooker.pm	(revision 158046)
+++ ModPerl-Registry/lib/ModPerl/RegistryCooker.pm	(working copy)
@@ -371,7 +371,7 @@
     return $rc unless $rc == Apache::OK;
 
     # convert the shebang line opts into perl code
-    $self->rewrite_shebang;
+	my $shebang = $self->shebang_to_perl;
 
     # mod_cgi compat, should compile the code while in its dir, so
     # relative require/open will work.
@@ -397,6 +397,7 @@
                     "sub handler {",
                     "local \$0 = '$script_name';",
                     $nph,
+                    $shebang,
                     $line,
                     ${ $self->{CODE} },
                     "\n}"; # last line comment without newline?
@@ -553,13 +554,13 @@
 }
 
 #########################################################################
-# func: rewrite_shebang
-# dflt: rewrite_shebang
+# func: shebang_to_perl
+# dflt: shebang_to_perl
 # desc: parse the shebang line and convert command line switches
 #       (defined in %switches) into a perl code.
 # args: $self - registry blessed object
-# rtrn: nothing
-# efct: the CODE field gets adjusted
+# rtrn: a Perl snippet to be put at the beginning of the CODE field
+#       by caller
 #########################################################################
 
 my %switches = (
@@ -572,7 +573,7 @@
    'w' => sub { "use warnings;\n" },
 );
 
-sub rewrite_shebang {
+sub shebang_to_perl {
     my $self = shift;
     my($line) = ${ $self->{CODE} } =~ /^(.*)$/m;
     my @cmdline = split /\s+/, $line;
@@ -588,7 +589,8 @@
             $prepend .= $switches{$_}->();
         }
     }
-    ${ $self->{CODE} } =~ s/^/$prepend/ if $prepend;
+
+	return $prepend;
 }
 
 #########################################################################


-- 
Dominique QUATRAVAUX                           Ingénieur senior
01 44 42 00 08                                 IDEALX


Re: [BUG] line numbering off-by-one with Apache::DB and RegistryCooker

Posted by Stas Bekman <st...@stason.org>.
Dominique Quatravaux wrote:
>>In which case you will end up with use warnings on the same line as the 
>>shebang line, no? which will look strange in the debugger
> 
> 
> Actually I find this pretty idiomatic, look:
> 
>    use warnings; #!/usr/bin/perl -w
> 
> No? :-) 

It may confuse some users who may claim that mod_perl has hijacked and 
rewritten their scripts :)

> Besides this line is seldom debugged anyway (at least in my
> experience) because the programmer typically sets a breakpoint with
> "$DB::single=1;" directly in his/her CGI.
> 
> The other problem is that I cannot Do The Right Thing while keeping
> the current API intact. I propose the attached patch as an alternative,
> but it rips the ModPerl::RegistryCooker->rewrite_shebang() method
> apart (hopefully nobody depends on it yet, as it is undocumented).

As 2.0 is not released yet we can still do minor changes. I doubt anybody 
has an overriden version of this particular API.

>>I wonder if it'd be cleaner to use the '#line number filename'
>>directive. 
> 
> 
> Actually this is what is currently done, but *before*
> ->rewrite_shebang() fiddles with the first line, which results in an
> off-by-one.

I like your solution, Dominique, since it's even faster than the current 
one as it avoids the:

   ${ $self->{CODE} } =~ s/^/$prepend/ if $prepend;

which forces a memory re-allocation to prepend a string (which can be 
pretty big).

Unless someone disagrees we will go with this change.

> Index: ModPerl-Registry/lib/ModPerl/RegistryCooker.pm
> ===================================================================
> --- ModPerl-Registry/lib/ModPerl/RegistryCooker.pm	(revision 157426)
> +++ ModPerl-Registry/lib/ModPerl/RegistryCooker.pm	(working copy)
> @@ -371,7 +371,7 @@
>      return $rc unless $rc == Apache::OK;
>  
>      # convert the shebang line opts into perl code
> -    $self->rewrite_shebang;
> +    my $shebang = $self->shebang_to_perl;
>  
>      # mod_cgi compat, should compile the code while in its dir, so
>      # relative require/open will work.
> @@ -397,6 +397,7 @@
>                      "sub handler {",
>                      "local \$0 = '$script_name';",
>                      $nph,
> +                    $shebang,
>                      $line,
>                      ${ $self->{CODE} },
>                      "\n}"; # last line comment without newline?
> @@ -553,13 +554,13 @@
>  }
>  
>  #########################################################################
> -# func: rewrite_shebang
> -# dflt: rewrite_shebang
> +# func: shebang_to_perl
> +# dflt: shebang_to_perl
>  # desc: parse the shebang line and convert command line switches
>  #       (defined in %switches) into a perl code.
>  # args: $self - registry blessed object
> -# rtrn: nothing
> -# efct: the CODE field gets adjusted
> +# rtrn: a Perl snippet to be put at the beginning of the CODE field
> +#       by caller
>  #########################################################################
>  
>  my %switches = (
> @@ -572,7 +573,7 @@
>     'w' => sub { "use warnings;\n" },
>  );
>  
> -sub rewrite_shebang {
> +sub shebang_to_perl {
>      my $self = shift;
>      my($line) = ${ $self->{CODE} } =~ /^(.*)$/m;
>      my @cmdline = split /\s+/, $line;
> @@ -588,7 +589,8 @@
>              $prepend .= $switches{$_}->();
>          }
>      }
> -    ${ $self->{CODE} } =~ s/^/$prepend/ if $prepend;
> +
> +    return $prepend;
>  }
>  
>  #########################################################################
> 
> 


-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

Re: [BUG] line numbering off-by-one with Apache::DB and RegistryCooker

Posted by Dominique Quatravaux <do...@idealx.com>.
> In which case you will end up with use warnings on the same line as the 
> shebang line, no? which will look strange in the debugger

Actually I find this pretty idiomatic, look:

   use warnings; #!/usr/bin/perl -w

No? :-) Besides this line is seldom debugged anyway (at least in my
experience) because the programmer typically sets a breakpoint with
"$DB::single=1;" directly in his/her CGI.

The other problem is that I cannot Do The Right Thing while keeping
the current API intact. I propose the attached patch as an alternative,
but it rips the ModPerl::RegistryCooker->rewrite_shebang() method
apart (hopefully nobody depends on it yet, as it is undocumented).

> I wonder if it'd be cleaner to use the '#line number filename'
> directive. 

Actually this is what is currently done, but *before*
->rewrite_shebang() fiddles with the first line, which results in an
off-by-one.

Index: ModPerl-Registry/lib/ModPerl/RegistryCooker.pm
===================================================================
--- ModPerl-Registry/lib/ModPerl/RegistryCooker.pm	(revision 157426)
+++ ModPerl-Registry/lib/ModPerl/RegistryCooker.pm	(working copy)
@@ -371,7 +371,7 @@
     return $rc unless $rc == Apache::OK;
 
     # convert the shebang line opts into perl code
-    $self->rewrite_shebang;
+    my $shebang = $self->shebang_to_perl;
 
     # mod_cgi compat, should compile the code while in its dir, so
     # relative require/open will work.
@@ -397,6 +397,7 @@
                     "sub handler {",
                     "local \$0 = '$script_name';",
                     $nph,
+                    $shebang,
                     $line,
                     ${ $self->{CODE} },
                     "\n}"; # last line comment without newline?
@@ -553,13 +554,13 @@
 }
 
 #########################################################################
-# func: rewrite_shebang
-# dflt: rewrite_shebang
+# func: shebang_to_perl
+# dflt: shebang_to_perl
 # desc: parse the shebang line and convert command line switches
 #       (defined in %switches) into a perl code.
 # args: $self - registry blessed object
-# rtrn: nothing
-# efct: the CODE field gets adjusted
+# rtrn: a Perl snippet to be put at the beginning of the CODE field
+#       by caller
 #########################################################################
 
 my %switches = (
@@ -572,7 +573,7 @@
    'w' => sub { "use warnings;\n" },
 );
 
-sub rewrite_shebang {
+sub shebang_to_perl {
     my $self = shift;
     my($line) = ${ $self->{CODE} } =~ /^(.*)$/m;
     my @cmdline = split /\s+/, $line;
@@ -588,7 +589,8 @@
             $prepend .= $switches{$_}->();
         }
     }
-    ${ $self->{CODE} } =~ s/^/$prepend/ if $prepend;
+
+    return $prepend;
 }
 
 #########################################################################


-- 
Dominique QUATRAVAUX                           Ingénieur senior
01 44 42 00 08                                 IDEALX


Re: [BUG] line numbering off-by-one with Apache::DB and RegistryCooker

Posted by Stas Bekman <st...@stason.org>.
Dominique Quatravaux wrote:

> Actually it was one, but Thunderbird munged it :-(. Retrying with
> mutt... Sorry for yet another blunder.
[...]
> -   'w' => sub { "use warnings;\n" },
> +   'w' => sub { "use warnings; " },
>  );
>  
>  sub rewrite_shebang {

In which case you will end up with use warnings on the same line as the 
shebang line, no? which will look strange in the debugger

I wonder if it'd be cleaner to use the '#line number filename' directive. 
mp1 had it as an optional feature enabled by $Apache::Registry::MarkLine

	    my $line = $Apache::Registry::MarkLine ?
		"\n#line 1 $filename\n" : "";

the added value is that you tell the compiler/debugger, not only the 
correct line, but also the filename, so the debugger should be able to 
jump to that file. I'm not sure though why it was never put into the main 
stream. May be it didn't quite work?

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

Re: [BUG] line numbering off-by-one with Apache::DB and RegistryCooker

Posted by Dominique Quatravaux <do...@idealx.com>.
> Thanks, Dominique. But care to post a unified patch? 'diff -u'
> Thanks!

Actually it was one, but Thunderbird munged it :-(. Retrying with
mutt... Sorry for yet another blunder.

Index: ModPerl-Registry/lib/ModPerl/RegistryCooker.pm
===================================================================
--- ModPerl-Registry/lib/ModPerl/RegistryCooker.pm	(revision 157426)
+++ ModPerl-Registry/lib/ModPerl/RegistryCooker.pm	(working copy)
@@ -556,7 +556,9 @@
 # func: rewrite_shebang
 # dflt: rewrite_shebang
 # desc: parse the shebang line and convert command line switches
-#       (defined in %switches) into a perl code.
+#       (defined in %switches) into a perl code. This routine must
+#       not shange the number of lines in the script, lest the #line
+#       statement be off-by-one in convert_script_to_compiled_handler().
 # args: $self - registry blessed object
 # rtrn: nothing
 # efct: the CODE field gets adjusted
@@ -569,7 +571,7 @@
              unless ${^TAINT};
        "";
    },
-   'w' => sub { "use warnings;\n" },
+   'w' => sub { "use warnings; " },
 );
 
 sub rewrite_shebang {


-- 
Dominique QUATRAVAUX                           Ingénieur senior
01 44 42 00 08                                 IDEALX


Re: [BUG] line numbering off-by-one with Apache::DB and RegistryCooker

Posted by Stas Bekman <st...@stason.org>.
Dominique Quatravaux wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Because ModPerl::RegistryCooker->rewrite_shebang() adds a line on top
> of the file in order to honor e.g. the "-w" in "#!/usr/bin/perl -w",
> the debugger loses count.
> 
> The (trivial) patch below against SVN trunk fixes that (the "use
> warnings; " statement now gets added on the same line).
> 
> And yes, it means that I succeeded in running Apache::DB with mod_perl
> 2, even without -X! (It was just a stupid mistake on my part, actually
> Apache::DB works like a charm out-of-the-box with a preforked,
> multiprocess Apache).
> 
> ===================================================================
> - --- ModPerl-Registry/lib/ModPerl/RegistryCooker.pm      (revision 157426)
> +++ ModPerl-Registry/lib/ModPerl/RegistryCooker.pm      (working copy)
> @@ -556,7 +556,9 @@
> ~ # func: rewrite_shebang
> ~ # dflt: rewrite_shebang
> ~ # desc: parse the shebang line and convert command line switches
> - -#       (defined in %switches) into a perl code.
> +#       (defined in %switches) into a perl code. This routine must
> +#       not shange the number of lines in the script, lest the #line
> +#       statement be off-by-one in convert_script_to_compiled_handler().
> ~ # args: $self - registry blessed object
> ~ # rtrn: nothing
> ~ # efct: the CODE field gets adjusted
> @@ -569,7 +571,7 @@
> ~              unless ${^TAINT};
> ~        "";
> ~    },
> - -   'w' => sub { "use warnings;\n" },
> +   'w' => sub { "use warnings; " },
> ~ );

Thanks, Dominique. But care to post a unified patch? 'diff -u' Thanks!

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com