Bug 17666

Summary: .perl atomic update does not work under kohadevbox
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: katrin.fischer, kyle, m.de.rooy, mtompset
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=16685
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 17762    
Attachments: Bug 17666 - .perl atomic update does not work under kohadevbox
Bug 17666: Revert "Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp"
[SIGNED-OFF] Bug 17666: Revert "Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp"
Bug 17666: Revert "Bug 16685 [QA Followup] - Use Path::Tiny::slurp instead of File::Slurp"
Bug 17666: [QA Follow-up] Mark File::Slurp as required

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 Mark 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.