Bug 28054 - SIPServer.pm is a program and requires a shebang
Summary: SIPServer.pm is a program and requires a shebang
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Colin Campbell
QA Contact: Testopia
URL:
Keywords:
Depends on: 26673
Blocks:
  Show dependency treegraph
 
Reported: 2021-03-30 11:47 UTC by Colin Campbell
Modified: 2021-12-13 21:10 UTC (History)
6 users (show)

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


Attachments
patch restoring the shebang (776 bytes, patch)
2021-03-30 11:55 UTC, Colin Campbell
Details | Diff | Splinter Review
Bug 28054 Reinstate the shebang in SIPServer.pm (840 bytes, patch)
2021-03-30 12:06 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Colin Campbell 2021-03-30 11:47:42 UTC
Bug 26673 removed the shebang from SIPServer.pm however despite its name it is not a module but a program if started other than 'perl SIPServer.pm' it will fail to run as no interpreter is found. This may effect custom startup files or attempts to test/debug by developers.

Why the misleading name? dont know, I think it reflects the heritage of OpenSIP which tried to do some clever things without fully understanding them leaving the rest of us to puzzle over the code subsequently
Comment 1 Colin Campbell 2021-03-30 11:55:48 UTC
Created attachment 118994 [details] [review]
patch restoring the shebang

restore the shebang so that
SIPServer.pm config_file
is again equivalent of
perl SIPServer.pm config_file
Comment 2 Martin Renvoize 2021-03-30 12:05:22 UTC
Doh!  This ought to have been caught at QA for bug 26673, but knowledge around SIP is somewhat limited in the QA circle :(.

Going for a straight PQA on this one.. it's a trivial and obvious correction to that initial mistake.
Comment 3 Martin Renvoize 2021-03-30 12:06:11 UTC
Created attachment 118996 [details] [review]
Bug 28054 Reinstate the shebang in SIPServer.pm

Bug 26673 renoved the shebang from SIPServer.pm. This was an error
SIPServer.pm is a program not a perl module, and needs the shebang
for the OS to assign an interpreter unless the command line
calls it as an argument to perl. Restore the shebang

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Jonathan Druart 2021-04-01 16:55:55 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 5 David Cook 2021-04-06 01:18:05 UTC
Yikes... looking at SIPServer.pm and shaking my head.

If we cared enough, we'd move that loose code in SIPServer.pm into a function, and then create a sip_server.pl script that invokes it.
Comment 6 Martin Renvoize 2021-04-08 06:55:24 UTC
I discussed that with Colin just prior the his retiring. He suggested it wasn't as simple as it looked. 

I'd love to clean up the sip code at some point, but alas I don't know if/when I'll have time to spend on that :(
Comment 7 Fridolin Somers 2021-04-15 10:21:18 UTC
Pushed to 20.11.x for 20.11.05
Comment 8 David Cook 2021-04-16 00:33:41 UTC
(In reply to Martin Renvoize from comment #6)
> I discussed that with Colin just prior the his retiring. He suggested it
> wasn't as simple as it looked. 

I didn't realize that Colin had retired! When was that?

> I'd love to clean up the sip code at some point, but alas I don't know
> if/when I'll have time to spend on that :(

Hard relate. I only have maybe 5 out of 100 libraries using SIP so it's not really a work-related priority.
Comment 9 Andrew Fuerste-Henry 2021-04-24 16:08:21 UTC
Pushed to 20.05.x for 20.05.11
Comment 10 Victor Grousset/tuxayo 2021-04-24 17:13:29 UTC
Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed.