Bug 7779

Summary: tools/letter.pl: scope vars for plack
Product: Koha Reporter: Dobrica Pavlinusic <dpavlin>
Component: ToolsAssignee: Dobrica Pavlinusic <dpavlin>
Status: CLOSED FIXED QA Contact: Ian Walls <koha.sekjal>
Severity: enhancement    
Priority: P5 - low CC: alex.arnaud, colin.campbell, paul.poulain
Version: 3.8   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 7172    
Attachments: proposed patch
Bug 7779 - tools/letter.pl: scope vars for plack
[SIGNED-OFF] Bug 7779 - tools/letter.pl: scope vars for plack

Description Dobrica Pavlinusic 2012-03-21 13:27:08 UTC Comment hidden (obsolete)
Comment 1 Dobrica Pavlinusic 2012-03-21 15:49:25 UTC Comment hidden (obsolete)
Comment 2 Alex Arnaud 2012-03-21 15:57:19 UTC
it's ok, signing-off
Comment 3 Alex Arnaud 2012-03-21 15:58:40 UTC
Created attachment 8447 [details] [review]
[SIGNED-OFF] Bug 7779 - tools/letter.pl: scope vars for plack
Comment 4 Paul Poulain 2012-03-22 17:18:14 UTC
QA comment : s/my/our/ changes only, for plack compliance, won't change anything

passed QA
Comment 5 Colin Campbell 2012-03-23 09:31:46 UTC
Would like to see a proper rationale for introducing package variables into scripts. Looks like we are pasting over bugs in the C4 code. Wouldbe helpful if bug described why this was done so that it does not propagate unnecessarily.
Comment 6 Paul Poulain 2012-03-23 09:59:55 UTC
(In reply to comment #5)
> Would like to see a proper rationale for introducing package variables into
> scripts. Looks like we are pasting over bugs in the C4 code. Wouldbe helpful
> if bug described why this was done so that it does not propagate
> unnecessarily.

Not sure I understand this comment but: have you seen http://wiki.koha-community.org/wiki/Plack#Running_Intranet and the paragraph:

Second day of plack testing showed that we are somewhat away from goal of running intranet under plack. Currently biggest problems are variabled declared with my which are inside sub (since plack wraps our cgi scripts within sub). This problem is similar to mod_perl problem described at http://modperlbook.org/html/6-2-Exposing-Apache-Registry-Secrets.html (just ignore mod_perl references :-) 


Those patches are just a quick way to solve the problem plack cause. A better, more long term & clean will have to be done later.
Comment 7 Dobrica Pavlinusic 2012-03-25 23:34:42 UTC
I added one paragraph to wiki page, and I'm including it also here for reference:

Most of the patches are simple changes which change scoping of variables used within subs inside *.pl files. This is needed because pack wraps each CGI script into sub. While this approach may seem like a cludge, as we are creating global variables this doesn't affect CGI since all variables are released on end of request. In case of plack, this will increase in-memory size of plack a little bit, but it's simplest way (IMHO :-) to get code running on plack while preserving compatibility with CGI execution of Koha.
Comment 8 Jared Camins-Esakov 2012-12-31 00:46:44 UTC
There have been no further reports of problems so I am marking this bug resolved.