Bug 17666 - .perl atomic update does not work under kohadevbox
Summary: .perl atomic update does not work under kohadevbox
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 17762
  Show dependency treegraph
 
Reported: 2016-11-22 08:21 UTC by Jonathan Druart
Modified: 2017-06-14 22:18 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 17666 - .perl atomic update does not work under kohadevbox (1.60 KB, patch)
2016-11-23 13:21 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17666: Revert "Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp" (2.21 KB, patch)
2016-12-13 11:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17666: Revert "Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp" (2.27 KB, patch)
2016-12-14 22:38 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17666: Revert "Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp" (2.36 KB, patch)
2016-12-15 07:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17666: [QA Follow-up] Mark File::Slurp as required (992 bytes, patch)
2016-12-15 07:52 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2016-11-22 08:21:53 UTC
% perl installer/data/mysql/updatedatabase.pl
DEV atomic update: bug_xxxx.perl

Error flock (1) on '/home/vagrant/kohaclone/installer/data/mysql/atomicupdate/bug_xxxx.perl': No locks available at installer/data/mysql/updatedatabase.pl line 13637.

This is because Path::Tiny->slurp[_utf8] uses flocks which does not work with NFS
Comment 1 Marcel de Rooy 2016-11-22 12:49:58 UTC
Can we remove Tiny::slurp in the meantime ?
Comment 2 Jonathan Druart 2016-11-22 12:56:13 UTC
(In reply to Marcel de Rooy from comment #1)
> Can we remove Tiny::slurp in the meantime ?

Path::Tiny you mean?
Which alternative do you suggest?
Comment 3 Jonathan Druart 2016-11-22 13:02:23 UTC
I also get
  lockd: cannot monitor kohadevbox
in /var/log/messages of the host.
Comment 4 Marcel de Rooy 2016-11-22 13:02:36 UTC
(In reply to Jonathan Druart from comment #2)
> (In reply to Marcel de Rooy from comment #1)
> > Can we remove Tiny::slurp in the meantime ?
> 
> Path::Tiny you mean?
> Which alternative do you suggest?

The old way is fine too
Comment 5 Jonathan Druart 2016-11-22 13:26:12 UTC
Do you mean using do instead of eval (i.e. revert of bug 16685)?
Comment 6 M. Tompsett 2016-11-22 16:59:30 UTC
Strange. It works for my windows box vagrant. Oh right, doesn't use NFS, uses built-in virtualbox driver.
Comment 7 Kyle M Hall 2016-11-23 13:21:34 UTC
Created attachment 57727 [details] [review]
Bug 17666 - .perl atomic update does not work under kohadevbox
Comment 8 Kyle M Hall 2016-11-23 13:22:10 UTC
Jonathan, does it work for you with this patch?
Comment 9 Jonathan Druart 2016-11-23 15:09:46 UTC
(In reply to Kyle M Hall from comment #8)
> Jonathan, does it work for you with this patch?

Yes of course it works but in that case it's certainly better to use File::Slurp::read_file
Comment 10 Marcel de Rooy 2016-11-25 13:01:39 UTC
(In reply to Jonathan Druart from comment #9)
> (In reply to Kyle M Hall from comment #8)
> > Jonathan, does it work for you with this patch?
> 
> Yes of course it works but in that case it's certainly better to use
> File::Slurp::read_file

File::Slurp is packaged on Debian.
It calls sysopen with O_RDONLY. So no flock.
So, it seems to be the best choice here.
Comment 11 Jonathan Druart 2016-12-13 11:09:08 UTC
Created attachment 58151 [details] [review]
Bug 17666: Revert "Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp"

This reverts commit 7ac1bc35372990520a7c67870b2c702c41bbebcc.

On a kohadevbox:
Error flock (1) on
'/home/vagrant/kohaclone/installer/data/mysql/atomicupdate/bug_xxxx.perl':
No locks available at installer/data/mysql/updatedatabase.pl line 13637.

This is because Path::Tiny->slurp[_utf8] uses flocks which does not work
with NFS

Test plan:
Execute a .perl atomic update file on a kohadevbox
=> Without this patch you should get the flock error
=> With this patch the update should work fine
Comment 12 Josef Moravec 2016-12-14 22:38:48 UTC
Created attachment 58193 [details] [review]
[SIGNED-OFF] Bug 17666: Revert "Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp"

This reverts commit 7ac1bc35372990520a7c67870b2c702c41bbebcc.

On a kohadevbox:
Error flock (1) on
'/home/vagrant/kohaclone/installer/data/mysql/atomicupdate/bug_xxxx.perl':
No locks available at installer/data/mysql/updatedatabase.pl line 13637.

This is because Path::Tiny->slurp[_utf8] uses flocks which does not work
with NFS

Test plan:
Execute a .perl atomic update file on a kohadevbox
=> Without this patch you should get the flock error
=> With this patch the update should work fine

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 13 Marcel de Rooy 2016-12-15 07:52:09 UTC
Created attachment 58196 [details] [review]
Bug 17666: Revert "Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp"

This reverts commit 7ac1bc35372990520a7c67870b2c702c41bbebcc.

On a kohadevbox:
Error flock (1) on
'/home/vagrant/kohaclone/installer/data/mysql/atomicupdate/bug_xxxx.perl':
No locks available at installer/data/mysql/updatedatabase.pl line 13637.

This is because Path::Tiny->slurp[_utf8] uses flocks which does not work
with NFS

Test plan:
Execute a .perl atomic update file on a kohadevbox
=> Without this patch you should get the flock error
=> With this patch the update should work fine

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Marcel de Rooy 2016-12-15 07:52:13 UTC
Created attachment 58197 [details] [review]
Bug 17666: [QA Follow-up] Mark File::Slurp as required

Not optional since we are 'using' it in updatedatabase.pl.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 15 Marcel de Rooy 2016-12-15 07:53:42 UTC
Note: Has been packaged for Jessie
Comment 16 Kyle M Hall 2016-12-16 11:14:44 UTC
Pushed to master for 17.05, thanks Jonathan, Marcel!
Comment 17 Katrin Fischer 2016-12-18 20:30:12 UTC
These patches have been pushed to 16.11.x, will be in 16.11.01.