Bug 10942 - Provide a way for package upgrades to update template translations
Summary: Provide a way for package upgrades to update template translations
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Packaging (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Testopia
URL: 163.178.105.2:8082
Whiteboard:
Keywords:
Depends on:
Blocks: 4861
  Show dependency treegraph
 
Reported: 2013-09-24 17:44 UTC by Tomás Cohen Arazi
Modified: 2015-06-04 23:23 UTC (History)
10 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 10942: Provide a way for package upgrades to update template translations (2.87 KB, patch)
2013-09-25 18:53 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 10942: Provide a way for package upgrades to update template translations (2.78 KB, patch)
2013-09-26 00:54 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 10942: Provide a way for package upgrades to update template translations (3.36 KB, patch)
2014-01-09 15:33 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 10942: Provide a way for package upgrades to update template translations (3.47 KB, patch)
2014-02-01 03:32 UTC, Chris Cormack
Details | Diff | Splinter Review
[PASSED QA] Bug 10942: Provide a way for package upgrades to update template translations (3.55 KB, patch)
2014-03-11 15:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 10942: Provide a way for package upgrades to update template translations (3.55 KB, patch)
2014-03-12 02:11 UTC, Galen Charlton
Details | Diff | Splinter Review
bug 10942: (follow-up) create a /etc/koha/koha-common.conf (3.05 KB, patch)
2014-03-12 02:12 UTC, Galen Charlton
Details | Diff | Splinter Review
Bug 10942: (follow-up) make AUTOMATIC_TRANSLATIONS_UPDATE default to yes (2.09 KB, patch)
2014-03-12 02:12 UTC, Galen Charlton
Details | Diff | Splinter Review
Bug 10942: Provide a way for package upgrades to update template translations (3.59 KB, patch)
2014-03-12 02:21 UTC, Robin Sheat
Details | Diff | Splinter Review
bug 10942: (follow-up) create a /etc/koha/koha-common.conf (3.09 KB, patch)
2014-03-12 03:09 UTC, Robin Sheat
Details | Diff | Splinter Review
Bug 10942: (follow-up) make AUTOMATIC_TRANSLATIONS_UPDATE default to yes (2.14 KB, patch)
2014-03-12 03:09 UTC, Robin Sheat
Details | Diff | Splinter Review
Bug 10942 - debconfing the update option (4.41 KB, patch)
2014-03-12 04:27 UTC, Robin Sheat
Details | Diff | Splinter Review
Bug 10942: Provide a way for package upgrades to update template translations (3.59 KB, patch)
2014-03-12 23:15 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
bug 10942: (follow-up) create a /etc/koha/koha-common.conf (3.15 KB, patch)
2014-03-12 23:15 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 10942: (follow-up) make AUTOMATIC_TRANSLATIONS_UPDATE default to yes (2.19 KB, patch)
2014-03-12 23:15 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 10942 - debconfing the update option (5.12 KB, patch)
2014-03-12 23:16 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 10942: (QA followup) Warnings should go to STDERR (1.97 KB, patch)
2014-03-13 14:39 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[PASSED QA] Bug 10942: Provide a way for package upgrades to update template translations (3.67 KB, patch)
2014-04-23 10:56 UTC, Martin Renvoize
Details | Diff | Splinter Review
[PASSED QA] Bug 10942: (follow-up) create a /etc/koha/koha-common.conf (3.23 KB, patch)
2014-04-23 10:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
[PASSED QA] Bug 10942: (follow-up) make AUTOMATIC_TRANSLATIONS_UPDATE default to yes (2.27 KB, patch)
2014-04-23 10:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
[PASSED QA] Bug 10942 - debconfing the update option (5.21 KB, patch)
2014-04-23 10:57 UTC, Martin Renvoize
Details | Diff | Splinter Review
[PASSED QA] Bug 10942: (QA followup) Warnings should go to STDERR (2.05 KB, patch)
2014-04-23 10:59 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2013-09-24 17:44:01 UTC
During the upgrade of the 'koha-common' package we should provide a way of enabling template translations update to the most current.

It should default to 'no' as there might be local changes to the templates that could need to be preserved.
Comment 1 Tomás Cohen Arazi 2013-09-25 18:53:05 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2013-09-25 18:55:53 UTC
I provide this patch just to have a way to solve this in the case we don't implement the l10n packages in time and still consider this enhancement important (to avoid breakage for the end-user with outdated translated templates).
Comment 3 Robin Sheat 2013-09-25 22:06:10 UTC
Comment on attachment 21459 [details] [review]
Bug 10942: Provide a way for package upgrades to update template translations

Review of attachment 21459 [details] [review]:
-----------------------------------------------------------------

This is a good approach.

::: debian/koha-common.postinst
@@ +13,5 @@
> +    . /etc/default/$NAME
> +elif [ -r /etc/sysconfig/$NAME ]; then
> +    # RedHat / SuSE
> +    . /etc/sysconfig/$NAME
> +fi

Probably don't need to be so cross-platform in debian/*.postint, though not likely to hurt anything.
Comment 4 Tomás Cohen Arazi 2013-09-26 00:54:30 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi 2014-01-09 15:33:26 UTC Comment hidden (obsolete)
Comment 6 Chris Cormack 2014-02-01 03:32:35 UTC Comment hidden (obsolete)
Comment 7 Pablo AB 2014-02-05 16:31:05 UTC
This is an important issue for non-English Koha users, at least Spanish speakers. We see a lot of people having problem with this on Koha-es (Spanish Koha list, http://groups.yahoo.com/group/koha-es).
Comment 8 Marcel de Rooy 2014-02-07 16:24:01 UTC
Tomas:
I changed the Assigned to:-column and put your name there. Just for the record, as I recall this is not the first time you apparently forgot to do so :) I remember seeing another report this morning. 
Thanks for this detail..
Marcel
Comment 9 KSolano 2014-02-07 17:42:00 UTC
We've experienced some difficulties using package updates to update KOHA, in this case, template translations, spanish (es-ES) specifically, such as log in problems of any users when translation spanish is on, home page redirections to other menu inside OPAC, Z39.50 servers additions errors, and log out from de OPAC using superlibrarian users.

I'm use koha-translate --update es-ES everytime this happens, but I feel this is not the kind of stability we are looking for a librarian web base system.

Among this situations, I consider KOHA one of the best and congrats to developer team for amazing job.

Thanks!
Comment 10 Robin Sheat 2014-02-09 21:34:54 UTC
(In reply to KSolano from comment #9)
> I'm use koha-translate --update es-ES everytime this happens, but I feel
> this is not the kind of stability we are looking for a librarian web base
> system.

At the moment, you should be running koha-translate --update every time you do an update.

Once this patch is in, you won't need to as it'll happen automatically.
Comment 11 Martin Renvoize 2014-03-11 15:39:07 UTC Comment hidden (obsolete)
Comment 12 Galen Charlton 2014-03-11 16:00:27 UTC
(In reply to Robin Sheat from comment #10)
> (In reply to KSolano from comment #9)
> > I'm use koha-translate --update es-ES everytime this happens, but I feel
> > this is not the kind of stability we are looking for a librarian web base
> > system.
> 
> At the moment, you should be running koha-translate --update every time you
> do an update.
> 
> Once this patch is in, you won't need to as it'll happen automatically.

To be clear, it will /not/ happen automatically by default, as you'd need to edit /etc/default/koha-common first.

But this is where I want to raise the question: why not make AUTOMATIC_TRANSLATIONS_UPDATE be the default?  If you're running from packages, you would expect to get the base English templates overwritten whenever you upgrade; why would the expectation be different for the generated translated templates?  IOW, I'd like to here from folks whether there are any who are both running from packages /and/ manually tweaking the generated translated templates?

I may be off-base of course, but in any event, if the default for AUTOMATIC_TRANSLATIONS_UPDATE does need to be no, I think it would be a good idea to ask during upgrade.
Comment 13 Katrin Fischer 2014-03-11 16:11:47 UTC
I think it could be on by default, as it happened quite too often that we forgot to update the templates in the past and I can't really think of a reason why you wouldn't want that, unless you have been tinkering with things and then you shoudl be able to set the variable to prevent automated updating. But if we leave it off, prompting sounds good.
Comment 14 Tomás Cohen Arazi 2014-03-11 16:46:13 UTC
Short: +1 for a followup that changes to 'yes'

Long: I'd prefer to enable it by default. People having custom stuff are clever enough to disable it if necessary.

That said, exploring the use of debconf to have dialogs for this and other preferences is something we need to do. As soon as possible.
Comment 15 Robin Sheat 2014-03-11 20:15:08 UTC
I have no problem with it being on by default.

We should probably also have a postrm hook that removes the translations when the package is removed, as they won't be tracked by the package manager as-is. 

They probably also should be going into /var, as writing to /usr is a bit of an odd thing to do really. This'll require more thought though.

Neither of these should stop this patch though.
Comment 16 Robin Sheat 2014-03-11 22:31:32 UTC
Talking with Galen, /etc/default/koha-common shouldn't be used as it's really for init scripts to be configured, which isn't the case here.

More probably, it should be something like /etc/koha/koha-common.conf.
Comment 17 Galen Charlton 2014-03-12 02:11:57 UTC Comment hidden (obsolete)
Comment 18 Galen Charlton 2014-03-12 02:12:06 UTC Comment hidden (obsolete)
Comment 19 Galen Charlton 2014-03-12 02:12:19 UTC Comment hidden (obsolete)
Comment 20 Galen Charlton 2014-03-12 02:14:16 UTC
I've signed off on the main patch, but attached two follow-ups for which I like review.  The first implements creating /etc/koha/koha-common.conf and moving AUTOMATIC_TRANSLATIONS_UPDATE to it.

The second follow-up, which is very much up for discussion, makes AUTOMATIC_TRANSLATIONS_UPDATE be on by default.  Ideally, it will never see the light of day in master if somebody can whip up debconf support quickly.
Comment 21 Robin Sheat 2014-03-12 02:21:18 UTC Comment hidden (obsolete)
Comment 22 Robin Sheat 2014-03-12 03:09:29 UTC Comment hidden (obsolete)
Comment 23 Robin Sheat 2014-03-12 03:09:43 UTC Comment hidden (obsolete)
Comment 24 Robin Sheat 2014-03-12 04:27:24 UTC Comment hidden (obsolete)
Comment 25 Robin Sheat 2014-03-12 04:28:35 UTC
(In reply to Robin Sheat from comment #24)
> Created attachment 26138 [details] [review] [review]
> Bug 10942 - debconfing the update option

Note that this is totally untested code at this stage. It might work, but really there are bound to be bugs there. I'm just putting it up to allow someone else the chance to review it if they get the chance, otherwise I'll get back to it tomorrow.
Comment 26 Tomás Cohen Arazi 2014-03-12 23:15:13 UTC Comment hidden (obsolete)
Comment 27 Tomás Cohen Arazi 2014-03-12 23:15:31 UTC Comment hidden (obsolete)
Comment 28 Tomás Cohen Arazi 2014-03-12 23:15:43 UTC Comment hidden (obsolete)
Comment 29 Tomás Cohen Arazi 2014-03-12 23:16:39 UTC Comment hidden (obsolete)
Comment 30 Tomás Cohen Arazi 2014-03-13 14:39:20 UTC Comment hidden (obsolete)
Comment 31 Tomás Cohen Arazi 2014-03-13 15:25:14 UTC
I've conveniently built a package for testing purposes:

http://es.koha-community.org/koha-common_3.15+20140313114337.f0f1e109_all.deb

It's current master+10942
Comment 32 Martin Renvoize 2014-04-23 10:56:56 UTC
Created attachment 27454 [details] [review]
[PASSED QA] Bug 10942: Provide a way for package upgrades to update template translations

This patch adds a new config variable AUTOMATIC_TRANSLATIONS_UPDATE at
/etc/default/koha-common that is used to control whether the upgrade
process should trigger a

 $ koha-translate --update <lang_code>

command for each installed template translation language.

To test:
- Have a koha-common setup with some languages installed
  (e.g. koha-translate --install es-ES)
- Apply the patch and build a package for it.
- Install it.
- A new AUTOMATIC_TRANSLATIONS_UPDATE config variable should be in place
  at /etc/default/koha-common
- Set AUTOMATIC_TRANSLATIONS_UPDATE to 'yes'
- Re-install the package to trigger the post-install script
- Verify that translations get updated.

Edit: added a warning message for the case AUTOMATIC_TRANSLATIONS_UPDATE=no
and there are translations installed (so they need to get updated).

Regards
To+

Sponsored-by: Universidad Nacional de Cordoba
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Works as advertised, default behaviour doesn't change.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Robin Sheat <robin@catalyst.net.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 33 Martin Renvoize 2014-04-23 10:57:03 UTC
Created attachment 27455 [details] [review]
[PASSED QA] Bug 10942: (follow-up) create a /etc/koha/koha-common.conf

This patch creates a new master configuration file for the
koha-common package, and moves the AUTOMATIC_TRANSLATIONS_UPDATE
variable rather than leaving in in /etc/default/koha, which is meant
to be used for init script settings.

The configuration format is simple - a shell script that
sets variables and which can sourced by another script or
trivially parsed.

To test:

- Apply the patch series for bug 10942 and build a package.
- Install the package.
- Verify that a new config file, /etc/koha/koha-common.conf.
- Follow the rest of the test plan for the main page (e.g.,
  set AUTOMATIC_TRANSLATIONS_UPDATE and force a package upgrade).

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Robin Sheat <robin@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 34 Martin Renvoize 2014-04-23 10:57:13 UTC
Created attachment 27456 [details] [review]
[PASSED QA] Bug 10942: (follow-up) make AUTOMATIC_TRANSLATIONS_UPDATE default to yes

This patch makes AUTOMATIC_TRANSLATIONS_UPDATE default to yes,
meaning that package upgrades will update the translations by
default.  This seems definitely sensible for new installations,
but as there are some older installations that may be in the
habit directly editing generated translated templates, it's
less clear whether this is a good idea for upgrades.

This patch is intentionally separate, and can be ignored if
the consensus swings towards having AUTOMATIC_TRANSLATIONS_UPDATE
be off even for new installations or if somebody counter-patches
with adding debconf support.

To test:

- As with the previous patches in the series, except that the
  translations would be automatically updated upon installing
  the new package.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Robin Sheat <robin@catalyst.net.nz>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 35 Martin Renvoize 2014-04-23 10:57:23 UTC
Created attachment 27457 [details] [review]
[PASSED QA] Bug 10942 - debconfing the update option

This causes a question to be asked at installation time as to whether
translations should be updated or not. The answer is written to the
config file, and stored in debconf. Effort is taken to ensure that if
the admin changes the config file, the update will be picked up and
reflected in debconf (i.e. that the admin's decision is always the
correct one.)

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Fixed two typos that made it fail and it worked like a charm.

Tested like this:

- Install the package
=> no errors, the file is created, defaults to 'yes'
- Install a language (koha-translate --install es-ES)
- Re-install the package (simulating an upgrade)
=> es-ES gets updated
- Set preference to 'no'
- Re-install
=> es-ES doesn't get updated, the warning is printed correctly
- Installed a second language (koha-translate --install pt-BR)
- did all the tests again
=> Success

Note: on master there are obvious template translation warnings.
A copy of the generated package can be grabbed from:
http://es.koha-community.org/koha-common_3.15+20140312172225.af7c0a23_all.deb

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 36 Martin Renvoize 2014-04-23 10:59:06 UTC
Created attachment 27458 [details] [review]
[PASSED QA] Bug 10942: (QA followup) Warnings should go to STDERR

As noted by Robin, STDOUT is used by the script to communicate with
debconf and, ence, the warning messages should be directed to STDERR.

This patch does that. To test:

- Set AUTOMATIC_TRANSLATIONS_UPDATE="no" so the warning normally shows
- Redirect STDOUT to /dev/null:
  dpkg -i koha-common...deb > /dev/null
=> Warning message doesn't show (i.e. it is sent to STDOUT)
- Apply the patch, rebuild package
- Redirect STDOUT to /dev/null:
  dpkg -i koha-common...deb > /dev/null
=> Warning message shows (i.e. is correctly sent to STDERR)
- Redirect STDERR to /dev/null:
  dpkg -i koha-common...deb 2> /dev/null
=> Warning message doesn't show
- Verify that previous tests pass

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 37 Martin Renvoize 2014-04-23 11:02:15 UTC
Yowsers, that was a long one.

I built packages from the resulting patch set, and tested:

1. That the config file is now created - tick
2. That the config file is used for upgrades and defaults to yes - tick
3. That the config file is read as upgrade time to correctly set debconf - tick
4. That it all passes the qa scripts still - tick

I'm happy that this all works as described now. Nice bit of team work going on there.

I'm going to open a further bug, so that the suggestion to move translations into /var doesn't get lost once this bugs life expires.
Comment 38 Robin Sheat 2014-04-23 23:12:17 UTC
(In reply to Martin Renvoize from comment #37)
> Yowsers, that was a long one.

The downside of trying to do things as correct as possible is that it takes longer :) The upside is that this will probably keep working for the foreseeable future as I think it's probably the best solution.
Comment 39 Robin Sheat 2014-04-23 23:13:37 UTC
(In reply to Robin Sheat from comment #38)
> longer :) The upside is that this will probably keep working for the
> foreseeable future as I think it's probably the best solution.

(until we get all the translations building with the packages, anyway.)
Comment 40 Galen Charlton 2014-04-24 12:51:57 UTC
Pushed to master.  Thanks, Tomás and Robin!
Comment 41 Fridolin SOMERS 2014-09-17 14:37:25 UTC
I choose to not apply to 3.14.x so I set as resolved.