generate compiled translation files correctly #799
Conversation
|
… wait, this fails on buster (but succeeds on sid, hmm)… |
|
I think for a proper fix we must use |
|
qmake in buster doesn’t seem to call lrelease at all‽ |
|
Right, buster is missing |
|
Ah right, Qt 5.12 and newer… |
|
Thanks but Jamulus should support all Qt 5 versions. How do we solve this? |
|
Turns out one cannot simply add Since this changes the C++ part of Qt itself… bad luck. |
|
Let’s see whether this works. Let users either add Update: Hmm, considering that … might even be useful to require manual lrelease calling or using Qt 5.12+ from users, so we can get rid of the |
|
How is your new code supposed to work? If I clone from your repo/branch and delete the *.qm files in the translation directory, under Windows I then get compiler errors. I thought that with lrelease the *.qm files are created on every build? |
|
Volker Fischer dixit:
How is your new code supposed to work? If I clone from your repo/branch
and delete the *.qm files in the translation directory, under Windows I
then get compiler errors. I thought that with lrelease the *.qm files
are created on every build?
Hm weird, it works for me, and yes they are.
Are you sure you’re using Qt 5.12 or newer?
If using Qt 5.11 or older, you have to either not delete the .qm files
or run lrelease manually before the build. (So no change to before.)
|
|
I have Qt version 5.15.2 on Windows. |
|
How do we proceed with this pull request? |
|
Volker Fischer dixit:
How do we proceed with this pull request?
Hm, good question. Someone should probably look at why this
doesn’t work on Windows®, but that one is not me, I think
the win2k installation I have on an old laptop somewhere is
not… exactly helpful.
If you commit this change but do NOT delete the .qm binary
files for now, will it break anything? In that case, I’d
expect the lrelease run to update the .qm files if it works
and the build to use the committed files if not.
This would allow me to exclude the binary files from the
Debian source and not need to patch the build system, as
lrelease “just works”, in bullseye/sid anyway, and for the
backport, I just run it manually before building:
https://evolvis.org/plugins/scmgit/cgi-bin/gitweb.cgi?p=alioth/jamulus.git;a=commitdiff;h=refs/heads/buster-backports;hp=refs/heads/master
|
I don't think so. To make sure I'll try that once more. I am still a bit concerned about this change: https://github.com/corrados/jamulus/pull/799/files#diff-3b059d403dbbf0c4c127642ee3318bc2c3b10f70f929b71e2746c4a6ef907244L654. This will influence the tar.gz release of Jamulus. If someone uses a Linux distrubition with an old Qt version, the .qm files will not be created and since they are no longer distributed, he will not have them at all. What do you think? |
|
Volker Fischer dixit:
> If you commit this change but do NOT delete the .qm binary files for now, will it break anything?
I don't think so. To make sure I'll try that once more.
OK, thanks.
This will influence the tar.gz release of Jamulus. If someone uses a
Linux distrubition with an old Qt version, the .qm files will not be
created and since they are no longer distributed, he will not have them
at all. What do you think?
Leave that part out (or, maybe better, revert it with an explicit
commit saying that the files need to stay part of that .tar.gz for
now until we can either assume working lrelease on users’ parts or
require them to run lrelease manually like I did in the buster
branch — this will allow later “git revert”ing the partial revert
once no longer needed).
If you wish I can add such a commit.
|
Yes, please. Thanks. |
I just checked it without deleting the .qm files and it compiles and runs fine on my Windows. |
users of older releases either must manually run… (cd src/res/translation && lrelease *.ts) … before the build or use precompiled *.qm files
as discussed in jamulussoftware#799 these files need to be present in the .tar.gz distribution until we can require every user to either have a working lrelease.prf (Qt 5.12 and above, but seemingly not working right under Windows® either) or to manually run lrelease before building, as is done e.g. for the Debian backport to buster: diff --git a/debian/rules b/debian/rules index 7d93ed2..5234e86 100755 --- a/debian/rules +++ b/debian/rules @@ -45,4 +49,5 @@ endif dh $@ override_dh_auto_configure: + cd src/res/translation && lrelease *.ts dh_auto_configure -- ${CONFIG_ARGS} ---- This is a separate commit from its parent so, when the time comes, it’ll be easy to “git revert” it.
|
Volker Fischer dixit:
> If you wish I can add such a commit.
Yes, please. Thanks.
I just did so, and rebased on latest master while at it.
|
|
Thank you. Do you need a official Jamulus release soon or is it enough for now to have it in the Git repo? |
|
Volker Fischer dixit:
Thank you. Do you need a official Jamulus release soon or is it enough for now to have it in the Git repo?
In the repo is enough, unless you changed much since 3.6.2 which is
the version I recently uploaded hoping it’ll make into the next
stable release, and it’s carrying this as a local patch.
Thanks!
|
lreleaseoptionTested in Debian.
A possible improvement on this is to set
CONFIG += embed_translationsas well and don’t add them tosrc/resources.qrc(but let qmake do that); we’d have to redo the part that lists the available translations insrc/util.cpp:CLocale::GetAvailableTranslations()then (but given it maps all languages from xx_XX to xx except Portuguese is doubled, some more insightful algorithm might be used there anyway, or the consumers could just deal with having the countries there as well).