fixes #8874 - rework POT/PO updates for gettext 3's edit.po #2096

domcleal · 2015-01-19T14:38:30Z

3.1.13 adds an intermediate .edit.po file alongside each .po, which is meant
to be kept outside of SCM and updated by users, whereupon it's merged back into
the .po on the next rake gettext:find execution.

Previously we overwrote all gettext .po file changes with files directly from
Transifex, but now these are downloaded to .edit.po and gettext merges them
back in. Fuzzy merges have been disabled as they were inaccurate.

Reminder, use make -C locale tx-update to trigger it all.

Note there are a couple of style changes in the POT file, but I think they're good. 1) "TRANSLATORS" is now showing up in the comments, 2) source locations are enabled, 3) the header is in template form.

More details on the ticket comments too.

lzap · 2015-01-21T14:27:38Z

locale/Makefile

 	tx pull -f
-	-git commit -a -m "i18n - extracting new, updating rails, pulling from tx"


Was this intentional? I think it is fine to have one less commit, I agree.

It was intentional. The old way used to result in one commit as it would amend this original commit later. It used to do this early commit because it would then revert .po file changes back.

lzap · 2015-01-21T14:29:13Z

Nice, I am going to test this with core and than with discovery.

domcleal · 2015-01-21T14:31:13Z

Thanks, I suspect we'll need to update .gitignore in plugins too.

domcleal · 2015-01-21T14:31:24Z

Oh, and .tx/config. I wonder what else..

lzap · 2015-01-22T08:17:19Z

Makefiles obviously... testing now.

lzap · 2015-01-22T08:21:20Z

Hmmm I am not sure why actually make -C locale target works, but it does. I don't see this target. It should be all-mo which is the default one. Do you happen to know why it does work? Maybe locale stands for the directory name calling the default one.

lzap · 2015-01-22T08:23:39Z

bundler.d/development.rb

@@ -4,7 +4,7 @@
  gem 'rubocop', '0.26.1'

  # for generating i18n files
-  gem 'gettext', '~> 3.1', :require => false
+  gem 'gettext', '>= 3.1.3', '< 4.0.0', :require => false


Remind me why do we repeat it here once again? Time to zap it?

development.rb is the correct location, assets.rb is the duplicate. It has a comment above explaining, but it's a missing dependency in gettext_i18n_rails_js on gettext, so if somebody didn't have the development group, they'd need the gettext dep too.

domcleal · 2015-01-22T09:07:04Z

I'm not sure I understand your comment about the target. The full command to extract & update translations is make -C locale tx-update. -C locale is the directory, tx-update is the target.

lzap · 2015-01-22T10:16:04Z

Oh yeah I missed the -C because I was calling it from locale/. Disregard :-)

Can you test/elaborate this patch? For me it does not create files named foreman_discovery.po anymore, but it names it just foreman.po for a plugin which was wrong when I was doing:

rake plugin:gettext[foreman_discovery]

Also I still see fuzzy messages added when doing this.

diff --git a/lib/tasks/gettext.rake b/lib/tasks/gettext.rake
index 676c461..abcb30a 100644
--- a/lib/tasks/gettext.rake
+++ b/lib/tasks/gettext.rake
@@ -22,7 +22,8 @@ begin
   desc 'Extract plugin strings - called via rake plugin:gettext[plugin_name]'
   task 'plugin:gettext', :engine do |t, args|

-    @engine = "#{args[:engine].camelize}::Engine".constantize
+    @domain = args[:engine]
+    @engine = "#{@domain.camelize}::Engine".constantize
     @engine_root = @engine.root

     namespace :gettext do
@@ -35,10 +36,13 @@ begin
         Dir.glob("#{@engine.root}/{app,db,lib,config,locale}/**/*.{rb,erb,haml,slim,rhtml,js}")
       end

+      def text_domain
+        @domain
+      end
+
     end

-    Foreman::Gettext::Support.add_text_domain args[:engine], "#{@engine_root}/locale"
-    ENV['TEXTDOMAIN'] = args[:engine]
+    Foreman::Gettext::Support.add_text_domain @domain, "#{@engine_root}/locale"

     Rake::Task['gettext:find'].invoke

lzap · 2015-01-22T10:17:23Z

config/initializers/1_fast_gettext.rb

+# Keep TRANSLATORS comments
+Rails.application.config.gettext_i18n_rails.xgettext = %w[--add-comments=TRANSLATORS:]
+# Disable fuzzy .po merging
+Rails.application.config.gettext_i18n_rails.msgmerge = %w[--no-fuzzy-matching]


We need those settings for plugins too. Are you able to add it somewhere?

I'll test plugins, but since it's in Foreman's initialiser, I'd have thought they'd inherit it naturally?

Yeah I'd expect that, but this is what I get for discovery:

msgstr "" "Project-Id-Version: foreman_discovery 2.0.0.rc1\n" "Report-Msgid-Bugs-To: \n" -"POT-Creation-Date: 2014-08-20 08:33+0100\n" "PO-Revision-Date: 2014-09-16 16:47+0000\n" "Last-Translator: Ettore Atalan <atalanttore@googlemail.com>\n" -"Language-Team: German (http://www.transifex.com/projects/p/foreman/language/de/)\n" +"Language-Team: German (http://www.transifex.com/projects/p/foreman/language/de" +"/)\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" @@ -36,15 +36,33 @@ msgstr "Standort zuweisen" msgid "Assign Organization" msgstr "Organisation zuweisen" +#, fuzzy +msgid "Auto Provision" +msgstr "Bereitstellen"

Unfortunately this has no effect on one of the two merges, as the merge from .edit.po to .po isn't configurable. I filed ruby-gettext/gettext#44 to have the options apply to both.

gettext 3.2.1 fixes the issue, I've updated the minimum dependency to get the fix.

What was the concern here? The presence of this +#, fuzzy line? I still get it in foreman_remote_execution when using this PR and gettext 3.2.1

Just that we don't intend to use fuzzy translation matching. Can you provide the specifics of what string and translation is in each file as you extract, pull and merge?

The PR is here, btw: theforeman/foreman_remote_execution#145

Can you provide the specifics of what string and translation is in each file as you extract, pull and merge?

What do you need? There aren't any translations yet for REX.

I followed the wiki page, incorporated the changes from theforeman/foreman_plugin_template#13, and ran the rake task. I'm not sure what I need to do to verify that things worked as expected though.

Are you referring to this?

+#, fuzzy +msgid "" +msgstr ""

If so, I'd suggest just removing the empty string from where you're extracting it (probably https://github.com/theforeman/foreman_remote_execution/pull/145/files#diff-4354b6d435b32105d6decf507c6bca49R7). It's unclear that you're actually exercising the code mentioned here if you're not doing a pull and merge.

As an aside, I'll try and review that PR as the strings are not good enough to translate.

domcleal · 2015-01-22T11:27:57Z

Seems plugins are somewhat tricky with .edit.po, as after doing a tx pull you'd then need to re-run the rake task in core. I'll have a longer look at that soon, thanks for testing it.

dLobatog · 2015-08-19T20:04:40Z

@domcleal ping?

bkearney · 2015-09-18T18:26:03Z

@domcleal Can you take a look at https://github.com/bkearney/foreman/commits/bkearney-8874-i18n-failures. I was able to get the plugins to generate correctly. That patch is a rebase of yours onto develop, and then a small patch on top of it.

domcleal · 2015-09-24T08:21:29Z

How does that work from the developer's perspective? They run rake task, run tx pull, run the rake task again manually?

bkearney · 2015-09-24T17:26:55Z

@domcleal

assuming that the plugin is configured to tx pull to domain.edit.po, the process would need to be:

a) from the foreman directory, run rake plugin:gettext[$PLUGIN_NAME]
b) from the plugin directory run tx pull -f
c) then either run a msgmerge from the plugin directory, or do step (a) again.

To be honest, I dont know how often the user needs to do a,b, and c. It seems like (a) should be done on a weekly basis, and (b,c) should be done right before release.

If the plugin is not using edit files, then I dont think step c is needed.

domcleal · 2016-01-21T12:44:17Z

Yes, that's the set of steps I was unhappy with in #2096 (comment). There's also an additional step after running the core rake task in (c) as the merged POs have to be recompiled into MO files (usually by running make in the plugin), which should be done with an extra target in the plugin's Makefile. I think we'll have to live with this four-step process for now.

theforeman/foreman_plugin_template#13 shows the matching changes from the plugin perspective, and I've outlined the steps in the plugin maintainer's docs here: http://projects.theforeman.org/projects/foreman/wiki/How_to_Create_a_Plugin#Pulling-translations-from-Transifex

3.1.13 adds an intermediate .edit.po file alongside each .po, which is meant to be kept outside of SCM and updated by users, whereupon it's merged back into the .po on the next rake gettext:find execution. Previously we overwrote all gettext .po file changes with files directly from Transifex, but now these are downloaded to .edit.po and gettext merges them back in. Fuzzy merges have been disabled as they were inaccurate.

lzap · 2016-02-03T12:41:28Z

Tested with Foreman Discovery (with theforeman/foreman_discovery#244).

lzap · 2016-02-03T12:45:22Z

lib/tasks/gettext.rake

    end

-    Foreman::Gettext::Support.add_text_domain args[:engine], "#{@engine_root}/locale"
-    ENV['TEXTDOMAIN'] = args[:engine]
+    Foreman::Gettext::Support.add_text_domain @domain, "#{@engine_root}/locale"

    Rake::Task['gettext:find'].invoke


For the record, I rarely see that this line fails with Nil exception, the task is sometimes not available:

# rake plugin:gettext[foreman_discovery] ... rake aborted! NoMethodError: undefined method `match' for nil:NilClass /home/lzap/work/foreman/lib/tasks/gettext.rake:39:in `block in <top (required)>' Tasks: TOP => gettext:po:update => gettext:po:ru:update (See full trace by running task with --trace)

Ori reproduced this as well, but since this was there before this patch, I am merging this and we can attempt to find this later.

lzap · 2016-02-03T12:47:05Z

Merged as 4406f5a, thank you!

theforeman-bot added Not yet reviewed Needs testing labels Jan 19, 2015

domcleal force-pushed the 8874-i18n-failures branch from 827cb64 to c90541d Compare January 19, 2015 15:26

lzap reviewed Jan 21, 2015
View reviewed changes

lzap reviewed Jan 22, 2015
View reviewed changes

domcleal force-pushed the 8874-i18n-failures branch from c90541d to 0cedc70 Compare January 22, 2015 09:05

lzap reviewed Jan 22, 2015
View reviewed changes

domcleal added Waiting on contributor and removed Not yet reviewed labels Jan 22, 2015

domcleal mentioned this pull request Jan 27, 2015

Add i18n support theforeman/foreman_plugin_template#4

Merged

domcleal mentioned this pull request Feb 12, 2015

fixes #8980 - update to gettext 3.x, unpin locale theforeman/hammer-cli#160

Merged

dLobatog removed the Needs testing label Feb 24, 2015

lzap mentioned this pull request Aug 13, 2015

Bumped Foreman dependency in engine.rb, extracted strings theforeman/foreman_discovery#209

Closed

bkearney mentioned this pull request Oct 28, 2015

Fixes #10240 - strings extracted for i18n theforeman/foreman_openscap#132

Merged

domcleal mentioned this pull request Jan 21, 2016

refs #8874 - rework POT/PO updates for gettext 3's edit.po theforeman/foreman_plugin_template#13

Closed

domcleal force-pushed the 8874-i18n-failures branch from 0cedc70 to 90b2663 Compare January 21, 2016 12:44

theforeman-bot added Needs re-review Needs testing and removed Waiting on contributor labels Jan 21, 2016

domcleal force-pushed the 8874-i18n-failures branch from 90b2663 to 4638360 Compare January 25, 2016 08:23

orrabin mentioned this pull request Feb 3, 2016

refs #8874 - updates for gettext 3 theforeman/foreman_discovery#244

Closed

lzap self-assigned this Feb 3, 2016

lzap added Ready to merge and removed Needs re-review Needs testing labels Feb 3, 2016

lzap reviewed Feb 3, 2016
View reviewed changes

lzap closed this Feb 3, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixes #8874 - rework POT/PO updates for gettext 3's edit.po #2096

fixes #8874 - rework POT/PO updates for gettext 3's edit.po #2096

domcleal commented Jan 19, 2015

lzap Jan 21, 2015

domcleal Jan 21, 2015

lzap commented Jan 21, 2015

domcleal commented Jan 21, 2015

domcleal commented Jan 21, 2015

lzap commented Jan 22, 2015

lzap commented Jan 22, 2015

lzap Jan 22, 2015

domcleal Jan 22, 2015

domcleal commented Jan 22, 2015

lzap commented Jan 22, 2015

lzap Jan 22, 2015

domcleal Jan 22, 2015

lzap Jan 22, 2015

domcleal Jan 21, 2016

domcleal Jan 25, 2016

stbenjam Feb 3, 2016

domcleal Feb 3, 2016

stbenjam Feb 3, 2016

stbenjam Feb 3, 2016

domcleal Feb 3, 2016

domcleal commented Jan 22, 2015

dLobatog commented Aug 19, 2015

bkearney commented Sep 18, 2015

domcleal commented Sep 24, 2015

bkearney commented Sep 24, 2015

domcleal commented Jan 21, 2016

lzap commented Feb 3, 2016

lzap Feb 3, 2016

lzap commented Feb 3, 2016

		tx pull -f
		-git commit -a -m "i18n - extracting new, updating rails, pulling from tx"

fixes #8874 - rework POT/PO updates for gettext 3's edit.po #2096

fixes #8874 - rework POT/PO updates for gettext 3's edit.po #2096

Conversation

domcleal commented Jan 19, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lzap commented Jan 21, 2015

domcleal commented Jan 21, 2015

domcleal commented Jan 21, 2015

lzap commented Jan 22, 2015

lzap commented Jan 22, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domcleal commented Jan 22, 2015

lzap commented Jan 22, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domcleal commented Jan 22, 2015

dLobatog commented Aug 19, 2015

bkearney commented Sep 18, 2015

domcleal commented Sep 24, 2015

bkearney commented Sep 24, 2015

domcleal commented Jan 21, 2016

lzap commented Feb 3, 2016

Choose a reason for hiding this comment

lzap commented Feb 3, 2016