Created attachment 8408 [details] [review]
Created attachment 8442 [details] [review]
Bug 7779 - tools/letter.pl: scope vars for plack
it's ok, signing-off
Created attachment 8447 [details] [review]
[SIGNED-OFF] Bug 7779 - tools/letter.pl: scope vars for plack
QA comment : s/my/our/ changes only, for plack compliance, won't change anything
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.
(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
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.
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.
There have been no further reports of problems so I am marking this bug resolved.