Add test of BUILD_TRANSLATIONS for setting _translations #73

Don't add translations to .desktop files if BUILD_TRANSLATIONS is off.
Otherwise with LINGUAS unset or off, all translations are added regardless of BUILD_TRANSLATIONS.

This is not a usable solution. Note that none of the core modules set BUILD_TRANSLATIONS because translations are in a separate module.

If the BUILD_TRANSLATIONS variable is to be used to affect the desktop files, there must first be tested whether the variable is defined. If not defined translations should be processed.

Thanks for the tip, it also works for a Core build now.

I believe that the solution used is not wanted.
See comment below.

tde-i18n translations can be added/removed at runtime by the users, while desktop file are created at build time and included in the distributed packages.
I believe the right thing to do is to always have translations in desktop files, so that everything works as expected when users add/remove translations for the rest of the system.

With this PR updated to cover core builds, the comments don't now address that additional objective, which is:
if no languages have been set in LINGUAS, don't include translations.

In principle, this would be identical to the BUILD_TRANSLATIONS option for messages - if off, don't include translations; if on, include available translations as set by LINGUAS, and available per package, and if no languages have been set in LINGUAS, no translations are included.

So for .desktop files, if there are languages set in LINGUAS, include the translations; if there are no languages set in LINGUAS, don't include translations and I added set(BUILD_TRANSLATIONS "$ENV{LINGUAS}") to prevent them being included for the core packages - currently, if there is nothing set in LINGUAS, all available translations are included.

... AND (NOT DEFINED BUILD_TRANSLATIONS OR BUILD_TRANSLATIONS) )

NOT DEFINED BUILD_TRANSLATIONS will allow the macro to proceed as it does now. I want _translations to remain unset, and the macro to go to the else() option, '# just copy the original file without translations'.

I believe the right thing to do is to always have translations in desktop files, so that everything works as expected when users add/remove translations for the rest of the system.

I'm not suggesting that shouldn't be the case, but for a distribution, the package builders should have to set LINGUAS="af ar az be ... wa zh_CN zh_TW" to include the full range of translations supported.

In a sense, what I'm proposing is a work-around for the LINGUAS="" or unset cases. The real problem is in TDEMacros.cmake:

          # Decide which translations to build; the ones selected in the
          # LINGUAS environment variable, or all that are available.
          if( DEFINED ENV{LINGUAS} )
            set( _linguas "$ENV{LINGUAS}" )
          else( )
            string( REPLACE ".po;" " " _linguas "${_translations};" )
          endif( )

so if LINGUAS is unset, all available translations are included.

and tde_l10n_merge.pl:

    if (my $linguas = $ENV{"LINGUAS"})
    {
...
        else
        {
            for my $po_file (glob "$PO_DIR/*.po") {
                $po_files_by_lang{po_file2lang($po_file)} = $po_file;

I'm not familiar with perl, but this looks as though it could be where all translations are added if LINGUAS is unset or null, which is what happens.

That doesn't make sense - if the build is set up for no additional languages, include all available translations !?

With this PR updated to cover core builds, the comments don't now address that additional objective, which is: if no languages have been set in LINGUAS, don't include translations. In principle, this would be identical to the BUILD_TRANSLATIONS option for messages - if off, don't include translations; if on, include available translations as set by LINGUAS, and available per package, and if no languages have been set in LINGUAS, no translations are included. So for .desktop files, if there are languages set in LINGUAS, include the translations; if there are no languages set in LINGUAS, don't include translations and I added `set(BUILD_TRANSLATIONS "$ENV{LINGUAS}")` to prevent them being included for the core packages - currently, if there is nothing set in LINGUAS, all available translations are included. >... AND (NOT DEFINED BUILD_TRANSLATIONS OR BUILD_TRANSLATIONS) ) ***NOT DEFINED BUILD_TRANSLATIONS*** will allow the macro to proceed as it does now. I want _translations to remain unset, and the macro to go to the else() option, '<i># just copy the original file without translations</i>'. >I believe the right thing to do is to always have translations in desktop files, so that everything works as expected when users add/remove translations for the rest of the system. I'm not suggesting that shouldn't be the case, but for a distribution, the package builders should have to set LINGUAS="af ar az be ... wa zh_CN zh_TW" to include the full range of translations supported. In a sense, what I'm proposing is a work-around for the LINGUAS="" or unset cases. The real problem is in TDEMacros.cmake: ``` # Decide which translations to build; the ones selected in the # LINGUAS environment variable, or all that are available. if( DEFINED ENV{LINGUAS} ) set( _linguas "$ENV{LINGUAS}" ) else( ) string( REPLACE ".po;" " " _linguas "${_translations};" ) endif( ) ``` so if LINGUAS is unset, all available translations are included. and tde_l10n_merge.pl: ``` if (my $linguas = $ENV{"LINGUAS"}) { ... else { for my $po_file (glob "$PO_DIR/*.po") { $po_files_by_lang{po_file2lang($po_file)} = $po_file; ``` I'm not familiar with perl, but this looks as though it could be where all translations are added if LINGUAS is unset **or** null, which is what happens. That doesn't make sense - if the build is set up for no additional languages, include all available translations !?

There is a clear rule about LINGUAS variable: If it is not set or empty, all translations will be used.

Here are many distributions that do not use LINGUAS variable ever – DEB based distributions, RPM based distributions, FreeBSD,… Therefore, it is nonsense to forced all such distributions to set the LINGUAS variable. Therefore, I do not intend to change this rule – it broke most of the distributions.

OK. I'll close the PR.

I'm wary about deleting a branch in the remote repository, although I'm sure there are safeguards in place - will you do that?

I'm wary about deleting a branch in the remote repository, although I'm sure there are safeguards in place - will you do that?

Never mind, the option came up and I did it.

Ok, if a solution that accepts the assumption that empty LINGUAS means to use all translations, it is not acceptable to you, it would mean the difficulty in finding a compromise.

Could it works for you a variant of setting LINGUAS=-?

Maybe we can have an option to control the behavior when LINGUAS is empty? Something like LINGUAS_EMPTY_INCLUDE_ALL?
By default this is set to true and everything works as it is now, that is if LINGUAS is empty, all translations are included.
If LINGUAS_EMPTY_INCLUDE_ALL is false, then in case of empty LINGUAS, no translations will be included.
What do you think?

Maybe we can have an option to control the behavior when LINGUAS is empty? Something like LINGUAS_EMPTY_INCLUDE_ALL?
By default this is set to true and everything works as it is now, that is if LINGUAS is empty, all translations are included.
If LINGUAS_EMPTY_INCLUDE_ALL is false, then in case of empty LINGUAS, no translations will be included.
What do you think?

Here is one fact: Tools such as intltool-merge (as well as our tde_l10n_merge.pl) and msgfmt (gettext) have their own rules for case of empty LINGUAS. And this does not affect the introduction of our specific variable. That's why it doesn't seem like a good idea for me.

I hoped in the functionality of LINGUAS=-. This could work uniquely for all cases.

Thanks Slavek, I was just thinking out loud about new ideas to see if we can find a way that works for @Ray-V too.
If LINGUAS=- does the job, no problem for me. Let's see what Ray thinks about it.

Could it works for you a variant of setting LINGUAS=-?

I'm not sure what this means.

It seems similar to what I do in my own builds, which is to add a dummy entry to LINGUAS, and then without any actual languages set, the result is no translations added.

But that means LINGUAS always has content, which avoids the empty/unset cases.

So how does LINGUAS=- fit into the macro?

that empty LINGUAS means to use all translations, it is not acceptable to you

I appreciate the effort to accommodate my point of view, but unless there's a benefit to the project or anybody else, I would say leave as is.

I only see issues from the point of view of a single user and one distribution, so I realise my ideas may not be workable for TDE as a whole. If I think I have something to offer, I'll do so, and if it isn't accepted, so be it. If I'm still convinced that the idea is a good one, I can always add a work-around to my build scripts.

     unset( _translations )
     if( EXISTS "${_po_dir}" AND IS_DIRECTORY "${_po_dir}" )
     if(NOT DEFINED BUILD_TRANSLATIONS)
       set(BUILD_TRANSLATIONS "$ENV{LINGUAS}")

     if(NOT DEFINED BUILD_TRANSLATIONS)
       set(BUILD_TRANSLATIONS "$ENV{LINGUAS}")
     endif()
     if( EXISTS "${_po_dir}" AND IS_DIRECTORY "${_po_dir}" AND BUILD_TRANSLATIONS )

     endif( )
     # are there any translations available?
     # are there any translations available, and required?

Add test of BUILD_TRANSLATIONS for setting _translations #73

Reviewers