We got a lot of support questions because scripts are executed without the correct environment/user. We could die and provide more useful information
Created attachment 69568 [details] [review] Bug 19760: Die if koha-conf is not accessible We got a lot of support questions because scripts are executed without the correct environment/user. We could die and provide more useful information
Created attachment 69571 [details] [review] Bug 19760: Die if koha-conf is not accessible We got a lot of support questions because scripts are executed without the correct environment/user. We could die and provide more useful information Tested on my kohadevbox VM by: 1) sudo chmod a-rx /etc/koha/sites/kohadev/koha-conf.xml 2) run help.pl from the command line to produce the desired die message 3) sudo chmod a+rx /etc/koha/sites/kohadev/koha-conf.xml 4) rerun help.pl successfully Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk>
I'm not sure about the message, should we recommend a tool that is only available on Debian package installs ? Also, there is a strange behaviour (not caused by this patch), when KOHA_CONF is empty or does not contain a valid/existing filename, it is just defaulted to /etc/koha/koha-conf.xml (if it exists). So, on my dev install, I can run `KOHA_CONF=/not/existing perl help.pl` and it uses /etc/koha/koha-conf.xml without warning
Changing status to In Discussion to get answers to comment 3
(In reply to Julian Maurice from comment #3) > I'm not sure about the message, should we recommend a tool that is only > available on Debian package installs ? It is the common mistake people does when they use kohadevbox, so they will have access to koha-shell. What do you suggest? > Also, there is a strange behaviour (not caused by this patch), when > KOHA_CONF is empty or does not contain a valid/existing filename, it is just > defaulted to /etc/koha/koha-conf.xml (if it exists). > So, on my dev install, I can run `KOHA_CONF=/not/existing perl help.pl` and > it uses /etc/koha/koha-conf.xml without warning Yes we should add a warning instead.
(In reply to Jonathan Druart from comment #5) > It is the common mistake people does when they use kohadevbox, so they will > have access to koha-shell. > What do you suggest? I'm not sure, I just don't like how Koha is being more and more coupled with Debian. I won't have time to think about another solution for the next two weeks and I don't want to block this bug, so I'm resetting the status to Signed Off. @QAers, feel free to take this one.
We could just add something like: If you are using a debian-based system you can use the koha-shell command otherwise you should check your permissions.
(In reply to Jonathan Druart from comment #7) > We could just add something like: If you are using a debian-based system you > can use the koha-shell command otherwise you should check your permissions. Maybe "Try running this again as the koha instance user (or use the koha-shell command in debian)"
Created attachment 70014 [details] [review] Bug 19760: Change wording
(In reply to Jon Knight from comment #2) > 3) sudo chmod a+rx /etc/koha/sites/kohadev/koha-conf.xml chmod 640 might be better ?
Created attachment 70116 [details] [review] Bug 19760: Die if koha-conf is not accessible We got a lot of support questions because scripts are executed without the correct environment/user. We could die and provide more useful information Tested on my kohadevbox VM by: 1) sudo chmod a-rx /etc/koha/sites/kohadev/koha-conf.xml 2) run help.pl from the command line to produce the desired die message 3) sudo chmod a+rx /etc/koha/sites/kohadev/koha-conf.xml 4) rerun help.pl successfully Signed-off-by: Jon Knight <J.P.Knight@lboro.ac.uk> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 70117 [details] [review] Bug 19760: Change wording Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 18.05, thanks to everybody involved!
Created attachment 70135 [details] [review] Bug 19760: Fix Config.t test Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Last patch pushed to master
Awesome work all, pushed to stable for 17.11.02
Pushed to 17.05.x for v17.05.08