Bug 16749 - Additional fixes for debian scripts
Summary: Additional fixes for debian scripts
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Packaging (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
: 13295 16226 (view as bug list)
Depends on: 16733
Blocks: 18502
  Show dependency treegraph
 
Reported: 2016-06-16 08:43 UTC by Marcel de Rooy
Modified: 2018-12-03 20:04 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16749: Adjustments for koha-translate (5.59 KB, patch)
2016-06-20 11:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Update debian docs for koha-translate (1004 bytes, patch)
2016-09-12 09:54 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 17049: Improve PERL5LIB handling for packaged dev install (1.88 KB, patch)
2017-02-09 12:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjust xmlstarlet calls in koha-functions (2.22 KB, patch)
2017-02-10 08:39 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-plack (2.49 KB, patch)
2017-02-10 09:38 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjust xmlstarlet calls in koha-functions (3.08 KB, patch)
2017-02-10 11:19 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-plack (2.83 KB, patch)
2017-02-10 11:19 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjust xmlstarlet calls in koha-functions (3.08 KB, patch)
2017-02-10 12:35 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-plack (2.83 KB, patch)
2017-02-10 12:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-translate (5.72 KB, patch)
2017-02-10 12:36 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-translate (5.71 KB, patch)
2017-02-10 13:03 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-translate (5.86 KB, patch)
2017-02-11 08:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-translate (6.22 KB, patch)
2017-02-11 08:29 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjust xmlstarlet calls in koha-functions (2.84 KB, patch)
2017-02-12 13:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-plack (3.35 KB, patch)
2017-02-12 13:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-translate (6.41 KB, patch)
2017-02-12 13:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-translate (6.65 KB, patch)
2017-02-12 14:17 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Update debian docs for koha-translate (998 bytes, patch)
2017-02-13 07:42 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Go with a boolean flag, we love booleans (2.04 KB, patch)
2017-02-13 09:28 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjust xmlstarlet calls in koha-functions (2.91 KB, patch)
2017-02-14 12:21 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-plack (3.42 KB, patch)
2017-02-14 12:21 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-translate (6.71 KB, patch)
2017-02-14 12:21 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 16749: Update debian docs for koha-translate (1.04 KB, patch)
2017-02-14 12:21 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 16749: Go with a boolean flag, we love booleans (2.10 KB, patch)
2017-02-14 12:21 UTC, Mirko Tietgen
Details | Diff | Splinter Review
Bug 16749: Use is_instance to make sure the dev instance exist (803 bytes, patch)
2017-02-16 12:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16749: Check instancename in adjust_paths_dev_install (1.06 KB, patch)
2017-02-16 13:35 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 16749: Adjust xmlstarlet calls in koha-functions (2.98 KB, patch)
2017-02-16 14:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-plack (3.49 KB, patch)
2017-02-16 14:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16749: Adjustments for koha-translate (6.78 KB, patch)
2017-02-16 14:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16749: Update debian docs for koha-translate (1.11 KB, patch)
2017-02-16 14:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16749: Go with a boolean flag, we love booleans (2.18 KB, patch)
2017-02-16 14:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16749: Use is_instance to make sure the dev instance exist (876 bytes, patch)
2017-02-16 14:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16749: Check instancename in adjust_paths_dev_install (1.09 KB, patch)
2017-02-16 14:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16749: Add is_instance check to koha-upgrade-schema (1.14 KB, patch)
2017-02-16 14:08 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2016-06-16 08:43:01 UTC

    
Comment 1 Marcel de Rooy 2016-06-20 11:29:49 UTC Comment hidden (obsolete)
Comment 2 Marcel de Rooy 2016-06-20 11:31:50 UTC
*** Bug 13295 has been marked as a duplicate of this bug. ***
Comment 3 Magnus Enger 2016-09-08 13:18:04 UTC
debian/docs/koha-translate.xml needs to be updated with the --dev option.
Comment 4 Marcel de Rooy 2016-09-12 09:54:58 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2016-12-02 11:45:54 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2016-12-02 11:48:42 UTC Comment hidden (obsolete)
Comment 7 Marcel de Rooy 2016-12-05 12:39:43 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2016-12-05 13:32:23 UTC Comment hidden (obsolete)
Comment 9 Marcel de Rooy 2016-12-05 14:17:40 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2017-01-02 10:55:14 UTC
(In reply to Marcel de Rooy from comment #9)
> (In reply to Jonathan Druart from comment #8)
> > Why don't you use intranetdir?
> 
> dev_install tells you too that this is a gitified install

In that case, does a boolean flag would be enough? If dev_install will always be equal to intranetdir it would make more sense to have a flag. What do you think?
Comment 11 Marcel de Rooy 2017-01-03 12:47:32 UTC
(In reply to Jonathan Druart from comment #10)
> (In reply to Marcel de Rooy from comment #9)
> > (In reply to Jonathan Druart from comment #8)
> > > Why don't you use intranetdir?
> > 
> > dev_install tells you too that this is a gitified install
> 
> In that case, does a boolean flag would be enough? If dev_install will
> always be equal to intranetdir it would make more sense to have a flag. What
> do you think?

That sounds reasonable ;)
Comment 12 Marcel de Rooy 2017-01-09 09:47:06 UTC Comment hidden (obsolete)
Comment 13 Marcel de Rooy 2017-01-09 09:47:46 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2017-01-24 08:28:55 UTC
Tomas, can we get your opinion on comment 10?
Comment 15 Tomás Cohen Arazi 2017-01-24 12:07:54 UTC
(In reply to Jonathan Druart from comment #10)
> (In reply to Marcel de Rooy from comment #9)
> > (In reply to Jonathan Druart from comment #8)
> > > Why don't you use intranetdir?
> > 
> > dev_install tells you too that this is a gitified install
> 
> In that case, does a boolean flag would be enough? If dev_install will
> always be equal to intranetdir it would make more sense to have a flag. What
> do you think?

I think we should go with a boolean flag. We love booleans.
Comment 16 Marcel de Rooy 2017-02-09 12:05:14 UTC Comment hidden (obsolete)
Comment 17 Marcel de Rooy 2017-02-09 12:07:58 UTC Comment hidden (obsolete)
Comment 18 Marcel de Rooy 2017-02-10 08:39:07 UTC Comment hidden (obsolete)
Comment 19 Marcel de Rooy 2017-02-10 09:38:10 UTC Comment hidden (obsolete)
Comment 20 Marcel de Rooy 2017-02-10 10:41:47 UTC Comment hidden (obsolete)
Comment 21 Marcel de Rooy 2017-02-10 11:19:01 UTC Comment hidden (obsolete)
Comment 22 Marcel de Rooy 2017-02-10 11:19:07 UTC Comment hidden (obsolete)
Comment 23 Marcel de Rooy 2017-02-10 12:35:59 UTC Comment hidden (obsolete)
Comment 24 Marcel de Rooy 2017-02-10 12:36:04 UTC Comment hidden (obsolete)
Comment 25 Marcel de Rooy 2017-02-10 12:36:08 UTC Comment hidden (obsolete)
Comment 26 Marcel de Rooy 2017-02-10 12:42:29 UTC Comment hidden (obsolete)
Comment 27 Marcel de Rooy 2017-02-10 13:03:55 UTC Comment hidden (obsolete)
Comment 28 Marcel de Rooy 2017-02-11 08:17:45 UTC Comment hidden (obsolete)
Comment 29 Marcel de Rooy 2017-02-11 08:29:34 UTC Comment hidden (obsolete)
Comment 30 Marcel de Rooy 2017-02-12 13:58:59 UTC
Created attachment 60129 [details] [review]
Bug 16749: Adjust xmlstarlet calls in koha-functions

In various scripts we use xmlstarlet to extract values from koha-conf.
If we call xmlstarlet on non-existing entries in koha-conf, this may
however result in silently failing scripts (when set -e is in effect).

A function run_safe_xmlstarlet is added for situations where the entry
might not exist. It will not halt execution.

This patch only adjusts koha-functions.sh and modifies the xmlstarlet calls
for dev_install and zebra_loglevels.

Note: The function does not need to check file existence. If the file does
not exist, xmlstarlet warns about it; the function returns empty string,
but does not set an error exit status.

Test plan: See second patch ("koha-plack adjustments").

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 31 Marcel de Rooy 2017-02-12 13:59:04 UTC
Created attachment 60130 [details] [review]
Bug 16749: Adjustments for koha-plack

[1] Use run_safe_xmlstarlet for plack workers and requests
[2] Simplify adjust_paths. The lazy export statement is actually enough to
    replace adjust_paths by one direct call to adjust_paths_dev_install.

Test plan:
[1] Copy koha-functions.sh and koha-plack:
    cp [YOUR_PATH]/debian/scripts/koha-functions.sh /usr/share/koha/bin/
    cp [YOUR_PATH]/debian/scripts/koha-plack /usr/sbin/
    where YOUR_PATH might well be /home/vagrant/kohaclone.
[2] Make sure that you have dev_install in koha-conf.
    Stop and start koha-plack. Verify with ps aux|grep plack.
[3] Rename dev_install to nodev_install (in start and end tag).
    Now stop/start koha-plack. Verify with ps aux|grep plack.
[4] Change plack_requests to 51 in your koha-conf.
    Restart Plack and check that you see 51 in ps aux|grep plack.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested on Jessie (Debian VM and Kohadevbox)
Comment 32 Marcel de Rooy 2017-02-12 13:59:08 UTC Comment hidden (obsolete)
Comment 33 Marcel de Rooy 2017-02-12 14:17:16 UTC
Created attachment 60132 [details] [review]
Bug 16749: Adjustments for koha-translate

[AMENDED February 10, 2017]

[1] Added reading /etc/default/koha-common as in the other debian scripts.
    We need it for KOHA_HOME.
[2] Add a -d|--dev parameter for dev installs.
[3] No hardcoded PERL5LIB or KOHA_INSTALL_DIR (KOHA_HOME).
    They are read from default file or set by adjust_paths_dev_install.
[4] Adjust template paths for dev installs: OPAC_TMPL, INTRANET_TMPL.
[5] Remove references to obsolete themes ccsr and prog.

Test plan:
[1] Regular package install:
    Copy koha-translate to /usr/sbin.
    Run koha-translate -l to show installed languages.
    Run koha-translate -l -a to show available languages.
    Add a language: koha-translate -i nl-NL.
    Check template folders in regular location (/usr/share/koha/...)
    Remove a language: koha-translate -r nl-NL. Check again.
[2] Dev install or kohadevbox:
    Copy koha-translate to /usr/sbin.
    If needed, add the <dev_install> line to koha-conf.xml.
    Run koha-translate -l -d yourinstance to show installed languages.
    (Note: You only see the languages installed in this instance.)
    Add a language: koha-translate -i nl-NL -d yourinstance.
    Check template folders in the clone.
    Remove a language: koha-translate -r nl-NL -d yourinstance.

    Note: Make sure you have sufficient file permissions for the kohaclone
    files and koha-conf.xml. On kohadevbox you might need to run sudo
    koha-translate within the the vagrant user context.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested on Jessie (Debian VM and Kohadevbox)
Comment 34 Marcel de Rooy 2017-02-13 07:42:22 UTC
Created attachment 60139 [details] [review]
Bug 16749: Update debian docs for koha-translate

This patch adds the -d option to the documentation (with thanks to
Magnus Enger).
Comment 35 Marcel de Rooy 2017-02-13 09:28:56 UTC
Created attachment 60140 [details] [review]
Bug 16749: Go with a boolean flag, we love booleans

As requested by Jonathan on comment 10 on Bugzilla and supported by Tomas,
we should use the dev_install entry in koha-conf as a boolean flag.
Since we already used it as a path, this patch is not too strict about it.
If the entry is not empty and not equal to "0", we will interpret it as
a true value. The path is taken from the intranetdir entry.

Test plan:
[1] Copy debian/scripts/koha-functions.sh to /usr/share/koha/bin
[2] For a dev install:
    Remove the dev install line, or toggle its value between empty string,
    0 or 1 and each time test stop/start koha-indexer.
    Check the path to rebuild_zebra with ps aux|grep indexer.
    If you have no entry, an empty entry or a zero, you should see a regular
    path.
    (Note: You can do something similar with koha-start-sip.)
[3] For a regular install:
    Remove the dev install line.
    Stop/start koha-indexer or koha-plack, and verify that it still works.
    Add a dev_install line with 0, and repeat stop/start.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 36 Marcel de Rooy 2017-02-13 09:40:22 UTC
This should do it for now.
We need these patches following 16733 in 17.05 (especially the first two).
Have been testing them quite extensively.
Hoping for further attention soon..
Comment 37 Mirko Tietgen 2017-02-13 14:15:06 UTC
This patch set appears to fix the koha-common.postinst error presumably introduced in bug 16733.
Comment 38 Mirko Tietgen 2017-02-13 14:47:15 UTC
I have tested patch 3 and 4 in a regular package installation of master+all patches from this bug and they work. I can also confirm that the postinst issue is gone with this patch set.

i tried plack_requests from the test plan in patch 2 in koha-conf.xml but it did not have any effect in the regular installation. I may be missing a parameter or it is not applicable for a regular install?
Comment 39 Marcel de Rooy 2017-02-13 18:05:41 UTC
(In reply to Mirko Tietgen from comment #38)
> I have tested patch 3 and 4 in a regular package installation of master+all
> patches from this bug and they work. I can also confirm that the postinst
> issue is gone with this patch set.
> 
> i tried plack_requests from the test plan in patch 2 in koha-conf.xml but it
> did not have any effect in the regular installation. I may be missing a
> parameter or it is not applicable for a regular install?

Thanks for testing.

Should work on a regular instance too.
Just to be sure: Master up-to-date including 17610? Did you copy the debian scripts from the master clone to /usr/sbin? And koha-functions.sh to /usr/share/koha/bin? And did you edit the koha-conf of the instance for which you are restarting plack? This setting is instance specific.

You should see something like (52 coming from my koha-conf):
master-+ 28433  0.0  0.1  61320 14636 ?        Ss   18:57   0:00 starman master -M FindBin --max-requests 52 --workers 2 --user=master-koha --group master-koha --pid /var/run/koha/master/plack.pid --daemonize --access-log /var/log/koha/master/plack.log --error-log /var/log/koha/master/plack-error.log -E deployment --socket /var/run/koha/master/plack.sock /etc/koha/plack.psgi
Comment 40 Mirko Tietgen 2017-02-14 07:10:51 UTC
I did not copy anything, I built a Debian package, so everything is in place already. I edited the correct instance koha-conf file.

According to bug 17610 it is plack_max_requests, not plack_requests. I will try that.
Comment 41 Marcel de Rooy 2017-02-14 08:38:41 UTC
(In reply to Mirko Tietgen from comment #40)
> I did not copy anything, I built a Debian package, so everything is in place
> already. I edited the correct instance koha-conf file.
> 
> According to bug 17610 it is plack_max_requests, not plack_requests. I will
> try that.

Oh sorry! You are of course completely right:
 <plack_max_requests>52</plack_max_requests>
 <plack_workers>2</plack_workers>

These unreliable test plans :)
Comment 42 Mirko Tietgen 2017-02-14 12:21:20 UTC
Created attachment 60193 [details] [review]
Bug 16749: Adjust xmlstarlet calls in koha-functions

In various scripts we use xmlstarlet to extract values from koha-conf.
If we call xmlstarlet on non-existing entries in koha-conf, this may
however result in silently failing scripts (when set -e is in effect).

A function run_safe_xmlstarlet is added for situations where the entry
might not exist. It will not halt execution.

This patch only adjusts koha-functions.sh and modifies the xmlstarlet calls
for dev_install and zebra_loglevels.

Note: The function does not need to check file existence. If the file does
not exist, xmlstarlet warns about it; the function returns empty string,
but does not set an error exit status.

Test plan: See second patch ("koha-plack adjustments").

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Tested in a package installation of master+16749
Comment 43 Mirko Tietgen 2017-02-14 12:21:28 UTC
Created attachment 60194 [details] [review]
Bug 16749: Adjustments for koha-plack

[1] Use run_safe_xmlstarlet for plack workers and requests
[2] Simplify adjust_paths. The lazy export statement is actually enough to
    replace adjust_paths by one direct call to adjust_paths_dev_install.

Test plan:
[1] Copy koha-functions.sh and koha-plack:
    cp [YOUR_PATH]/debian/scripts/koha-functions.sh /usr/share/koha/bin/
    cp [YOUR_PATH]/debian/scripts/koha-plack /usr/sbin/
    where YOUR_PATH might well be /home/vagrant/kohaclone.
[2] Make sure that you have dev_install in koha-conf.
    Stop and start koha-plack. Verify with ps aux|grep plack.
[3] Rename dev_install to nodev_install (in start and end tag).
    Now stop/start koha-plack. Verify with ps aux|grep plack.
[4] Change plack_requests to 51 in your koha-conf.
    Restart Plack and check that you see 51 in ps aux|grep plack.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested on Jessie (Debian VM and Kohadevbox)

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Tested in a package installation of master+16749
Comment 44 Mirko Tietgen 2017-02-14 12:21:36 UTC
Created attachment 60195 [details] [review]
Bug 16749: Adjustments for koha-translate

[AMENDED February 10, 2017]

[1] Added reading /etc/default/koha-common as in the other debian scripts.
    We need it for KOHA_HOME.
[2] Add a -d|--dev parameter for dev installs.
[3] No hardcoded PERL5LIB or KOHA_INSTALL_DIR (KOHA_HOME).
    They are read from default file or set by adjust_paths_dev_install.
[4] Adjust template paths for dev installs: OPAC_TMPL, INTRANET_TMPL.
[5] Remove references to obsolete themes ccsr and prog.

Test plan:
[1] Regular package install:
    Copy koha-translate to /usr/sbin.
    Run koha-translate -l to show installed languages.
    Run koha-translate -l -a to show available languages.
    Add a language: koha-translate -i nl-NL.
    Check template folders in regular location (/usr/share/koha/...)
    Remove a language: koha-translate -r nl-NL. Check again.
[2] Dev install or kohadevbox:
    Copy koha-translate to /usr/sbin.
    If needed, add the <dev_install> line to koha-conf.xml.
    Run koha-translate -l -d yourinstance to show installed languages.
    (Note: You only see the languages installed in this instance.)
    Add a language: koha-translate -i nl-NL -d yourinstance.
    Check template folders in the clone.
    Remove a language: koha-translate -r nl-NL -d yourinstance.

    Note: Make sure you have sufficient file permissions for the kohaclone
    files and koha-conf.xml. On kohadevbox you might need to run sudo
    koha-translate within the the vagrant user context.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested on Jessie (Debian VM and Kohadevbox)

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Tested in a package installation of master+16749
Comment 45 Mirko Tietgen 2017-02-14 12:21:46 UTC
Created attachment 60196 [details] [review]
Bug 16749: Update debian docs for koha-translate

This patch adds the -d option to the documentation (with thanks to
Magnus Enger).

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Tested in a package installation of master+16749
Comment 46 Mirko Tietgen 2017-02-14 12:21:52 UTC
Created attachment 60197 [details] [review]
Bug 16749: Go with a boolean flag, we love booleans

As requested by Jonathan on comment 10 on Bugzilla and supported by Tomas,
we should use the dev_install entry in koha-conf as a boolean flag.
Since we already used it as a path, this patch is not too strict about it.
If the entry is not empty and not equal to "0", we will interpret it as
a true value. The path is taken from the intranetdir entry.

Test plan:
[1] Copy debian/scripts/koha-functions.sh to /usr/share/koha/bin
[2] For a dev install:
    Remove the dev install line, or toggle its value between empty string,
    0 or 1 and each time test stop/start koha-indexer.
    Check the path to rebuild_zebra with ps aux|grep indexer.
    If you have no entry, an empty entry or a zero, you should see a regular
    path.
    (Note: You can do something similar with koha-start-sip.)
[3] For a regular install:
    Remove the dev install line.
    Stop/start koha-indexer or koha-plack, and verify that it still works.
    Add a dev_install line with 0, and repeat stop/start.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Tested in a package installation of master+16749
Comment 47 Marcel de Rooy 2017-02-14 13:19:28 UTC
(In reply to Mirko Tietgen from comment #46)
> Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>

Great, thanks.
Comment 48 Jonathan Druart 2017-02-14 13:39:07 UTC
Why do not we use the dev_install flag for koha-translate?
Comment 49 Tomás Cohen Arazi 2017-02-14 13:55:53 UTC
(In reply to Jonathan Druart from comment #48)
> Why do not we use the dev_install flag for koha-translate?

Why not leave koha-translate alone? Remember there might be dev and non-dev installs. It is easy to just go to the source code and run perl translate install <code>. And koha-translate is system-wide, so not attached to a specific instance
Comment 50 Marcel de Rooy 2017-02-14 14:06:13 UTC
(In reply to Jonathan Druart from comment #48)
> Why do not we use the dev_install flag for koha-translate?

Because we do not pass an instance name when using it for regular installs.
Comment 51 Marcel de Rooy 2017-02-14 14:07:56 UTC
(In reply to Tomás Cohen Arazi from comment #49)
> (In reply to Jonathan Druart from comment #48)
> > Why do not we use the dev_install flag for koha-translate?
> 
> Why not leave koha-translate alone? Remember there might be dev and non-dev
> installs. It is easy to just go to the source code and run perl translate
> install <code>. And koha-translate is system-wide, so not attached to a
> specific instance

It is not that hard to make it work. The patch may prove that.
Any specific objection ?
Comment 52 Tomás Cohen Arazi 2017-02-14 14:15:13 UTC
(In reply to Marcel de Rooy from comment #51)
> (In reply to Tomás Cohen Arazi from comment #49)
> > (In reply to Jonathan Druart from comment #48)
> > > Why do not we use the dev_install flag for koha-translate?
> > 
> > Why not leave koha-translate alone? Remember there might be dev and non-dev
> > installs. It is easy to just go to the source code and run perl translate
> > install <code>. And koha-translate is system-wide, so not attached to a
> > specific instance
> 
> It is not that hard to make it work. The patch may prove that.
> Any specific objection ?

It felt a bit overkill to me, but no objections.
Comment 53 Jonathan Druart 2017-02-16 12:57:35 UTC
Created attachment 60335 [details] [review]
Bug 16749: Use is_instance to make sure the dev instance exist
Comment 54 Jonathan Druart 2017-02-16 13:05:17 UTC
% sudo koha-upgrade-schema
failed to load external entity "/etc/koha/sites/dev_install/koha-conf.xml"
Comment 55 Jonathan Druart 2017-02-16 13:06:46 UTC
(In reply to Jonathan Druart from comment #54)
> % sudo koha-upgrade-schema
> failed to load external entity "/etc/koha/sites/dev_install/koha-conf.xml"

Same for koha-rebuild-zebra, and certainly others.
Comment 56 Marcel de Rooy 2017-02-16 13:08:00 UTC
What instances do you use and what command did you type?
Comment 57 Marcel de Rooy 2017-02-16 13:35:15 UTC
Created attachment 60340 [details] [review]
Bug 16749: Check instancename in adjust_paths_dev_install

Check if the instance does not exist (or was not even passed).
Comment 58 Jonathan Druart 2017-02-16 14:07:41 UTC
Created attachment 60343 [details] [review]
Bug 16749: Adjust xmlstarlet calls in koha-functions

In various scripts we use xmlstarlet to extract values from koha-conf.
If we call xmlstarlet on non-existing entries in koha-conf, this may
however result in silently failing scripts (when set -e is in effect).

A function run_safe_xmlstarlet is added for situations where the entry
might not exist. It will not halt execution.

This patch only adjusts koha-functions.sh and modifies the xmlstarlet calls
for dev_install and zebra_loglevels.

Note: The function does not need to check file existence. If the file does
not exist, xmlstarlet warns about it; the function returns empty string,
but does not set an error exit status.

Test plan: See second patch ("koha-plack adjustments").

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Tested in a package installation of master+16749

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 59 Jonathan Druart 2017-02-16 14:07:45 UTC
Created attachment 60344 [details] [review]
Bug 16749: Adjustments for koha-plack

[1] Use run_safe_xmlstarlet for plack workers and requests
[2] Simplify adjust_paths. The lazy export statement is actually enough to
    replace adjust_paths by one direct call to adjust_paths_dev_install.

Test plan:
[1] Copy koha-functions.sh and koha-plack:
    cp [YOUR_PATH]/debian/scripts/koha-functions.sh /usr/share/koha/bin/
    cp [YOUR_PATH]/debian/scripts/koha-plack /usr/sbin/
    where YOUR_PATH might well be /home/vagrant/kohaclone.
[2] Make sure that you have dev_install in koha-conf.
    Stop and start koha-plack. Verify with ps aux|grep plack.
[3] Rename dev_install to nodev_install (in start and end tag).
    Now stop/start koha-plack. Verify with ps aux|grep plack.
[4] Change plack_requests to 51 in your koha-conf.
    Restart Plack and check that you see 51 in ps aux|grep plack.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested on Jessie (Debian VM and Kohadevbox)

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Tested in a package installation of master+16749

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 60 Jonathan Druart 2017-02-16 14:07:50 UTC
Created attachment 60345 [details] [review]
Bug 16749: Adjustments for koha-translate

[AMENDED February 10, 2017]

[1] Added reading /etc/default/koha-common as in the other debian scripts.
    We need it for KOHA_HOME.
[2] Add a -d|--dev parameter for dev installs.
[3] No hardcoded PERL5LIB or KOHA_INSTALL_DIR (KOHA_HOME).
    They are read from default file or set by adjust_paths_dev_install.
[4] Adjust template paths for dev installs: OPAC_TMPL, INTRANET_TMPL.
[5] Remove references to obsolete themes ccsr and prog.

Test plan:
[1] Regular package install:
    Copy koha-translate to /usr/sbin.
    Run koha-translate -l to show installed languages.
    Run koha-translate -l -a to show available languages.
    Add a language: koha-translate -i nl-NL.
    Check template folders in regular location (/usr/share/koha/...)
    Remove a language: koha-translate -r nl-NL. Check again.
[2] Dev install or kohadevbox:
    Copy koha-translate to /usr/sbin.
    If needed, add the <dev_install> line to koha-conf.xml.
    Run koha-translate -l -d yourinstance to show installed languages.
    (Note: You only see the languages installed in this instance.)
    Add a language: koha-translate -i nl-NL -d yourinstance.
    Check template folders in the clone.
    Remove a language: koha-translate -r nl-NL -d yourinstance.

    Note: Make sure you have sufficient file permissions for the kohaclone
    files and koha-conf.xml. On kohadevbox you might need to run sudo
    koha-translate within the the vagrant user context.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested on Jessie (Debian VM and Kohadevbox)

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Tested in a package installation of master+16749

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 61 Jonathan Druart 2017-02-16 14:07:54 UTC
Created attachment 60346 [details] [review]
Bug 16749: Update debian docs for koha-translate

This patch adds the -d option to the documentation (with thanks to
Magnus Enger).

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Tested in a package installation of master+16749

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 62 Jonathan Druart 2017-02-16 14:07:58 UTC
Created attachment 60347 [details] [review]
Bug 16749: Go with a boolean flag, we love booleans

As requested by Jonathan on comment 10 on Bugzilla and supported by Tomas,
we should use the dev_install entry in koha-conf as a boolean flag.
Since we already used it as a path, this patch is not too strict about it.
If the entry is not empty and not equal to "0", we will interpret it as
a true value. The path is taken from the intranetdir entry.

Test plan:
[1] Copy debian/scripts/koha-functions.sh to /usr/share/koha/bin
[2] For a dev install:
    Remove the dev install line, or toggle its value between empty string,
    0 or 1 and each time test stop/start koha-indexer.
    Check the path to rebuild_zebra with ps aux|grep indexer.
    If you have no entry, an empty entry or a zero, you should see a regular
    path.
    (Note: You can do something similar with koha-start-sip.)
[3] For a regular install:
    Remove the dev install line.
    Stop/start koha-indexer or koha-plack, and verify that it still works.
    Add a dev_install line with 0, and repeat stop/start.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Tested in a package installation of master+16749

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 63 Jonathan Druart 2017-02-16 14:08:02 UTC
Created attachment 60348 [details] [review]
Bug 16749: Use is_instance to make sure the dev instance exist

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 64 Jonathan Druart 2017-02-16 14:08:07 UTC
Created attachment 60349 [details] [review]
Bug 16749: Check instancename in adjust_paths_dev_install

Check if the instance does not exist (or was not even passed).

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 65 Jonathan Druart 2017-02-16 14:08:12 UTC
Created attachment 60350 [details] [review]
Bug 16749: Add is_instance check to koha-upgrade-schema

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 66 Kyle M Hall 2017-02-17 13:21:34 UTC
Pushed to master for 17.05, thanks Marcel, Jonathan!
Comment 67 Katrin Fischer 2017-02-19 20:28:15 UTC
Depends on bug 16733 which is not in 16.11.x.

This won't get ported back to 16.11.x as it is an enhancement.
Comment 68 Katrin Fischer 2017-05-16 05:59:39 UTC
These patches have been pushed to 16.11.x and will be in 16.11.08.
Comment 69 Mason James 2017-05-24 03:06:39 UTC
Blocked by Enhancement, skipping for 16.05.x
Comment 70 Tomás Cohen Arazi 2017-07-20 13:41:49 UTC
*** Bug 16226 has been marked as a duplicate of this bug. ***