Bug 32356 - xx-XX installer dir /kohadevbox/koha/installer/data/mysql/xx-XX already exists.
Summary: xx-XX installer dir /kohadevbox/koha/installer/data/mysql/xx-XX already exists.
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: I18N/L10N (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 24262
Blocks:
  Show dependency treegraph
 
Reported: 2022-11-28 20:41 UTC by Katrin Fischer
Modified: 2023-12-28 20:44 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.03,22.05.10,21.11.18


Attachments
Bug 32356: Install installer translated files on update (1.42 KB, patch)
2022-11-29 09:26 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 32356: Install installer translated files on update (1.47 KB, patch)
2022-12-04 12:40 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32356: Install installer translated files on update (1.48 KB, patch)
2022-12-04 13:08 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32356: Install installer translated files on update (1.67 KB, patch)
2023-02-09 12:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2022-11-28 20:41:22 UTC
When installing a language that has already been installed before you'll end up with this error:

fr-FR installer dir /kohadevbox/koha/installer/data/mysql/fr-FR already exists.

This is a problem, as the files in the directory won't be replaced with their updated versions or files will be missing:

Hadn't been installed before:
kohadev-koha@kohadevbox:/kohadevbox/koha/misc/translator$ ls -l ../../installer/data/mysql/es-ES
total 12
drwxr-xr-x 2 kohadev-koha kohadev-koha 4096 Nov 28 20:25 mandatory
drwxr-xr-x 4 kohadev-koha kohadev-koha 4096 Nov 28 20:25 marcflavour
drwxr-xr-x 2 kohadev-koha kohadev-koha 4096 Nov 28 20:25 optional


Had been installed before:
kohadev-koha@kohadevbox:/kohadevbox/koha/misc/translator$ ls -l ../../installer/data/mysql/fr-FR/
total 0

There have been reports on problems with installing/updating languages on the mailing list and IRC:

https://lists.katipo.co.nz/pipermail/koha/2022-November/058691.html
https://lists.koha-community.org/pipermail/koha-devel/2022-November/047398.html
http://irc.koha-community.org/koha/2022-11-28#i_2464321

Something is not going well here.

Another thing:
The start page looks ok for es-ES but appears broken for de-DE in my test. Still investigating, but also wondering if it's another file related issue.
Comment 1 Jonathan Druart 2022-11-29 08:25:15 UTC
How to recreate the problem? I've run twice translate install fr-FR and don't see the error.
Comment 2 Katrin Fischer 2022-11-29 08:30:37 UTC
I think it should be visible when you run the commands koha-translate --update language and the language is already installled. It also appeared for me when I used the old way: kshell, cd misc/translator, perl translate install de-DE. I always had the message.
Comment 3 Katrin Fischer 2022-11-29 08:32:24 UTC
On IRC I also talked to Tylathos in PM and he showed me the same error message on his package installation.
Comment 4 Jonathan Druart 2022-11-29 09:07:47 UTC
I think there are several things here.

The de-DE interface is broken because of "% l: Unknown or unsupported format specification"

To get the origin of the error:
% junitmsgfmt misc/translator/po/de-DE*

 <failure message="msgfmt failure">misc/translator/po/de-DE-staff-prog.po:12520: 'msgstr' is not a valid C format string, unlike 'msgid'. Reason: The character that terminates the directive number 2 is not a valid conversion specifier.
 misc/translator/po/de-DE-staff-prog.po:66878: 'msgstr' is not a valid C format string, unlike 'msgid'. Reason: The string ends in the middle of a directive.
 msgfmt: found 2 fatal errors


12519 msgid "Are you sure you want to delete \"%s\"?"
12520 msgstr "Sind Sie sicher, dass Sie \"%s% löschen möchten?"
Comment 5 Katrin Fischer 2022-11-29 09:10:50 UTC
(In reply to Jonathan Druart from comment #4)
> I think there are several things here.
> 
> The de-DE interface is broken because of "% l: Unknown or unsupported format
> specification"
> 
> To get the origin of the error:
> % junitmsgfmt misc/translator/po/de-DE*
> 
>  <failure message="msgfmt
> failure">misc/translator/po/de-DE-staff-prog.po:12520: 'msgstr' is not a
> valid C format string, unlike 'msgid'. Reason: The character that terminates
> the directive number 2 is not a valid conversion specifier.
>  misc/translator/po/de-DE-staff-prog.po:66878: 'msgstr' is not a valid C
> format string, unlike 'msgid'. Reason: The string ends in the middle of a
> directive.
>  msgfmt: found 2 fatal errors
> 
> 
> 12519 msgid "Are you sure you want to delete \"%s\"?"
> 12520 msgstr "Sind Sie sicher, dass Sie \"%s% löschen möchten?"

Yes, I already fixed these. But I also had display problems with other installed languages. It was fr-FR - maybe also some errors there. Let's keep the display issue separate from the files not being updated in the installer directory.
Comment 6 Jonathan Druart 2022-11-29 09:19:10 UTC
> https://lists.katipo.co.nz/pipermail/koha/2022-November/058691.html

Aleisha, did you manage to recreate the problem in ktd?
Comment 7 Jonathan Druart 2022-11-29 09:26:03 UTC
Created attachment 144311 [details] [review]
Bug 32356: Install installer translated files on update

If the installer files exist for a given language, the translate script
won't update it.
We should get a confirmation from Bernardo (author of bug 24262), but I
don't understand why it could be needed (side-effects?)

Test plan:
Installer several times the same language, drop the DB and run the
installer+onboarding process.
Check files installed by the installer (yaml for notice templates,
biblio frameworks) have inserted the data properly in DB.
Comment 8 Jonathan Druart 2022-11-29 09:27:16 UTC
This patch might fix Michael's problem, but it shouldn't fix Aleisha's.
Comment 9 Katrin Fischer 2022-12-04 12:40:04 UTC
Created attachment 144418 [details] [review]
Bug 32356: Install installer translated files on update

If the installer files exist for a given language, the translate script
won't update it.
We should get a confirmation from Bernardo (author of bug 24262), but I
don't understand why it could be needed (side-effects?)

Test plan:
Installer several times the same language, drop the DB and run the
installer+onboarding process.
Check files installed by the installer (yaml for notice templates,
biblio frameworks) have inserted the data properly in DB.

Signed-off-by: Katrin <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2022-12-04 13:08:34 UTC
Created attachment 144422 [details] [review]
Bug 32356: Install installer translated files on update

If the installer files exist for a given language, the translate script
won't update it.
We should get a confirmation from Bernardo (author of bug 24262), but I
don't understand why it could be needed (side-effects?)

Test plan:
Installer several times the same language, drop the DB and run the
installer+onboarding process.
Check files installed by the installer (yaml for notice templates,
biblio frameworks) have inserted the data properly in DB.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Tomás Cohen Arazi 2022-12-15 13:02:51 UTC
Isn't this a permissions problem because people are using koha-translate as root?
Comment 12 Tomás Cohen Arazi 2022-12-15 13:06:12 UTC
koha-translate --update removes the translated templates before re-generating:

136        if print_installed | grep -q $lang; then
137            # Check po files are present
138            check_lang_po_files $lang
139            remove_lang $lang
140            install_lang $lang
Comment 13 Jonathan Druart 2022-12-15 14:02:27 UTC
(In reply to Tomás Cohen Arazi from comment #12)
> koha-translate --update removes the translated templates before
> re-generating:
> 
> 136        if print_installed | grep -q $lang; then
> 137            # Check po files are present
> 138            check_lang_po_files $lang
> 139            remove_lang $lang
> 140            install_lang $lang

I cannot remember, maybe this patch fixes the problem if you are not using koha-translate?

It's not consistent to skip installer if the lang directory exist, and not doing it in other LangInstaller::install_* subroutines (install_prefs, install_tmpl, install_messages) anyway.
Comment 14 Marcel de Rooy 2022-12-16 09:28:57 UTC
(In reply to Tomás Cohen Arazi from comment #11)
> Isn't this a permissions problem because people are using koha-translate as
> root?

I did run misc/translate/translator with root until now without problems.. Being lazy, and note my container root actually is quite limited.
Comment 15 Marcel de Rooy 2022-12-16 09:29:13 UTC
Here is a dumb question but couldnt we add TESTS for this module too ?

Interesting that we have misc/translator/translate and debian/scripts/koha-translate. Shouldnt the latter one only be a very thin wrapper around the first one? Currently, it is a bit more. It implements an update option that we are missing in LangInstaller although the POD claims that it is present (but proposing something else):
=head2 update
For the current language, update .po files.

sub install_installer
=> Nothng better than choosing a good subroutine name :)

sub install_messages
mkdir "$progdir/$self->{lang}/js";
=> No check here ?

Reading the comments, I think that we still need feedback from a few people (Bernardo, Aleisha, maybe others). And it feels like only removing the -d code is a bit of trial and error. This area of Koha (also) needs a bit more attention than that?
Comment 16 Katrin Fischer 2022-12-16 09:41:03 UTC
(In reply to Tomás Cohen Arazi from comment #12)
> koha-translate --update removes the translated templates before
> re-generating:
> 
> 136        if print_installed | grep -q $lang; then
> 137            # Check po files are present
> 138            check_lang_po_files $lang
> 139            remove_lang $lang
> 140            install_lang $lang

This diddn't work for me.
Comment 17 Tomás Cohen Arazi 2022-12-16 13:04:25 UTC
We need to decide which problem is the one we are solving here. And I don't think `koha-translate` should be used on KTD or a packages install. Just plain `perl translate...`

Different scenarios seem to be mixed in the conversation.

Regarding running as root vs. not doing so, it is clear to me that if you use the root user for all the things related to the repo (branches, new files as generated by perl translate, etc) you won't see any problems. If you work outside the container with your regular user, on the repo, then you will notice git repo problems (e.g. 'fatal: detected dubious ownership in repository').
Comment 18 Tomás Cohen Arazi 2022-12-16 13:24:10 UTC
For the record:

root@kohadevbox:kohadevbox$ kshell
kohadev-koha@kohadevbox:/kohadevbox/koha$ cd misc/translator/
kohadev-koha@kohadevbox:/kohadevbox/koha/misc/translator$ perl translate install fr-FR
kohadev-koha@kohadevbox:/kohadevbox/koha/misc/translator$ perl translate install fr-FR
kohadev-koha@kohadevbox:/kohadevbox/koha/misc/translator$ perl translate install fr-FR
kohadev-koha@kohadevbox:/kohadevbox/koha/misc/translator$
Comment 19 Katrin Fischer 2022-12-18 23:12:56 UTC
What about the --dev option for koha-translate?
Comment 20 Jonathan Druart 2023-02-09 10:34:43 UTC
I really don't understand why this has not been pushed already.

(In reply to Jonathan Druart from comment #13)
> It's not consistent to skip installer if the lang directory exist, and not
> doing it in other LangInstaller::install_* subroutines (install_prefs,
> install_tmpl, install_messages) anyway.

It's obvious what's happening here, the files are not updated when they exist already, we skip the "installer" task!

To recreate:
- fr-FR and es-ES not installed
% perl translate install fr-FR
% grep -r Maybe ../../installer/data/mysql/fr-FR/
=> There is not occurrence of "Maybe" in fr-FR!
# Add it:
% vim $SYNC_REPO/installer/data/mysql/en/mandatory/auth_values.yml
Add a new entry
  38         - category: "YES_NO"
  39           authorised_value: "1"
  40           lib: "Maybe"
  41           lib_opac: "Maybe"

% perl translate install es-ES
% grep -r Maybe ../../installer/data/mysql/es-ES
../../installer/data/mysql/es-ES/mandatory/auth_values.yml:      lib: Maybe
../../installer/data/mysql/es-ES/mandatory/auth_values.yml:      lib_opac: Maybe

=> OK!

% perl translate install fr-FR
% grep -r Maybe ../../installer/data/mysql/fr-FR/
=> KO, It's not updated!
Comment 21 Tomás Cohen Arazi 2023-02-09 11:47:05 UTC
(In reply to Jonathan Druart from comment #20)
> I really don't understand why this has not been pushed already.

I think it is because of the confusion on the report and what's being actually fixed.

- file already exists error (which my last post highlights I wasn't able to reproduce)
- no, it was koha-translate (which deletes before re-generating, so that's not a real issue)
- asking for a --dev option for koha-translate (maybe but that's another feature, I guess?)
Comment 22 Tomás Cohen Arazi 2023-02-09 12:54:15 UTC
Created attachment 146426 [details] [review]
Bug 32356: Install installer translated files on update

If the installer files exist for a given language, the translate script
won't update it.
We should get a confirmation from Bernardo (author of bug 24262), but I
don't understand why it could be needed (side-effects?)

Test plan:
Installer several times the same language, drop the DB and run the
installer+onboarding process.
Check files installed by the installer (yaml for notice templates,
biblio frameworks) have inserted the data properly in DB.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 23 Tomás Cohen Arazi 2023-02-09 13:01:32 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 24 Jacob O'Mara 2023-02-13 09:46:07 UTC
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.
Comment 25 Lucas Gass 2023-02-15 21:51:49 UTC
Backported to 22.05.x for upcoming 22.05.10
Comment 26 Arthur Suzuki 2023-02-17 13:08:48 UTC
applied to 21.11.x for 21.11.18
Comment 27 wainuiwitikapark 2023-03-15 02:18:53 UTC
Not backported to 21.05.x