Bug 7779 - tools/letter.pl: scope vars for plack
Summary: tools/letter.pl: scope vars for plack
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: 3.8
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Dobrica Pavlinusic
QA Contact: Ian Walls
URL:
Keywords:
Depends on:
Blocks: 7172
  Show dependency treegraph
 
Reported: 2012-03-21 13:27 UTC by Dobrica Pavlinusic
Modified: 2013-12-05 20:04 UTC (History)
3 users (show)

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


Attachments
proposed patch (1.49 KB, patch)
2012-03-21 13:27 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
Bug 7779 - tools/letter.pl: scope vars for plack (1.63 KB, patch)
2012-03-21 15:49 UTC, Dobrica Pavlinusic
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 7779 - tools/letter.pl: scope vars for plack (1.68 KB, patch)
2012-03-21 15:58 UTC, Alex Arnaud
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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.