Bug 26658

Created attachment 111466 [details] [review]
Bug 26658: Fix shebangs of bin/intranet and bin/opac

Is this because of Mojo? I have read things like this, so I thought the "env" version was OK:

"Because of this, on modern Linux/Unix systems, people might prefer to use #!/usr/bin/env perl as the hash-bang line."
https://perlmaven.com/hashbang

But I ain't no expert! :-)

(In reply to Magnus Enger from comment #2)
> Is this because of Mojo? I have read things like this, so I thought the
> "env" version was OK:
> 
> "Because of this, on modern Linux/Unix systems, people might prefer to use
> #!/usr/bin/env perl as the hash-bang line."
> https://perlmaven.com/hashbang
> 
> But I ain't no expert! :-)

No it's not related to Mojo. Yes the env version is OK, it's how I write Perl shebangs most of the time. But the important thing is that all shebangs should be the same in the entire Koha code base. That's why it needs to be fixed :)

(In reply to Julian Maurice from comment #3)
> No it's not related to Mojo. Yes the env version is OK, it's how I write
> Perl shebangs most of the time. But the important thing is that all shebangs
> should be the same in the entire Koha code base. That's why it needs to be
> fixed :)

Agreed. I think that the env version allows for more flexibility and power, but I think that we should keep the shebangs consistent in Koha. (Otherwise, we might start up some Perl code using /opt/perl-new/bin/perl, which then invokes other code using /usr/bin/perl.)

That being said... I've finally decided to do a little search for existing '#!/usr/bin/env perl' usage in Koha... and it turns out we have more inconsistencies than I thought.

root@kohadevbox:koha(bug26658)$ grep --exclude-dir=blib -R '#!/usr/bin/env perl' *
api/v1/app.pl:#!/usr/bin/env perl
bin/opac:#!/usr/bin/env perl
bin/intranet:#!/usr/bin/env perl
C4/TTParser.pm:#!/usr/bin/env perl
misc/translator/po2json:#!/usr/bin/env perl
misc/migration_tools/ifla/update.pl:#!/usr/bin/env perl
t/db_dependent/Koha_ExternalContent_OverDrive.t:#!/usr/bin/env perl
t/db_dependent/Koha/Objects/Mixin/AdditionalFields.t:#!/usr/bin/env perl
t/db_dependent/Koha/Template/Plugin/Asset.t:#!/usr/bin/env perl
t/db_dependent/Acquisition/close_reopen_basket.t:#!/usr/bin/env perl
t/db_dependent/Acquisition/populate_order_with_prices.t:#!/usr/bin/env perl
t/db_dependent/api/v1/auth.t:#!/usr/bin/env perl
t/db_dependent/api/v1/auth_authenticate_api_request.t:#!/usr/bin/env perl
t/db_dependent/api/v1/smtp_servers.t:#!/usr/bin/env perl
t/db_dependent/api/v1/items.t:#!/usr/bin/env perl
t/db_dependent/api/v1/return_claims.t:#!/usr/bin/env perl
t/db_dependent/api/v1/holds.t:#!/usr/bin/env perl
t/db_dependent/api/v1/checkouts.t:#!/usr/bin/env perl
t/db_dependent/api/v1/acquisitions_vendors.t:#!/usr/bin/env perl
t/db_dependent/api/v1/auth_basic.t:#!/usr/bin/env perl
t/db_dependent/api/v1/advanced_editor_macros.t:#!/usr/bin/env perl
t/db_dependent/api/v1/oauth.t:#!/usr/bin/env perl
t/db_dependent/api/v1/cities.t:#!/usr/bin/env perl
t/db_dependent/api/v1/libraries.t:#!/usr/bin/env perl
t/db_dependent/api/v1/biblios.t:#!/usr/bin/env perl
t/db_dependent/api/v1/acquisitions_funds.t:#!/usr/bin/env perl
t/db_dependent/api/v1/stockrotationstage.t:#!/usr/bin/env perl
t/db_dependent/api/v1/acquisitions_orders.t:#!/usr/bin/env perl
t/db_dependent/api/v1/patrons.t:#!/usr/bin/env perl
t/db_dependent/api/v1/clubs_holds.t:#!/usr/bin/env perl
t/db_dependent/api/v1/illrequests.t:#!/usr/bin/env perl
t/db_dependent/www/regressions.t:#!/usr/bin/env perl
t/db_dependent/Search/History.t:#!/usr/bin/env perl
t/Members/cardnumber.t:#!/usr/bin/env perl
t/Search/History.t:#!/usr/bin/env perl
t/00-testcritic.t:#!/usr/bin/env perl
xt/author/podcorrectness.t:#!/usr/bin/env perl
xt/author/icondirectories.t:#!/usr/bin/env perl

Looks like they come from a mix of authors.

I might actually send a listserv email about this one...

(In reply to David Cook from comment #4)
> Agreed. I think that the env version allows for more flexibility and power,
> but I think that we should keep the shebangs consistent in Koha. (Otherwise,
> we might start up some Perl code using /opt/perl-new/bin/perl, which then
> invokes other code using /usr/bin/perl.)

Actually, David, your concern might be unfounded.

Based on the following:
https://metacpan.org/release/Plack/source/lib/Plack/App/CGIBin.pm
https://metacpan.org/release/Plack/source/lib/Plack/App/WrapCGI.pm

You can see that Perl files aren't invoked by the exec() function, and instead are turned into anonymous functions via CGI::Compile. At a glance, it seems like the shebang in a CGI script is irrelevant for Plack...

Closing this as WONTFIX.
Feel free to reopen if needed