Bug 26658 - Shebangs are wrong in bin/intranet and bin/opac
Summary: Shebangs are wrong in bin/intranet and bin/opac
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Julian Maurice
QA Contact: Testopia
URL:
Keywords:
Depends on: 20582
Blocks:
  Show dependency treegraph
 
Reported: 2020-10-12 06:20 UTC by Julian Maurice
Modified: 2023-12-28 20:43 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 26658: Fix shebangs of bin/intranet and bin/opac (724 bytes, patch)
2020-10-12 06:21 UTC, Julian Maurice
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Maurice 2020-10-12 06:20:16 UTC
Currently it's #!/usr/bin/env perl, it should be #!/usr/bin/perl
Comment 1 Julian Maurice 2020-10-12 06:21:03 UTC
Created attachment 111466 [details] [review]
Bug 26658: Fix shebangs of bin/intranet and bin/opac
Comment 2 Magnus Enger 2020-10-12 06:50:45 UTC
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! :-)
Comment 3 Julian Maurice 2020-10-12 07:04:37 UTC
(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 :)
Comment 4 David Cook 2020-10-12 22:24:52 UTC
(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.)
Comment 5 David Cook 2020-10-12 22:28:25 UTC
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
Comment 6 David Cook 2020-10-12 22:41:09 UTC
Looks like they come from a mix of authors.

I might actually send a listserv email about this one...
Comment 7 David Cook 2020-10-12 23:04:01 UTC
(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...
Comment 8 Julian Maurice 2023-02-17 11:24:43 UTC
Closing this as WONTFIX.
Feel free to reopen if needed