As discussed on bug 37303 we do now require yarn for running the translation scripts because of po2json. We need to fix this urgently as it means that production installations updating (and with security releases they should) can no longer install and update languages. We already had several reports in Mattermost and on the mailing list.
Which versions will be affected by this?
(In reply to Magnus Enger from comment #1) > Which versions will be affected by this? Ah, probably the ones that have bug 37303? So 24.11.00 and 24.05.04?
(In reply to Magnus Enger from comment #2) > (In reply to Magnus Enger from comment #1) > > Which versions will be affected by this? > > Ah, probably the ones that have bug 37303? So 24.11.00 and 24.05.04? That should be correct. I am setting keywords.
Created attachment 172714 [details] [review] Bug 38164: Revert "Bug 37303: Replace po2json with a JS version" This reverts commit 4eb981635453871fa2a33396391f3f75a6baa8b1.
Created attachment 172715 [details] [review] Bug 38164: Skip fuzzy translations This patch was envisioned by Julian Maurice. Test plan: 1. Go to `misc/translator/po` 2. es-ES-messages-js.po 3. Mark "Edit" string as fuzzy (around line 48). It should look like: ``` msgid "Edit" msgstr "Editar" ``` 4. Edit ./intranet-main.tt and add the following lines at the bottom, inside the `$(document).ready` block: ```javascript console.log(_("Edit")); console.log(__("Edit")); ``` 5. Install the templates k$ koha-translate --install es-ES --dev kohadev && restart_all 6. Enable *es-ES* by searching for `language` in the sysprefs and switch to it for the staff interface. 7. Go to the Koha home page, open the browser console => FAIL: Notice that the second log in the console is displaying the fuzzy string (i.e. is being translated when it shouldn't) 8. Apply this patch 9. Re-install the translated templates: k$ koha-translate --update es-ES --dev kohadev && restart_all 10. Repeat 7 => SUCCESS: With this patch applied both logs show the English version of the string. 11. Remove the fuzzy flag on `es-ES-messages-js.po` 12. Repeat 9 and 10 => SUCCESS: The results are similar to step 7, but in this case they are expected as the string is not marked fuzzy. 13. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 172756 [details] [review] Bug 38164: Revert "Bug 37303: Replace po2json with a JS version" This reverts commit 4eb981635453871fa2a33396391f3f75a6baa8b1. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Created attachment 172757 [details] [review] Bug 38164: Skip fuzzy translations This patch was envisioned by Julian Maurice. Test plan: 1. Go to `misc/translator/po` 2. es-ES-messages-js.po 3. Mark "Edit" string as fuzzy (around line 48). It should look like: ``` msgid "Edit" msgstr "Editar" ``` 4. Edit ./intranet-main.tt and add the following lines at the bottom, inside the `$(document).ready` block: ```javascript console.log(_("Edit")); console.log(__("Edit")); ``` 5. Install the templates k$ koha-translate --install es-ES --dev kohadev && restart_all 6. Enable *es-ES* by searching for `language` in the sysprefs and switch to it for the staff interface. 7. Go to the Koha home page, open the browser console => FAIL: Notice that the second log in the console is displaying the fuzzy string (i.e. is being translated when it shouldn't) 8. Apply this patch 9. Re-install the translated templates: k$ koha-translate --update es-ES --dev kohadev && restart_all 10. Repeat 7 => SUCCESS: With this patch applied both logs show the English version of the string. 11. Remove the fuzzy flag on `es-ES-messages-js.po` 12. Repeat 9 and 10 => SUCCESS: The results are similar to step 7, but in this case they are expected as the string is not marked fuzzy. 13. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com>
Created attachment 172758 [details] [review] Bug 38164: Revert "Bug 37303: Replace po2json with a JS version" This reverts commit 4eb981635453871fa2a33396391f3f75a6baa8b1. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 172759 [details] [review] Bug 38164: Skip fuzzy translations This patch was envisioned by Julian Maurice. Test plan: 0. Apply the 1st patch (the revert) 1. Go to `misc/translator/po` 2. es-ES-messages-js.po 3. Mark "Edit" string as fuzzy (around line 48). It should look like: ``` msgid "Edit" msgstr "Editar" ``` 4. Edit ./intranet-main.tt and add the following lines at the bottom, inside the `$(document).ready` block: ```javascript console.log(_("Edit")); console.log(__("Edit")); ``` 5. Install the templates k$ koha-translate --install es-ES --dev kohadev && restart_all 6. Enable *es-ES* by searching for `language` in the sysprefs and switch to it for the staff interface. 7. Go to the Koha home page, open the browser console => FAIL: Notice that the second log in the console is displaying the fuzzy string (i.e. is being translated when it shouldn't) 8. Apply this patch 9. Re-install the translated templates: k$ koha-translate --update es-ES --dev kohadev && restart_all 10. Repeat 7 => SUCCESS: With this patch applied both logs show the English version of the string. 11. Remove the fuzzy flag on `es-ES-messages-js.po` 12. Repeat 9 and 10 => SUCCESS: The results are similar to step 7, but in this case they are expected as the string is not marked fuzzy. 13. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 172760 [details] [review] Bug 38164: Revert "Bug 37303: Replace po2json with a JS version" This reverts commit 4eb981635453871fa2a33396391f3f75a6baa8b1. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 172761 [details] [review] Bug 38164: Skip fuzzy translations This patch was envisioned by Julian Maurice. Test plan: 0. Apply the 1st patch (the revert) 1. Go to `misc/translator/po` 2. es-ES-messages-js.po 3. Mark "Edit" string as fuzzy (around line 48). It should look like: ``` msgid "Edit" msgstr "Editar" ``` 4. Edit ./intranet-main.tt and add the following lines at the bottom, inside the `$(document).ready` block: ```javascript console.log(_("Edit")); console.log(__("Edit")); ``` 5. Install the templates k$ koha-translate --install es-ES --dev kohadev && restart_all 6. Enable *es-ES* by searching for `language` in the sysprefs and switch to it for the staff interface. 7. Go to the Koha home page, open the browser console => FAIL: Notice that the second log in the console is displaying the fuzzy string (i.e. is being translated when it shouldn't) 8. Apply this patch 9. Re-install the translated templates: k$ koha-translate --update es-ES --dev kohadev && restart_all 10. Repeat 7 => SUCCESS: With this patch applied both logs show the English version of the string. 11. Remove the fuzzy flag on `es-ES-messages-js.po` 12. Repeat 9 and 10 => SUCCESS: The results are similar to step 7, but in this case they are expected as the string is not marked fuzzy. 13. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Testing notes: 1st apply the reverting patch. > 7. I get - Editar - Editar ok, that's the bug > 10. «With this patch applied both logs show the > English version of the string.» I get - Editar - Edit ????? :o > 12. I get - Editar - Editar ok I looked at the test plan of bug 37303 and it says «second log in the console is displaying the fuzzy string» 😵💫 😵💫 😵💫 😵💫 (the order of _() and __() was the same) It seems the thing is working and there is uncertainly of behavior of __() and _() regarding which is/are being affected by the initial issue. Anyway, it's the same output on main with po2json JS installed. As far as not needing yarn on production it works :) (there is still something suspicious about _() outputting fuzzy strings)
Thanks Aleisha for the signoff, I merged both. As for QA, the yarn.lock file isn't reverted or updated. So we have po2json not in package.json anymore but still in the yarn.lock. And po2json isn't listed as a dependency of anything else. So it's almost certain it's an issue. But xt/verify-yarnlock.t is passing ?!?!?!?!?!?!?!
Created attachment 172762 [details] [review] Bug 38164: Update yarn.lock for removal of po2json.js
Apparently to sync yarn.lock with package.json on can just run `yarn` In our setup in KTD that's: sudo yarn --modules-folder ../node_modules/ (maybe there is a better way to currently use yarn) KTD start works. Translating work.
(In reply to Victor Grousset/tuxayo from comment #12) > (there is still something suspicious about _() outputting fuzzy strings) _("") picks from -staff-prog.po, not -messages-js.po
(In reply to Victor Grousset/tuxayo from comment #14) > Created attachment 172762 [details] [review] [review] > Bug 38164: Update yarn.lock for removal of po2json.js Skipped this patch, this is a task for RM/RMaints.
Created attachment 172766 [details] [review] Bug 38164: Revert "Bug 37303: Replace po2json with a JS version" This reverts commit 4eb981635453871fa2a33396391f3f75a6baa8b1. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 172767 [details] [review] Bug 38164: Skip fuzzy translations This patch was envisioned by Julian Maurice. Test plan: 0. Apply the 1st patch (the revert) 1. Go to `misc/translator/po` 2. es-ES-messages-js.po 3. Mark "Edit" string as fuzzy (around line 48). It should look like: ``` msgid "Edit" msgstr "Editar" ``` 4. Edit ./intranet-main.tt and add the following lines at the bottom, inside the `$(document).ready` block: ```javascript console.log(_("Edit")); console.log(__("Edit")); ``` 5. Install the templates k$ koha-translate --install es-ES --dev kohadev && restart_all 6. Enable *es-ES* by searching for `language` in the sysprefs and switch to it for the staff interface. 7. Go to the Koha home page, open the browser console => FAIL: Notice that the second log in the console is displaying the fuzzy string (i.e. is being translated when it shouldn't) 8. Apply this patch 9. Re-install the translated templates: k$ koha-translate --update es-ES --dev kohadev && restart_all 10. Repeat 7 => SUCCESS: With this patch applied both logs show the English version of the string. 11. Remove the fuzzy flag on `es-ES-messages-js.po` 12. Repeat 9 and 10 => SUCCESS: The results are similar to step 7, but in this case they are expected as the string is not marked fuzzy. 13. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
IMO we are only postponing the problem to the next time we will need yarn. It would be better to try with alternative approaches. But I do understand that we want to provide a quick solution now, however I also know that we won't deal with the yarn issue as we are lacking manpower and so will hit this one again.
A note regarding node-po2json deb package: we don't want it I think, see bug 37303 comment 21 => we don't want to be stuck with v0.4.5
(In reply to Jonathan Druart from comment #20) > IMO we are only postponing the problem to the next time we will need yarn. I think this patch should target 24.05. And we should find a solution for 24.11
(In reply to Tomás Cohen Arazi (tcohen) from comment #22) > (In reply to Jonathan Druart from comment #20) > > IMO we are only postponing the problem to the next time we will need yarn. > > I think this patch should target 24.05. And we should find a solution for > 24.11 That means I would not push it to main, but the RMaints pick it up directly from here. Correct?
(In reply to Katrin Fischer from comment #23) > (In reply to Tomás Cohen Arazi (tcohen) from comment #22) > > (In reply to Jonathan Druart from comment #20) > > > IMO we are only postponing the problem to the next time we will need yarn. > > > > I think this patch should target 24.05. And we should find a solution for > > 24.11 > > That means I would not push it to main, but the RMaints pick it up directly > from here. Correct? Exactly. This was a stopgap for stable.
Handing over to @ping Lucas then! Should we open a separate bug for the 24.11 fix? We need to keep sight of it.
(In reply to Jonathan Druart from comment #17) > (In reply to Victor Grousset/tuxayo from comment #14) > > Created attachment 172762 [details] [review] [review] [review] > > Bug 38164: Update yarn.lock for removal of po2json.js > > Skipped this patch, this is a task for RM/RMaints. Right, that makes sense especially with backports and changes due to yarn versions. Like SCSS and DBIC. Not a problem then. Still something to keep in mind because having to sync yarn.lock after package.json changes isn't mentioned. https://wiki.koha-community.org/wiki/Release_management https://wiki.koha-community.org/wiki/Release_maintenance Especially since xt/verify-yarnlock.t doesn't cover this case (opened Bug 38170 for that) ----- (In reply to Tomás Cohen Arazi (tcohen) from comment #22) > (In reply to Jonathan Druart from comment #20) > > IMO we are only postponing the problem to the next time we will need yarn. > > I think this patch should target 24.05. And we should find a solution for > 24.11 Are we talking about finding a solution about fuzzy strings or "the next time we will need yarn" as in the next time a node package would be of use for us in production? It seems you both weren't in the same issue. As for the fuzzy strings thing, the fixed po2json.pl might be enough. VS po2json.js which has a similar amount of code as our .pl but even 1.0.0-beta-3 has dependencies on fixed versions https://npmgraph.js.org/?q=po2json%401.0.0-beta-3 po2json.pl has also dependencies on stuff that isn't on fixed version but that doesn't look much maintained: https://metacpan.org/pod/JSON https://metacpan.org/pod/Locale::PO Maybe it's ok, it's just interfaces for reasonably simple file formats. Anyway both are not in a great shape themselves and the dependencies they use (.pl => the dependencies themselves, .js => the fact that they are fixed). The biggest difference is po2json.pl can be maintained by us without having to fork it (it already is) and is easier to integrate into Koha (it already is) than a node lib. If we find a non trivial advantage for one, that could easily supersede the above IHMO. Especially if we are sure we will end up having to use node libs in production.
(In reply to Jonathan Druart from comment #20) > IMO we are only postponing the problem to the next time we will need yarn. > > It would be better to try with alternative approaches. But I do understand > that we want to provide a quick solution now, however I also know that we > won't deal with the yarn issue as we are lacking manpower and so will hit > this one again. We shouldn't need "yarn" with a deployed/installed Koha. In the case of bug 37303, "yarn run po2json" wouldn't work anyway, even if "yarn" were installed, since po2json isn't installed in the deployed/installed Koha environment. Plus "yarn run po2json" is just running the "bin/po2json" file which is just being executed using node, which is also not available with a deployed/installed Koha. So at the end of the day what we're really trying to do is run "node <path_to_po2json_proj>/bin/po2json". When we're developing, we need to be thinking more about the deployment environment and not just the development environment.
At this point, I don't know if the Javascript "po2json" is our only option or not. If it is, then the question should be... do we make NodeJS an additional prod (not just dev) dependency of Koha? (Keeping in mind we already have a million dependencies.) For what it's worth, I have a small PHP project where I have added NodeJS as a dependency, because there is some functionality that I can only access using Nodejs. So I do understand the merit of doing that. But... if we did do that... we'd need to think a lot about how we structure the source code. We'd be supporting another complete ecosystem in prod. That's big. If we do this, I'd recommend that we use our existing package.json to *bundle po2json* (or our own customized po2json) into a single script, so that in prod we'd just have 1 file to manage.
Tomas has pointed out bug 38179 to me, so I'll move my discussion over to there, and not clog up this backporting work.
Udpated the bug slightly. We decided on bug 38179 to also use this fix for main. It is also an URGENT bug fix for 24.05, but not below.
Pushed for 24.11! Well done everyone, thank you!
Backported to 24.05.x for upcoming 24.05.05
Depends on Bug 37303 not in 23.11.x
(In reply to Fridolin Somers from comment #33) > Depends on Bug 37303 not in 23.11.x You need to pick the second patch only.