Bug 16685

Summary: Use eval instead of do for .perl atomicupdates
Product: Koha Reporter: Kyle M Hall <kyle>
Component: Architecture, internals, and plumbingAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: f.demians, jonathan.druart, m.de.rooy, mirko, mtj, mtompset, tomascohen
Version: masterKeywords: dependency, nowheezy
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17666
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Bug 16685 - Use eval instead of do for .perl atomicupdates
Bug 16685 - Use eval instead of do for .perl atomicupdates
Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp
Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp
Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp
[SIGNED-OFF] Bug 16685 - Use eval instead of do for .perl atomicupdates
[SIGNED-OFF] Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp
Bug 16685 - Use eval instead of do for .perl atomicupdates
Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp
Bug 16685 Path::Tiny Control file update (master)

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