Makefile loses translations if .po file is defect

[Fadiabb]

Hallo,

`makemessages' Makefile replaces all the already translated strings in .po file with empty strings if .po file is defect and just shows an error message.

reproduce the problem : either
a- duplicate the same string in .po file in different lines. or b- add extra new line which breaks the .po file syntax (e.g. because of wrong .po files merging this characters ">>>>>>" stayed in the .po file) that will lead to lose all translated strings and replace them with empty ones when executing 'makemessages'. the following errors will be shown: a- duplicate message definition... b- syntax Error... msgmerge: 2 fatal error has occurred

we want to avoid deleting the old translated strings and improve the behavoir of Makefile in this case.

[vperron]

Sure ! Could you come up with a pull request to fix that issue ? I'm not sure we want to support broken PO files though.

The makemessages command is a little old though, we could replace it with something that only uses easygettext for both JS and HTML: cf. https://github.com/Polyconseil/systematic/blob/master/mk/main.mk#L186-L197 (and reflect that in the README soince we won't need some GNU tools anymore)

[Fadiabb]

We lost the translations when Makefile detected an error in .po file (it overwirtes translated strings with empty ones) What expected:
Makefile should abort the execution when an error occurs instead of deleting all of the already translated strings. so we dont lose any data and we can fix the error in the broken file without something been lost

[vperron]

As I said, I'm expecting a pull request :)

[bernhardreiter]

Taking a look at the problem now to see if I can come up with a solution.

Technical idea how to solve it: If the condition is detected, stop further processing in the Makefile and revert to the original .po file to avoid losing data.

[bernhardreiter]

Analysis

Identifying the place of problematic behaviour

The problem seems to be within https://github.com/Polyconseil/vue-gettext/blob/317f1631e509bb42f9cf856d0a2ac21ba45b602a/Makefile#L44-L45 and the alternative part that is executed if msgmerge fails behind the ||. This seems to recreate a fresh and empty po file.

Understanding the problematic behaviour

This problem probably has been in there from the initial commit in https://github.com/Polyconseil/vue-gettext/blob/118f72d0da7d9bbb85bf293c9ddc149333c72ad8/Makefile coming from https://github.com/Polyconseil/systematic/blob/866d5a7b44b5926b7033271bbb2969d9d2a3dc9b/mk/main.mk#L181 . No quickly found description of this part of the behaviour.

Guessing: the code should create the po file if it is not there.

[bernhardreiter]

Solution

1. First fix the logic to only create a new .po file if none was there. Use if to make this more readable.
2. Make sure the Makefile stops if msgmerge bails out, for this an extra break statement is needed within the shell loop.
3. Move the output to the right place within the if. This is fine as msginit will also output something, so there is a message each time.

The following diff should do the job:

--- Makefile   Tue Feb 05 11:31:15 2019 +0100
+++ Makefile   Tue Feb 05 13:08:24 2019 +0100
@@ -52,10 +52,14 @@
 # Generate .po files for each available language.
        @for lang in $(LOCALES); do \
                export PO_FILE=$(OUTPUT_DIR)/locale/$$lang/LC_MESSAGES/app.po; \
-               echo "msgmerge --update $$PO_FILE $@"; \
                mkdir -p $$(dirname $$PO_FILE); \
-               [ -f $$PO_FILE ] && msgmerge --lang=$$lang --update $$PO_FILE $@ || msginit --no-translator --locale=$$lang --input=$@ --output-file=$$PO_FILE; \
-               msgattrib --no-wrap --no-obsolete -o $$PO_FILE $$PO_FILE; \
+               if [ -f $$PO_FILE ]; then  \
+                       echo "msgmerge --update $$PO_FILE $@"; \
+                       msgmerge --lang=$$lang --update $$PO_FILE $@ || break ;\
+               else \
+                       msginit --no-translator --locale=$$lang --input=$@ --output-file=$$PO_FILE || break ; \
+                       msgattrib --no-wrap --no-obsolete -o $$PO_FILE $$PO_FILE || break; \
+               fi; \
        done;

 $(OUTPUT_DIR)/locale/translations.json: $(LOCALE_FILES)