Remove use of LINGUAS file to prevent writes to the source directory #55

This is for the resolution of bug 3133

Signed-off-by: aneejit1 aneejit1@gmail.com

It's good that this is solved by writing to the source folder, but it needs to be changed, because it overrides any original value of the environment variable LINGUAS that the user may have set.

The current code worked so that unless the user specified the value of the environment variable LINGUAS, all languages were installed. If the user wants to limit the installed translations, he can set the LINGUAS environment variable to the languages he wants to install – this is usual for a Gentoo user, for example.

The proposed solution prevents the user from making such a choice. And that's not good.

OK, I've added a check to see if the environment variable is set. If it is, then the command is issued without the export/unset, if not, then it uses the information grabbed from the translation directory and does do the export/unset.

I've now got a problem trying to do a "git push". It's giving me the following errors:

To https://mirror.git.trinitydesktop.org/gitea/TDE/tde-common-cmake.git
! [rejected] bug/3133/LINGUAS-source-write -> bug/3133/LINGUAS-source-write (non-fast-forward)
error: failed to push some refs to 'https://mirror.git.trinitydesktop.org/gitea/TDE/tde-common-cmake.git'
To prevent you from losing history, non-fast-forward updates were rejected
Merge the remote changes (e.g. 'git pull') before pushing again. See the
'Note about fast-forwards' section of 'git push --help' for details.

I've tried the "git pull", but I don't know what I'm supposed to be pulling and that fails too. Help!

If you fixed the existing commit in the branch, then use push with force:

git push origing HEAD -f

That got it! Thanks.

I think we can make some simplifications:

The environment variables of the sub-process disappear along with the sub-process => there is no need to unset it.
You can populate the CMake variable _linguas so that if the evironment variable LINGUAS is set, it will be used, if it does not exist, a list of existing files will be used => then one call to add_custom_command will suffice.
In other words, use the LINGUAS condition when setting the _linguas variable, instead for call add_custom_command. The code will be clearer. Something like this:

if( DEFINED ENV{LINGUAS} )
  set( _linguas "$ENV{LINGUAS}" )
else()
  string( REPLACE ".po;" " " _linguas "${_translations};" )
endif

I'm not sure how the launch of two separate COMMANDs is implemented in CMake – if they will share the same environment or not. There should be more safe to use CMake call with environment variable settings:

COMMAND ${CMAKE_COMMAND} -E env "LINGUAS=${_linguas}"
${MSGFMT_EXECUTABLE} --desktop --template ${_src} -d ${_po_dir} -o ${_basename} ${_keywords_arg}

I built dolphin with LINGUAS set to "nl" and got only the files for that language generated, so the separate "export" command does change the environment for msgfmt. Leave the export as-is, or change it?

I built dolphin with LINGUAS set to "nl" and got only the files for that language generated, so the separate "export" command does change the environment for msgfmt. Leave the export as-is, or change it?

Using ${CMAKE_COMMAND} -E env ... should guarantee that it will work on all systems.

Sorry, ignore the last comment. The context in which I build dolphin isn't valid with the most recent change. I'll change it.

OK, 'tis done.

Great, it looks good, thank you! There is only one little thing.

I was always told not to put inline comments into code because they don't get maintained. Guess I just proved that one right! Old comment removed and new comment added.

Great, it looks good.

I gave it to severity minor not because there is an obstacle in the merge, but because msgfmt is not currently used for desktop files and therefore the merge can wait until there are more PRs waiting to be merged.

Anyway, thank you for a good work!

I gave it to severity minor not because there is an obstacle in the merge, but because msgfmt is not currently used for desktop files and therefore the merge can wait until there are more PRs waiting to be merged.

Anyway, thank you for a good work!

just to explain a bit more for @aneejijt1, PR on common cmake/admin modules will trigger a bunch on update commit on all modules, so we try to minimize the time we merge them by combining them in batches.

				`if( DESKTOP_MERGE_MSGFMT )`

				`# create LINGUAS file for msgfmt`