Summary: | Remove pl-PL installer data | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Installation and upgrade (web-based installer) | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Joonas Kylmälä <joonas.kylmala> |
Severity: | minor | ||
Priority: | P5 - low | CC: | bgkriegel, fridolin.somers, gmcharlt, jonathan.druart, joonas.kylmala, koha-bugs, lucas, martin.renvoize, testopia, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
URL: | https://gitlab.com/joubu/Koha/-/tree/bug_27623 | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: |
This removes the SQL installer directory from the Koha source code. Installing Polish sample data will still be possible, but translations have been moved into Koha's translation system using .po files maintained on https://translate.koha-community.org.
|
Version(s) released in: |
21.05.00
|
Circulation function: | |||
Bug Depends on: | 24871, 24973, 28154, 28281, 28291 | ||
Bug Blocks: | 27829 |
Description
Jonathan Druart
2021-02-04 08:59:09 UTC
I've worked through the patch and it makes sense to me.. Signing off (Will leave Jonathan to add the SO line to the branch) pl-PL appears to be a mostly untranslated copy of the English files. I think the only thing we should think about for a custom.sql here is active currency and adding PLN. -INSERT INTO `currency` (currency, rate, symbol, active) VALUES -('PLN', 1.0, 'zł', '1'), Missing change to xt/sample_notices.t Added Martin's signed-off-by line to the first commit, Added 2 new commits: Bug 27623: Fix sample_notices.t Bug 27623: Add PLN currency to custom.sql Doesn't apply anymore (In reply to Joonas Kylmälä from comment #6) > Doesn't apply anymore It was actually easy to fix this rebase issue: > git rm installer/data/mysql/pl-PL/mandatory/sample_notices.sql > git am --continue However, in the installer I get > Cannot decode string with wide characters at /usr/lib/x86_64-linux-gnu/perl> /5.24/Encode.pm line 202 > in Encode::decode at /usr/lib/x86_64-linux-gnu/perl/5.24/Encode.pm line 202 and also if following the test plan I get: > $ ./translate update pl-PL > misc/translator/Koha-pref.pot:5085:37: invalid multibyte sequence not sure if caused by this change or it got broken already before, but it makes testing this patch not possible. (In reply to Joonas Kylmälä from comment #7) > (In reply to Joonas Kylmälä from comment #6) > > Doesn't apply anymore > > It was actually easy to fix this rebase issue: > > > git rm installer/data/mysql/pl-PL/mandatory/sample_notices.sql > > git am --continue > > However, in the installer I get > > > Cannot decode string with wide characters at /usr/lib/x86_64-linux-gnu/perl> /5.24/Encode.pm line 202 > > in Encode::decode at /usr/lib/x86_64-linux-gnu/perl/5.24/Encode.pm line 202 Which step, I didn't recreate that. > and also if following the test plan I get: > > > $ ./translate update pl-PL > > misc/translator/Koha-pref.pot:5085:37: invalid multibyte sequence > > not sure if caused by this change or it got broken already before, but it > makes testing this patch not possible. I am seeing that Looks like it has been caused by bug 22824. I am investigating it. (In reply to Jonathan Druart from comment #8) > (In reply to Joonas Kylmälä from comment #7) > Which step, I didn't recreate that. First run following commands: > $ koha-mysql kohadev < installer/data/mysql/kohastructure.sql > $ ./translate install pl-PL Then open the installer and select pl-PL as the language, then after few pages the error happens after clicking OK in the MARC21/UNIMARC selection page: http://localhost:8081/cgi-bin/koha/installer/install.pl?step=3&op=choosemarc Should have been fixed by bug 28154. Joonas, Katrin, can I get a QA stamp please? (In reply to Jonathan Druart from comment #11) > Joonas, Katrin, can I get a QA stamp please? Still the same problem with installer so cannot test this. > Still the same problem with installer so cannot test this.
Odd, I was able to complete the installer with the branch directly and with a rebase on master. Env: koha-testing-docker
Note, there is a conflict when rebasing. Some recent commits modified one of the deleted files. So their changes might need to be moved somewhere (if these changes are still needed)
Rechecked, seems there is no issue => signed off! Victor what were the exact steps you used, I still get the error, I even deleted koha-testing-docker (Debian GNU/Linux 9 version) completely and re-created it from scratch and I still get the error after:
$ koha-mysql kohadev < installer/data/mysql/kohastructure.sql
$ restart_all
$ ./translate update pl-PL
$ ./translate install pl-PL
$ restart_all
--> Go to installer page and select pl-PL as installer language, click through it and notice it gives error:
> Cannot decode string with wide characters at /usr/lib/x86_64-linux-gnu/perl/5.24/Encode.pm line 202
in the MARC21 step.
I found the cause and opened bug 28281. I will work on a fix (or I already have but missing test). I'm still unable to test this even after bug 28281 because misc/translator/LangInstaller.pm / ./translate script double *encodes* the output of installer/data/mysql/pl-PL/marcflavour/marc21/mandatory/authorities_normal_marc21.yml I tried to now change it like this: - open(my $fh, ">:encoding(UTF-8)", "$intradir/$tdir/$file"); - YAML::XS::DumpFile( $fh, $translated_yaml ); - close($fh); but that still produces broken UTF-8 in the file: installer/data/mysql/pl-PL/marcflavour/marc21/mandatory/authorities_normal_marc21.yml NOTE: Please be careful and remember to use "git clean -d -f" and git reset --hard HEAD before testing this bug. Otherwise the ./translate script doesn't update the translation files. (In reply to Joonas Kylmälä from comment #17) > I tried to now change it like this: > > - open(my $fh, ">:encoding(UTF-8)", > "$intradir/$tdir/$file"); > - YAML::XS::DumpFile( $fh, $translated_yaml ); > - close($fh); err, missed the "+" part of the diff, I made it to look like: + YAML::XS::DumpFile( "$intradir/$tdir/$file", $translated_yaml ); ah, there is even a third bug! I have a fix now for that as well, will open new bug report and make this depends on that. I have now fix for that as well in bug 28291 but tests still missing... Hm, this might be the same problem Victor already saw, but using the branch, I got stuck at this: YAML::XS::Load Error: The problem: control characters are not allowed was found at document: 0 at /usr/lib/x86_64-linux-gnu/perl5/5.24/YAML/XS.pm line 51 in YAML::XS::LoadFile at /usr/lib/x86_64-linux-gnu/perl5/5.24/YAML/XS.pm line 51 Trying again with 28281 applied as well, but got stuck with the same error again :( Help? (In reply to Katrin Fischer from comment #22) > Hm, this might be the same problem Victor already saw, but using the branch, Which problem? It seems I didn't get a problem each time I tried. (In reply to Katrin Fischer from comment #23) > Trying again with 28281 applied as well, but got stuck with the same error > again :( > > Help? You need to have all those dependencies listed in the bug report here, not just 28281. Rebasing the patches on top of master is your best bet I think, it contains already those. But note I have not yet tried if it works still even after all those fixes because I have not yet went through the full installer after those bugs being fixed. Also please make sure to run "git clean -d -f" after your testing to make sure the translation files get updated. (In reply to Joonas Kylmälä from comment #25) > (In reply to Katrin Fischer from comment #23) > > Trying again with 28281 applied as well, but got stuck with the same error > > again :( > > > > Help? > > You need to have all those dependencies listed in the bug report here, not > just 28281. Rebasing the patches on top of master is your best bet I think, > it contains already those. But note I have not yet tried if it works still > even after all those fixes because I have not yet went through the full > installer after those bugs being fixed. Also please make sure to run "git > clean -d -f" after your testing to make sure the translation files get > updated. I've run the translation, but was probably missing other dependencies as you suggest. As this is on a branch, I think it'd be easier to have Joubu rebase as I can't attach a fixed patch set. Remote branch rebased against master. Passing QA: Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Pushed to master for 21.05, thanks to everybody involved! Not pushed to 20.11.x |