Remove use of LINGUAS file to prevent writes to the source directory #55
Merged
SlavekB
merged 1 commits from bug/3133/LINGUAS-source-write
into master
4 years ago
Loading…
Reference in new issue
There is no content yet.
Delete Branch 'bug/3133/LINGUAS-source-write'
Deleting a branch is permanent. It CANNOT be undone. Continue?
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:
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 toadd_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:
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.
if( DESKTOP_MERGE_MSGFMT )
# create LINGUAS file for msgfmt
This comment seems worth deserving to change 😺
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!
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.
8cf356884f
.