Bug 63612

Comment on attachment 78102 [details] [review]
Patch to improve Perl and TAP support. Applies against commit 0c41ac8

Review of attachment 78102 [details] [review]:
-----------------------------------------------------------------

::: freedesktop.org.xml.in
@@ +2826,4 @@
>        <match type="string" value='eval \"exec /usr/local/bin/perl' offset="0"/>
>        <match type="string" value="/bin/perl" offset="1:16"/>
>        <match type="string" value="/bin/env perl" offset="1:16"/>
> +      <match type="string" value="#!perl" offset="0"/>

That exists? I've never seen that before.

@@ +2826,5 @@
>        <match type="string" value='eval \"exec /usr/local/bin/perl' offset="0"/>
>        <match type="string" value="/bin/perl" offset="1:16"/>
>        <match type="string" value="/bin/env perl" offset="1:16"/>
> +      <match type="string" value="#!perl" offset="0"/>
> +      <match type="string" value="package " offset="0:256"/>

This is far too generic, so are most of the ones below.

@@ +2849,5 @@
> +    <glob pattern="*.pod"/><!-- documentation -->
> +    <glob pattern="*.t" weight="10"/><!-- CPAN-style Perl test script -->
> +  </mime-type>
> +  <mime-type type="application/x-tap">
> +    <_comment>Test Anything Protocol output</_comment>

You should separate this in another patch. Given the really generic magic, I'm unlikely to take this though.

Created attachment 86830 [details] [review]
Improve Perl detection

I've removed the TAP support bits, and removed the too-generic magic. Putting on NEEDINFO until a patch is provided for the rest.

commit a6bdb6a198f8f9fe4153f72e5b4d10e1b5057c3e
Author: Salve J. Nilsen <sjn-bugs.freedesktop.org@pvv.org>
Date:   Tue Apr 16 17:24:00 2013 +0000

    Improve Perl detection
    
    https://bugs.freedesktop.org/show_bug.cgi?id=63612

See review commends and new bug 105838

Comment on attachment 78102 [details] [review]
Patch to improve Perl and TAP support. Applies against commit 0c41ac8

Review of attachment 78102 [details] [review]:
-----------------------------------------------------------------

::: freedesktop.org.xml.in
@@ +2826,4 @@
>        <match type="string" value='eval \"exec /usr/local/bin/perl' offset="0"/>
>        <match type="string" value="/bin/perl" offset="1:16"/>
>        <match type="string" value="/bin/env perl" offset="1:16"/>
> +      <match type="string" value="#!perl" offset="0"/>

I've seen #!perl on Windows machines.  It's not portable (execve doesn't search PATH), but the ActiveState Perl documentation mentions it for some reason:

https://docs.activestate.com/activeperl/5.26/perl/faq/Windows/ActivePerl-Winfaq4.html#What_s_the_equivalent_of_the_she

@@ +2827,5 @@
>        <match type="string" value="/bin/perl" offset="1:16"/>
>        <match type="string" value="/bin/env perl" offset="1:16"/>
> +      <match type="string" value="#!perl" offset="0"/>
> +      <match type="string" value="package " offset="0:256"/>
> +      <match type="string" value="use strict" offset="0:256"/>

This is also too broad.  It incorrectly matches against ECMAScript's strict-mode syntax, which can look like one of the following lines (including quotes and semicolon):

    "use strict";
    'use strict';

Since this code has already been released, I opened up a separate bug report here:
https://bugs.freedesktop.org/show_bug.cgi?id=105838

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xdg/shared-mime-info/issues/85.