When enabling Plack, koha-plack uncomment plack related Include in instance's apache conf. Little problem occurs when old pre-plack configs are used, without required Include lines. In this case koha-plack still report success, with no change done. So in the end we have running Plack, but it's doing nothing. plack.log is empty, but there is an entry in plack-error.log that can trick people into thinking they have Plack properly configured and running: 2018/05/01-21:06:27 Starman::Server (type Net::Server::PreFork) starting! pid(10257) Binding to UNIX socket file "/var/run/koha/galway/plack.sock" Setting gid to "1001 1001" Setting uid to "1001" koha-plack:enable_plack() should check if Plack lines exists before trying to uncomment them and either fail with warn message, or add them.
What says the about page (tab "system information")?
Created attachment 75038 [details] [review] Bug 20692: Return and display and error if koha-plack failed If koha-plack did not comment/uncomment the line it should fails and display an error instead of success. Useful for old apache config files
Created attachment 75040 [details] [review] Bug 20692: Return and display and error if koha-plack failed If koha-plack did not comment/uncomment the line it should fails and display an error instead of success. Useful for old apache config files
Created attachment 75510 [details] [review] Bug 20692: Return and display and error if koha-plack failed If koha-plack did not comment/uncomment the line it should fails and display an error instead of success. Useful for old apache config files
Created attachment 75511 [details] [review] Bug 20692: Return and display and error if koha-plack failed If koha-plack did not comment/uncomment the line it should fails and display an error instead of success. Useful for old apache config files
The 'q' portion of the sed logic is faulty.
Created attachment 75594 [details] [review] Bug 20692: Return and display and error if koha-plack failed If koha-plack did not comment/uncomment the line it should fails and display an error instead of success. Useful for old apache config files
Well, this is really not error proof but should improve a bit the behavior (not the code!)
(In reply to Jonathan Druart from comment #8) > Well, this is really not error proof but should improve a bit the behavior > (not the code!) Hmmm... it gives me an idea... I'll see if I can create a better counter patch, if not, I'll sign this off.
Created attachment 75597 [details] [review] Bug 20692: Extreme toggle checking for plack TEST PLAN --------- sudo koha-plack --disable sudo koha-plack --disable sudo koha-plack --enable sudo koha-plack --disable echo $? sudo koha-plack --disable echo $? sudo koha-list --noplack echo $? sudo koha-plack --enable echo $? sudo koha-plack --enable echo $? sudo koha-list --plack echo $? git bz apply 20692 sudo perl ~/misc4dev/cp_debian_files.pl then repeat the test. It should also be noted, because the enable and disable are now split, if for some reason they become out of sync, an enable or disable should correct them. Because these are bash scripts, the test tool (/home/vagrant/qa-test-tools/koha-qa.pl) does not apply.
Created attachment 88085 [details] [review] Bug 20692: Extreme toggle checking for plack TEST PLAN --------- sudo koha-plack --disable sudo koha-plack --disable sudo koha-plack --enable sudo koha-plack --disable echo $? sudo koha-plack --disable echo $? sudo koha-list --noplack echo $? sudo koha-plack --enable echo $? sudo koha-plack --enable echo $? sudo koha-list --plack echo $? git bz apply 20692 sudo perl ~/misc4dev/cp_debian_files.pl then repeat the test. It should also be noted, because the enable and disable are now split, if for some reason they become out of sync, an enable or disable should correct them. Because these are bash scripts, the test tool (/home/vagrant/qa-test-tools/koha-qa.pl) does not apply. Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Created attachment 88698 [details] [review] Bug 20692: Extreme toggle checking for plack TEST PLAN --------- sudo koha-plack --disable sudo koha-plack --disable sudo koha-plack --enable sudo koha-plack --disable echo $? sudo koha-plack --disable echo $? sudo koha-list --noplack echo $? sudo koha-plack --enable echo $? sudo koha-plack --enable echo $? sudo koha-list --plack echo $? git bz apply 20692 sudo perl ~/misc4dev/cp_debian_files.pl then repeat the test. It should also be noted, because the enable and disable are now split, if for some reason they become out of sync, an enable or disable should correct them. Because these are bash scripts, the test tool (/home/vagrant/qa-test-tools/koha-qa.pl) does not apply. Signed-off-by: Liz Rea <wizzyrea@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Nice bit of code curling there guys, a solid improvement, thanks. Passing QA
Awesome work all! Pushed to master for 19.05
Pushed to 18.11.x for 18.11.05
backported to 18.05.x for 18.05.13