Bug 16685 - Use eval instead of do for .perl atomicupdates
Summary: Use eval instead of do for .perl atomicupdates
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords: dependency, nowheezy
Depends on:
Blocks:
 
Reported: 2016-06-07 14:08 UTC by Kyle M Hall
Modified: 2017-06-14 22:16 UTC (History)
7 users (show)

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


Attachments
Bug 16685 - Use eval instead of do for .perl atomicupdates (1.83 KB, patch)
2016-06-07 14:11 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 16685 - Use eval instead of do for .perl atomicupdates (1.83 KB, patch)
2016-06-27 15:58 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp (1.95 KB, patch)
2016-06-27 15:59 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp (1.95 KB, patch)
2016-08-17 14:59 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp (1.95 KB, patch)
2016-08-17 15:01 UTC, Mark Tompsett
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16685 - Use eval instead of do for .perl atomicupdates (1.90 KB, patch)
2016-08-23 00:33 UTC, Aleisha Amohia
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp (2.02 KB, patch)
2016-08-23 00:33 UTC, Aleisha Amohia
Details | Diff | Splinter Review
Bug 16685 - Use eval instead of do for .perl atomicupdates (1.96 KB, patch)
2016-08-23 10:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp (2.08 KB, patch)
2016-08-23 10:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16685 Path::Tiny Control file update (master) (877 bytes, patch)
2016-10-12 06:36 UTC, Mirko Tietgen
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2016-06-07 14:08:49 UTC
If we use eval instead of do for our .perl atomic update files, it will allow developers to put in the exact code that should go into updatedatabase.pl. The problem with do is that none of the variables defined in updatadatabase.pl are available, whereas with eval they are.
Comment 1 Kyle M Hall 2016-06-07 14:11:58 UTC Comment hidden (obsolete)
Comment 2 Frédéric Demians 2016-06-15 15:36:26 UTC
I remember Jonathan saying that File::Slurp was 'bad'. It the same time, I can see it in several place, and recently added with Edifact.

http://blogs.perl.org/users/leon_timmermans/2015/08/fileslurp-is-broken-and-wrong.html

Path::Tiny is generally regarded as a better alternative.
Comment 3 Kyle M Hall 2016-06-27 15:58:59 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2016-06-27 15:59:05 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi 2016-07-26 19:14:48 UTC
Nice one Kyle, can u please rebase? I'd put the new dep somewhere alphabetically. Is it packaged already?
Comment 6 Mark Tompsett 2016-08-17 14:59:39 UTC Comment hidden (obsolete)
Comment 7 Mark Tompsett 2016-08-17 15:01:29 UTC Comment hidden (obsolete)
Comment 8 Mark Tompsett 2016-08-17 15:02:43 UTC
Rebased. Kitten saved.
Comment 9 Aleisha Amohia 2016-08-23 00:33:29 UTC Comment hidden (obsolete)
Comment 10 Aleisha Amohia 2016-08-23 00:33:36 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2016-08-23 10:18:10 UTC
Created attachment 54758 [details] [review]
Bug 16685 - Use eval instead of do for .perl atomicupdates

If we use eval instead of do for our .perl atomic update files, it will
allow developers to put in the exact code that should go into
updatedatabase.pl. The problem with do is that none of the variables
defined in updatadatabase.pl are available, whereas with eval they are.

Test Plan:
1) Apply this patch
2) Create a .perl file in atomicupdates with the following in it:
say "DBversion: $DBversion";
3) Run updatadatabase.pl
4) Note the output
5) Add a syntax error to your atomic update
6) Run updatedatabase.pl
7) Note the error is displayed

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 12 Jonathan Druart 2016-08-23 10:18:23 UTC
Created attachment 54759 [details] [review]
Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp

File::Slurp is considered better to use than File::Slurp.

RM Note: The package version in wheezy is too old, the package version
         in jessie is just fine and should be added to the community
         repo if wheezy will be supported for the coming release.

Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 13 Kyle M Hall 2016-09-09 13:53:07 UTC
Pushed to master for 16.11!
Comment 14 Mirko Tietgen 2016-10-12 06:36:06 UTC
Created attachment 56219 [details] [review]
Bug 16685 Path::Tiny Control file update (master)
Comment 15 Mirko Tietgen 2016-10-12 13:12:56 UTC
Comment on attachment 56219 [details] [review]
Bug 16685 Path::Tiny Control file update (master)

Moving the control file patch to another bug
Comment 16 Mirko Tietgen 2016-10-12 13:38:32 UTC
(In reply to Jonathan Druart from comment #12)
> RM Note: The package version in wheezy is too old, the package version
>          in jessie is just fine and should be added to the community
>          repo if wheezy will be supported for the coming release.

Wheezy will not be supported by 16.11.
Comment 17 Jonathan Druart 2016-11-22 08:22:35 UTC
This patchset breaks DB update on kohadevbox, see bug 17666