Bug 41223 - Improve translation process
Improve translation process
Status: CLOSED FIXED
Product: UCS
Classification: Unclassified
Component: General
UCS 4.1
Other Linux
: P5 normal (vote)
: UCS 4.1-2-errata
Assigned To: Eduard Mai
Alexander Kläser
:
Depends on:
Blocks: 41912
  Show dependency treegraph
 
Reported: 2016-05-09 16:06 CEST by Alexander Kläser
Modified: 2016-09-21 20:38 CEST (History)
3 users (show)

See Also:
What kind of report is it?: Feature Request
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): Internationalization
Max CVSS v3 score:


Attachments
Welsh translation package (622.73 KB, application/x-gzip)
2016-07-15 16:45 CEST, Alexander Kläser
Details
WIP (24.54 KB, patch)
2016-07-25 20:35 CEST, Eduard Mai
Details | Diff
WIP (28.96 KB, patch)
2016-07-26 19:19 CEST, Eduard Mai
Details | Diff
Implemented both testcases as proposed. All issues from comments were adressed. (2.83 KB, patch)
2016-07-27 18:30 CEST, Eduard Mai
Details | Diff
WIP (9.50 KB, patch)
2016-07-27 18:34 CEST, Eduard Mai
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Kläser univentionstaff 2016-05-09 16:06:31 CEST
It would be nice to improve the translation process in the following way:

* Before building a package, it is currently necessary to manually call 
  univention-ucs-translation-update-target-files.py. It would be nice if the 
  package build did this automatically as this step is somewhat confusing.

* An update script that automatically checks out the current SVN branch and 
  updates the po files would be very helpful.

[Ticket#2016031421000382]
Comment 1 Eduard Mai univentionstaff 2016-07-12 16:11:00 CEST
Rev. 70946: debian/changelog
Rev. 70943: Simplification of the translation process.

The script univention-ucs-translation-build-package now yields a source package, which can built directly after translation.

Package: univention-ucs-translation-template
Version: 3.0.1-8.11.201607121545
Comment 2 Florian Best univentionstaff 2016-07-13 08:43:48 CEST
There is some Code which does nothing. Better would be printing a error message:
svn r70943
+       except dh_umc.Error as exc:
+               repr(exc)
+       except TypeError as exc:
+               repr(exc)

In translationhelper.py is a lot of tab/space mixes.

It's better to define a own exception than using AttributeError - which might happens also accidentally.
Comment 3 Florian Best univentionstaff 2016-07-13 08:44:09 CEST
(In reply to Eduard Mai from comment #1)
> The script univention-ucs-translation-build-package now yields a source
> package, which can built directly after translation.
This sounds very nice and useful.
Comment 4 Eduard Mai univentionstaff 2016-07-15 15:57:10 CEST
Rev. 71030:
univention-ucs-translation-merge merges new messages from a local Subversion directory into an existing translation source package(univention-ucs-translation-*). It detects and removes files obsoleted by SVN changes.

Package: univention-ucs-translation-template
Version: 3.0.1-9.12.201607151539

There still is room for improvement. Especially regarding error handling, as comment #2 already stated.
Comment 5 Alexander Kläser univentionstaff 2016-07-15 16:29:33 CEST
There are some TODO comments and some parts commented out.
univention-ucs-translation-merge is not installed, as an entry in debian/*.install is missing.

→ REOPEN
Comment 6 Alexander Kläser univentionstaff 2016-07-15 16:30:51 CEST
... and univention-ucs-translation-build-package.py vs. univention-ucs-translation-merge. Could you remove .py for both file names?
Comment 7 Alexander Kläser univentionstaff 2016-07-15 16:32:33 CEST
Is this output correct?

> ./management/univention-management-console-module-adtakeover/univention-management-console-module-adtakeover.umc-modules
> could not obtain packagename from directory ./management/univention-management-console-module-adtakeover/univention-management-console-module-adtakeover.umc-modules
Comment 8 Alexander Kläser univentionstaff 2016-07-15 16:33:36 CEST
> /root/translation/ucs-4.1-1/management/univention-management-console-frontend/umc/dialog/LoginDialog.js:450: Warnung: Zeichenkette nicht korrekt terminiert
> Traceback (most recent call last):
>   File "/usr/bin/univention-ucs-translation-merge", line 108, in <module>
>     merge_po_file_trees(output_dir, os.path.join(args.translation, args.lang_code))
>   File "/usr/bin/univention-ucs-translation-merge", line 71, in merge_po_file_trees
>     upstream = polib.pofile(po_file.replace(target_tree, source_tree))
>   File "/usr/lib/python2.7/dist-packages/polib.py", line 108, in pofile
>     return _pofile_or_mofile(pofile, 'pofile', **kwargs)
>   File "/usr/lib/python2.7/dist-packages/polib.py", line 74, in _pofile_or_mofile
>     instance = parser.parse()
>   File "/usr/lib/python2.7/dist-packages/polib.py", line 1280, in parse
>     (self.instance.fpath, i))
> IOError: Syntax error in po file None (line 1)
Comment 9 Alexander Kläser univentionstaff 2016-07-15 16:35:41 CEST
(In reply to Alexander Kläser from comment #8)
> > /root/translation/ucs-4.1-1/management/univention-management-console-frontend/umc/dialog/LoginDialog.js:450: Warnung: Zeichenkette nicht korrekt terminiert
> > Traceback (most recent call last):
> [...]

Hm, when running a second time, it went fine.
Comment 10 Alexander Kläser univentionstaff 2016-07-15 16:45:55 CEST
Created attachment 7809 [details]
Welsh translation package

(In reply to Alexander Kläser from comment #8)
> > /root/translation/ucs-4.1-1/management/univention-management-console-frontend/umc/dialog/LoginDialog.js:450: Warnung: Zeichenkette nicht korrekt terminiert
> > Traceback (most recent call last):
> [...]

Attached a Welsh translation package that provokes this error.
Comment 11 Alexander Kläser univentionstaff 2016-07-15 16:58:20 CEST
(In reply to Alexander Kläser from comment #10)
> Created attachment 7809 [details]
> Welsh translation package
> 
> (In reply to Alexander Kläser from comment #8)
> > > /root/translation/ucs-4.1-1/management/univention-management-console-frontend/umc/dialog/LoginDialog.js:450: Warnung: Zeichenkette nicht korrekt terminiert
> > > Traceback (most recent call last):
> > [...]
> 
> Attached a Welsh translation package that provokes this error.

This package has been created with the old tool.

More points:

* No Makefile has been generated. I guess the Makefile should be regenerated in any case (e.g., also when files have been added to the SVN).

* A warning message in the Makefile would be nice (à la "This file has been auto-generated by...").

* The ??_merge directory should be removed.

* The absolute path is ideal, it should be relative to the package directory
> #: /root/translation/ucs-4.1-1/management/univention-directory-manager-modules/modules/univention/admin/handlers/legacy/computers/managedclient.py:435
> msgid " At least posix or kerberos is required."
> msgstr " Ar POSIX neu Kerberos lleiaf sydd ei angen."

* Please check the special case for the UMC frontend... univention-ucs-translation-cy/cy/management/univention-management-console-frontend/umc/cy.po has been removed and univention-ucs-translation-cy/cy/management/univention-management-console-frontend/usr/share/univention-management-console-frontend/js/umc/i18n/cy/cy.po has been added.
Comment 12 Alexander Kläser univentionstaff 2016-07-15 17:05:18 CEST
I changed in some translation in management/univention-management-console-module-top/umc/js/top.js. When running the merge tool, I obtain:

> #: umc/js/top.js:61
> #, python-format
> msgid "Please confirm sending %s to the selected process BAZBAZ!"
> msgstr ""
> 
> #~ msgid "Please confirm sending %s to the selected process!"
> #~ msgstr "Cadarnhewch anfon %s at y broses a ddewiswyd!"

Expected result:

> #: umc/js/top.js:61
> #, fuzzy, python-format
> msgid "Please confirm sending %s to the selected process BAZBAZ!"
> msgstr "Cadarnhewch anfon %s at y broses a ddewiswyd!"
Comment 13 Alexander Kläser univentionstaff 2016-07-15 17:14:19 CEST
The Makefile contains some comments:

> %.json:
>     #python -c "import univention.dh_umc as dhumc; dhumc.create_json_file('$<')"
>     dh-umc-po2json "$<"  # creates .json file in the same directory as the .po file
>     mkdir -p $(@D)
>     mv "$(patsubst %.po,%.json,$<)" "$@"
>     #install -D $($<:.po=.json) "$@"
Comment 14 Alexander Kläser univentionstaff 2016-07-15 17:15:31 CEST
I would expect the build process to fail in case there are any fuzzy translations (as they will simply be ignored when the translation package is being installed).
Comment 15 Alexander Kläser univentionstaff 2016-07-15 17:23:35 CEST
I suggest to split Makefile into Makefile + dependencies.mk:
---------- Makfile ----------
#!/usr/bin/make -f

include dependencies.mk

%.mo:
        mkdir -p $(@D)
        msgfmt --check --output-file="$@" "$<"

%.json:
        #python -c "import univention.dh_umc as dhumc; dhumc.create_json_file('$<')"
        dh-umc-po2json "$<"  # creates .json file in the same directory as the .po file
        mkdir -p $(@D)
        mv "$(patsubst %.po,%.json,$<)" "$@"
        #install -D $($<:.po=.json) "$@"

build:

install: $(ALL_TARGETS)
---------- 8< ----------

---------- dependencies.mk ----------
#!/usr/bin/make -f

ALL_TARGETS = $(DESTDIR)/usr/share/locale/sp/LC_MESSAGES/ucs-test-umc-module.mo \
        $(DESTDIR)/usr/share/univention-management-console-frontend/js/umc/modules/i18n/sp/ucstest.json \
        $(DESTDIR)/usr/share/univention-management-console/i18n/sp/ucstest.mo \
        $(DESTDIR)/usr/share/locale/sp/LC_MESSAGES/univention-management-console-module-updater.mo \
[...]
$(DESTDIR)/usr/share/locale/sp/LC_MESSAGES/univention-admin-handlers-users.mo: sp/management/univention-directory-manager-modules/modules/univention/admin/handlers/users/sp.po
$(DESTDIR)/usr/share/locale/sp/LC_MESSAGES/univention-admin-handlers-container-msgpo.mo: sp/services/univention-s4-connector/modules/univention/admin/handlers/container/msgpo/sp.po
$(DESTDIR)/usr/share/locale/sp/LC_MESSAGES/univention-admin-handlers-saml.mo: sp/saml/univention-saml/modules/univention/admin/handlers/saml/sp.po
---------- 8< ----------
Comment 16 Alexander Kläser univentionstaff 2016-07-15 17:24:21 CEST
(In reply to Alexander Kläser from comment #15)
> I suggest to split Makefile into Makefile + dependencies.mk:
> [...]

... then you only would need to auto and re-generate dependencies.mk + Makfile could be adjusted manually.
Comment 17 Alexander Kläser univentionstaff 2016-07-15 17:41:43 CEST
Another traceback that I come across (this package was built with the new tool):
> /root/translation/ucs-4.1-1/management/univention-management-console-frontend/umc/dialog/LoginDialog.js:450: Warnung: Zeichenkette nicht korrekt terminiert
> Traceback (most recent call last):
>   File "/usr/bin/univention-ucs-translation-merge", line 108, in <module>
>     merge_po_file_trees(output_dir, os.path.join(args.translation, args.lang_code))
>   File "/usr/bin/univention-ucs-translation-merge", line 65, in merge_po_file_trees
>     os.makedirs(os.path.dirname(new_po_path))
>   File "/usr/lib/python2.7/os.py", line 157, in makedirs
>     mkdir(name, mode)
> OSError: [Errno 17] File exists: '/root/translation/univention-ucs-translation-sp/cy/base/univention-firewall/umc'
Comment 18 Alexander Kläser univentionstaff 2016-07-15 17:45:02 CEST
(In reply to Alexander Kläser from comment #17)
> Another traceback that I come across (this package was built with the new
> tool):
> > /root/translation/ucs-4.1-1/management/univention-management-console-frontend/umc/dialog/LoginDialog.js:450: Warnung: Zeichenkette nicht korrekt terminiert
> [...]

My error, I used as language "cy" instead of "sp" when calling univention-ucs-translation-merge. Maybe an upfront check for the directory "sp" would be nice here.
Comment 19 Alexander Kläser univentionstaff 2016-07-15 18:07:33 CEST
As discussed, could you prepare one ucs-test script with the following checks:
* SVN checkout
* Build translation package + add fake translations
* Change randomly translation strings in js/py files of the SVN directory
* Merge the changes into the translation package
  → Check whether fuzzy entries have been generated
* remove fuzzy tags
* SVN reset
* Merge the changes into the translation package
  → Check whether fuzzy entries have been generated
* remove fuzzy tags
  → State should be identical to initial translation package

... and one with the following other test case:
* SVN checkout
* Build translation package + add fake translations
* Add dummy UMC module with translations
* Merge the changes into the translation package
  → New .po files should exist + Makefile should contain a new dependency
* SVN reset
* Merge the changes into the translation package
  → .po files should still exist + Makefile dependency needs to be removed

For a first step, this seems to be sufficient as a test.
Comment 20 Eduard Mai univentionstaff 2016-07-21 20:20:50 CEST
(In reply to Alexander Kläser from comment #7)
> Is this output correct?
> 
> > ./management/univention-management-console-module-adtakeover/univention-management-console-module-adtakeover.umc-modules
> > could not obtain packagename from directory ./management/univention-management-console-module-adtakeover/univention-management-console-module-adtakeover.umc-modules

$ find management/univention-management-console-module-adtakeover -name '*.umc-modules'
management/univention-management-console-module-adtakeover/debian/univention-management-console-module-adtakeover.umc-modules
management/univention-management-console-module-adtakeover/univention-management-console-module-adtakeover.umc-modules

AFAIS the second one is a stray file and should be removed. The output is correct.
Comment 21 Eduard Mai univentionstaff 2016-07-25 20:35:04 CEST
Created attachment 7832 [details]
WIP
Comment 22 Eduard Mai univentionstaff 2016-07-26 19:19:12 CEST
Created attachment 7838 [details]
WIP
Comment 23 Eduard Mai univentionstaff 2016-07-27 18:30:47 CEST
Created attachment 7842 [details]
Implemented both testcases as proposed. All issues from comments were adressed.
Comment 24 Eduard Mai univentionstaff 2016-07-27 18:34:52 CEST
Created attachment 7843 [details]
WIP

Implemented both test cases as proposed. All issues from comments were addressed.
Comment 25 Eduard Mai univentionstaff 2016-08-02 14:44:04 CEST
Revisions 71317 through 71347:
* All issues from QA have been addressed
* Two proposed test cases were implemented (changed as discussed)

Affected Packages:

Package: univention-ucs-translation-template
Version: 3.0.1-10.13.201608021438
Branch: ucs_4.1-0
Scope: errata4.1-2

Package: ucs-test
Version: 6.0.35-11.1514.201608021439
Branch: ucs_4.1-0
Scope: errata4.1-2
Comment 26 Alexander Kläser univentionstaff 2016-08-02 16:57:33 CEST
Looks fine :) !

I found this traceback when executing the test case:
---------- 8< ----------
usage: univention-ucs-translation-merge [-h] upstream translation
univention-ucs-translation-merge: error: unrecognized arguments: univention-ucs-translation-XX
Error: Subprocess exited unsuccessfully. Attempted command:
univention-ucs-translation-merge XX svn univention-ucs-translation-XX
Traceback (most recent call last):
  File "00_fuzzy_entries_on_merge", line 111, in <module>
    tools.call('univention-ucs-translation-merge', 'XX', SVN_PATH, TRANSLATION_PKG_NAME)
  File "/usr/share/ucs-test/85_ucs-translation-template/tools.py", line 57, in call
    raise InvalidCommandError()
tools.InvalidCommandError
Starting 1 ucs-test at 2016-08-02 16:45:02 to /dev/null
univention-ucs-translation-merge fuzzy entries................................................................................... Test failed
---------- 8< ----------

Do you remove the temporary folder?

$ du -sh /tmp/translation-template-test-00-3Wao6A/svn/
1,4G    /tmp/translation-template-test-00-3Wao6A/svn/
→ Maybe it would be sufficient (and faster) to use just one subfolder in the SVN?

What about a transition from the old format? It would be just important to re-copy the Makefile + debian/rules.
Comment 27 Alexander Kläser univentionstaff 2016-08-02 17:01:58 CEST
(In reply to Alexander Kläser from comment #26)
> [...]
> What about a transition from the old format? It would be just important to
> re-copy the Makefile + debian/rules.

... and debian/control.

And could you move the build target in Makefile just after the include statement?
Comment 28 Eduard Mai univentionstaff 2016-08-02 18:35:56 CEST
Rev. 71365:
* Tests remove temporary folders. 
* On merge debian/rules is adjusted when old package format is detected

Packages: 

Package: univention-ucs-translation-template
Version: 3.0.1-11.14.201608021831

Package: ucs-test
Version: 6.0.35-12.1515.201608021831
Comment 29 Eduard Mai univentionstaff 2016-08-02 18:54:34 CEST
Rev. 71368, 71370: Copy Makefile on merge with legacy translation package.

Package: univention-ucs-translation-template
Version: 3.0.1-12.15.201608021852
Comment 30 Eduard Mai univentionstaff 2016-08-02 19:18:47 CEST
Rev. 71371: Set exposure of tests to safe.

Package: ucs-test
Version: 6.0.35-13.1516.201608021916
Comment 31 Alexander Kläser univentionstaff 2016-08-02 19:28:55 CEST
I observed that the legacy debian/*install file needs to be removed, as well, for the transition from an legacy package to a new one.
Comment 32 Eduard Mai univentionstaff 2016-08-02 19:42:03 CEST
Rev.: Remove *.install on merge with legacy translation package

Package: univention-ucs-translation-template
Version: 3.0.1-13.16.201608021940
Comment 33 Alexander Kläser univentionstaff 2016-08-02 19:49:09 CEST
Works like a charm now :) !
→ VERIFIED
Comment 34 Janek Walkenhorst univentionstaff 2016-08-03 15:56:50 CEST
<http://errata.software-univention.de/ucs/4.1/225.html>
Comment 35 Alexander Kläser univentionstaff 2016-08-04 13:04:23 CEST
I could not find the tests being executed via Jenkins as the tests are currently not auto-installed. I fixed this.

ucs-test (6.0.35-14):
r71409 | Bug #41223: make sure that translation tests are auto-installed