Bug #15362

Pre-commit hook/Run check_po whenever we try to commit a po file in all Git repositories

Added by Anonymous 2018-03-02 14:13:43 . Updated 2019-06-27 17:16:53 .

Status:
Resolved
Priority:
Normal
Assignee:
Category:
Target version:
Tails_3.15
Start date:
2018-03-02
Due date:
% Done:

30%

Feature Branch:
tails.git:translation_platform_hooks
Type of work:
Communicate
Blueprint:

Starter:
Affected tool:
Deliverable for:

Description

Currently, contribute/l10n_tricks/check_po.sh calls i18inspector and executes some checks. However, it does not check for inconsistent line endings and the like. We should fix that by looking at “git log -p *.po” and determining the manual fixes we regularly do.

Subtasks

Related issues

Related to Tails - Bug #15361: Implement automatic checking & correction of inconsistent strings in Weblate Resolved 2018-03-02
Related to Tails - Bug #15605: Make check_po optionally accept a list of files Resolved 2018-05-17
Related to Tails - Feature #8358: Automatically check PO files in all our repositories Rejected 2014-12-02
Related to Tails - Feature #16102: List of potential checks we might want to do on PO files Resolved 2018-11-05
Blocked by Tails - Feature #16328: Merge a stricter version of check_po whose expectations are realistic into our master branch Resolved 2019-01-08

History

#1 Updated by Anonymous 2018-03-02 14:14:05

  • related to Bug #15361: Implement automatic checking & correction of inconsistent strings in Weblate added

#2 Updated by Anonymous 2018-03-02 14:14:45

#3 Updated by Anonymous 2018-03-02 14:16:48

  • Subject changed from Improve checks to be done in check_po.sh to Improve integrity checks in check_po.sh

check for missing headers:

- pot-creation-date

check for wrong charsets

- charset should be set and should be utf-8

- inconsistent line endings are currently not checked

#4 Updated by Anonymous 2018-03-13 12:31:53

Actually this seems to be checked but as we don’t call this regulary, sometimes wrong files get committed.

#5 Updated by Anonymous 2018-03-13 13:12:09

  • Subject changed from Improve integrity checks in check_po.sh to Run check_po whenever we try to commit a po file in all Git repositories

We will likely want to add this check to the pre-commit hook:

#!/bin/sh
# If we try to commit po files, check that they do not contain errors.
if ! git diff --ignore-submodules --name-only --exit-code -- . '*.po' \
   && ! ./wiki/src/contribute/l10n_tricks/check_po.sh
then
   echo >&2 "The po files you're trying to commit contain errors. Please fix them and try again."
fi

#6 Updated by hefee 2018-03-13 14:34:31 

grep -l '"Content-Type: text/plain; charset=UTF-8\n"' *po

should be empty to make sure no file has another charset.

#7 Updated by bertagaz 2018-05-10 11:09:11

  • Target version changed from Tails_3.7 to Tails_3.8

#8 Updated by Anonymous 2018-05-17 13:16:05

pre-commit to test:


# If we try to commit po files, check that they do not contain errors.
if test $(git diff --ignore-submodules --name-only --exit-code -- . '*.po' $against |
    LC_ALL=C tr -d '[ -~]\0' | wc -c) != 0 ; then
    if ! ./wiki/src/contribute/l10n_tricks/check_po.sh ; then
        echo
        echo "The po files you're trying to commit contain errors. Please fix them and try again."
        echo
        exit 1
    fi
fi

#9 Updated by Anonymous 2018-05-17 13:23:06

  • Feature Branch set to tails.git:translation_platform_hooks

#10 Updated by Anonymous 2018-05-17 13:26:02

I’ve created a branch which shall contain all the hooks and scripts we need to test.

#11 Updated by Anonymous 2018-05-17 14:06:42

  • Status changed from Confirmed to In Progress

#12 Updated by Anonymous 2018-05-17 14:14:31

  • related to Bug #15605: Make check_po optionally accept a list of files added

#13 Updated by hefee 2018-05-17 16:51:39

  • % Done changed from 0 to 30

updated the pre-commit hook to process only changed files (ignoring also deleted files).

Side note: The pre-commit hook using the file in the working directy and not the “real” diff that will be commited.
So you can still commit unclean po files if you have outstanding diffs for the po files. I think this is not a bug issue, as you normally will commit what you have changed in further commits. For the main git this is not an issue, as it has no chackedout version.

#14 Updated by Anonymous 2018-05-29 09:05:50

hefee wrote:
> updated the pre-commit hook to process only changed files (ignoring also deleted files).

Thanks!

> So you can still commit unclean po files if you have outstanding diffs for the po files. I think this is not a bug issue, as you normally will commit what you have changed in further commits. For the main git this is not an issue, as it has no chackedout version.

Ack.

#15 Updated by Anonymous 2018-05-29 09:22:36

u wrote:
> Currently, contribute/l10n_tricks/check_po.sh calls i18inspector and executes some checks. However, it does not check for inconsistent line endings and the like. We should fix that by looking at “git log -p *.po” and determining the manual fixes we regularly do.

The new pre-commit hook checks for whitespace errors.

#16 Updated by intrigeri 2018-06-26 16:27:46

  • Target version changed from Tails_3.8 to Tails_3.9

#17 Updated by Anonymous 2018-08-18 12:53:36

  • related to Feature #8358: Automatically check PO files in all our repositories added

#18 Updated by intrigeri 2018-08-18 14:39:15

What does “in all Git repositories” mean here? Given the parent task of this ticket, I understand it as “all Git repositories for our website”, but on Feature #8358 u suggested that it might also apply to our other Git repos (which is what Feature #8358 is about).

#19 Updated by Anonymous 2018-08-19 07:22:56

intrigeri, thanks for making this clear. I meant only the website Git repositories indeed. This said, it could eventually be applied to others.

#20 Updated by intrigeri 2018-08-19 07:56:08

> This said, it could eventually be applied to others.

Good to know! I’ve updated the description of Feature #8358 to list the available options and stated my opinion in a comment.

#21 Updated by Anonymous 2018-09-03 17:55:26

  • Target version changed from Tails_3.9 to Tails_3.10.1

Won’t be able to do this in the next 48 hours (release of 3.9)

#22 Updated by intrigeri 2018-10-24 17:03:33

  • Target version changed from Tails_3.10.1 to Tails_3.11

#23 Updated by Anonymous 2018-11-02 17:35:56

  • Subject changed from Run check_po whenever we try to commit a po file in all Git repositories to Pre-commit hook/Run check_po whenever we try to commit a po file in all Git repositories
  • Assignee set to hefee

file is here: bin/pre-commit-translation
strategy for implementation unclear.

#24 Updated by emmapeel 2018-11-03 08:54:58

I think that the script needs to say more than:

The po files you're trying to commit contain errors. Please fix them and try again.

Like what is the file, what is the problem, etc. Many of our contributors will not know how to get out of that situation.

#25 Updated by Anonymous 2018-11-05 12:16:47

@emmapeel: Did you test the hook?
Because it calls an updated version of check_po that actually gives you more details on what went wrong.
The error message you quote is simply a last message in the list, when check_po returns an error.

We’ve sent around requests for testing this hook several months ago, without any reply from translators.
These emails also contained information on how to prepare po files for compliance with check_po.
I can resend them, if needed.

#26 Updated by hefee 2018-11-05 16:31:31

  • related to Feature #16102: List of potential checks we might want to do on PO files added

#27 Updated by hefee 2018-11-05 16:44:31

  • QA Check set to Pass

> file is here: bin/pre-commit-translation
the file is fine so far. When we rename check_po, than we need also update the filepath.

> strategy for implementation unclear.

  1. make pre-commit-hook not failing, just warning:
    replace exit 1 with exit 0
  2. merge the translation_platform_hooks
  3. install the post-update-hook at tails servers, than everyone gets warned about issues in po file
  4. send a mail:
    - in 2 weeks the po files will get updated to pass the new checks.
    - the hook will be activated afterwards to failmode.
    - users should make sure that any non-commited stuff is pushed before, because if we update all po files merge conflicts may happen.
  5. 2 weeks later:
    - update all po files
    - change exit 0 -> exit 1 and make the hook active.
    - send a mail to inform the users.

#28 Updated by CyrilBrulebois 2018-12-16 13:58:01

  • Target version changed from Tails_3.11 to Tails_3.12

#29 Updated by intrigeri 2019-01-08 16:19:33

  • blocks Feature #15082: Have the Weblate Git communicate with our main Git repository added

#30 Updated by intrigeri 2019-01-08 17:02:25

  • blocked by Feature #16328: Merge a stricter version of check_po whose expectations are realistic into our master branch added

#31 Updated by intrigeri 2019-01-08 17:02:34

  • blocked by deleted (Feature #15082: Have the Weblate Git communicate with our main Git repository)

#32 Updated by intrigeri 2019-01-08 17:03:11

Updated plan: don’t block on this ticket and for now, focus on Feature #16328.

#33 Updated by anonym 2019-01-30 11:59:12

  • Target version changed from Tails_3.12 to Tails_3.13

#34 Updated by intrigeri 2019-03-12 17:09:38

  • Deliverable for deleted (Sponsor_L)

#35 Updated by CyrilBrulebois 2019-03-20 14:34:04

  • Target version changed from Tails_3.13 to Tails_3.14

#36 Updated by intrigeri 2019-04-10 17:28:58

  • Target version deleted (Tails_3.14)

intrigeri wrote:
> Updated plan: don’t block on this ticket and for now, focus on Feature #16328.

Given this + the past history of “Target version” values here, I guess we can drop the target version, at least for now :)

#37 Updated by intrigeri 2019-06-02 13:51:56

  • QA Check deleted (Pass)

(Preparing to drop the “QA Check” field as per “[Tails-dev] Proposal: Redmine workflow change”. I see no review having happened here.)

#38 Updated by hefee 2019-06-03 08:48:34

intrigeri wrote:
> (Preparing to drop the “QA Check” field as per “[Tails-dev] Proposal: Redmine workflow change”. I see no review having happened here.)

@intrigeri the code was written by u and I reviewd it. That’s why I set QA Check as “Pass”. So we loose the status, that the code got reviewed, but it is still living in a non merged branch.

#39 Updated by intrigeri 2019-06-26 10:59:52

  • Assignee changed from hefee to intrigeri
  • Target version set to Tails_3.15
  • Type of work changed from Code to Communicate

In the end we did the work to have this pre-commit hook (even if we had decided not to block on it) so I’ll announce it.

#40 Updated by intrigeri 2019-06-26 11:32:57

  • Status changed from In Progress to Resolved

#41 Updated by intrigeri 2019-06-27 17:16:54

  • Assignee deleted (intrigeri)