I agree with your solution and explanations. I had would go with the option that duplicates and freezes the given string (just as I18n does), because having different behaviors when calling to _() looks a little bit surprising to me. But of course, if that supposes to kill our memory, it clearly doesn't pay off. Thanks to explain it ;) On 2/19/19 12:12 PM, Ladislav Slezak wrote:

Hi all,

I was fixing a "can't modify frozen String" crash in YaST registration when processing a translated string. After debugging and trying to write an unit test for it I found an interesting difference: it depends whether the translation for the message is available or not.

Translation Available ---------------------

...

rpm -q yast2-trans-cs yast2-trans-cs-84.87.20180514.157a0650d-lp150.1.1.noarch LC_ALL=cs_CZ.UTF-8 ruby -ryast -e 'include Yast::I18n; textdomain "base"; puts _("&Yes"); puts _("&Yes").frozen?' &Ano true

So the result is a *frozen* translated string. The code would crash in this case.

Translation Not Available -------------------------

...

rpm -q yast2-trans-es package yast2-trans-es is not installed LC_ALL=es_ES.UTF-8 ruby -ryast -e 'include Yast::I18n; textdomain "base"; puts _("&Yes"); puts _("&Yes").frozen?' &Yes false

In this case you'll get the original untranslated and *not frozen* string. In this case it would NOT crash. :-S

The Problem -----------

The problem is that when running the unit tests in Travis, Jenkins or in OBS no translation is installed and the tests run in the English locale. However, on user systems or during installation the translations are usually available. That means it's hard to test it in the same environment, the unit tests might not find this problem in the code.

Moreover the translations for new texts might be available later, so it could work fine but crash later after updating the translation package without changing the YaST module. That's a debugging nightmare...

Proposed Solution -----------------

The ruby-bindings code contains this line [1]:

found ? Translation._(str) : str

The fast_gettext gem returns nil when the translation is not available. In that case we return the original text.

My proposed solution is to also freeze the not translated message to have consistent output:

found ? Translation._(str) : str.freeze

The Disadvantage ----------------

The disadvantage is that as a side effect of this change the passed parameter will be modified (becomes frozen), like this:

a = "text" a.frozen? # => false puts _(a) a.frozen? # => true

Usually that should not make any troubles as we either use string literals _("foo") or we use constants which should be preferably frozen anyway:

TEXT = N_("foo") ... _(TEXT)

(Or at least I'm not aware of any place where it would make troubles.)

Of course, we could duplicate the string to not modify the original one:

found ? Translation._(str) : str.dup.freeze

but I'm afraid this would have memory and performance impact as all not translated strings would be duplicated.

So I'd prefer the first solution and if you need to avoid freezing the original string you can make the copy yourselves, e.g.:

puts _(a.dup) a.frozen? # => false

(Or course this behavior and the solution needs to be well documented.)

Any comments or ideas to this topic?

Thank you!

[1] https://github.com/yast/yast-ruby-bindings/blob/4652448963160abc234d1199f06b...

-- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org