Bug 28054

Summary:	SIPServer.pm is a program and requires a shebang
Product:	Koha	Reporter:	Colin Campbell <colin.campbell>
Component:	SIP2	Assignee:	Colin Campbell <colin.campbell>
Status:	CLOSED FIXED	QA Contact:	Testopia <testopia>
Severity:	minor
Priority:	P5 - low	CC:	andrew, dcook, fridolin.somers, jonathan.druart, martin.renvoize, victor
Version:	Main
Hardware:	All
OS:	All
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
Circulation function:
Bug Depends on:	26673
Bug Blocks:
Attachments:	patch restoring the shebang Bug 28054 Reinstate the shebang in SIPServer.pm

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 (ashimema) 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 (ashimema) 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 (ashimema) 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.