jamulus icon indicating copy to clipboard operation
jamulus copied to clipboard

Generate qm files and embedded resource during build

Open softins opened this issue 2 years ago • 20 comments

Short description of changes

Generate the translation qm files during build and remove them from github.

Context: Fixes an issue?

The qm files are binary dependants of the ts text files. Historically they have been generated manually and stored in github. Qt versions from 5.12 include qmake rules for generating and embedding them.

This revisits previous closed PR #1303

Does this change need documentation? What needs to be documented and how?

Status of this Pull Request

Working with Qt >= 5.12. ~If we need to keep supporting older Qt, we will need a compatible set of fallback rules.~ Fallback rules added.

What is missing until this pull request can be merged?

~Support for Qt < 5.12, or a decision to stop supporting those versions.~ Checking that all artifacts build and run correctly, particularly Mac Legacy.

Checklist

  • [x] I've verified that this Pull Request follows the general code principles
  • [x] I tested my code and it does what I want
  • [x] My code follows the style guide
  • [ ] I waited some time after this Pull Request was opened and all GitHub checks completed without errors.
  • [ ] I've filled all the content above

softins avatar Feb 14 '22 23:02 softins

Currently in Qt < 5.12, compilation still succeeds, but no translations are available in the executable.

softins avatar Feb 14 '22 23:02 softins

Is the legacy MacOS build affected?

pljones avatar Feb 15 '22 07:02 pljones

Is the legacy MacOS build affected?

Yes, at the moment it would build but have no translations. So if we want to continue legacy support in 3.9 there is more work to do yet.

I see there are also issues with some of the builds.

softins avatar Feb 15 '22 08:02 softins

We shouldn’t break backwards compatibility unless really needed (security, stability for new versions). So I would hope there’s a way to support translations for the legacy build.

https://github.com/jamulussoftware/jamulus/blob/master/CONTRIBUTING.md#supported-platforms

ann0see avatar Feb 15 '22 10:02 ann0see

I fear that we would still continue to provide the legacy Mac builds, wouldn't we?

If we want to continue supporting older versions, the blockers would be:

  • Lack of CONFIG+=lrelease support: We should be able to work around this like that though (maybe with a version guard): https://github.com/jamulussoftware/jamulus/pull/1303/files#diff-3b059d403dbbf0c4c127642ee3318bc2c3b10f70f929b71e2746c4a6ef907244R49
  • Lack of CONFIG+=embed_translations support:
    • Either we don't change that for now (I wasn't even aware that the files are loaded from the file system and thought they would have already been embedded)
    • Or we make embedding conditional (both in the .pro file and in the code). Carries some more version-specific logic, but could be acceptable, IMO.

hoffie avatar Feb 15 '22 10:02 hoffie

@softins this needs to be rebased.

ann0see avatar Mar 05 '22 17:03 ann0see

@softins this needs to be rebased.

Yes, I still haven't worked out what to do about embedding translations for Qt < 5.12. I haven't yet got it to work, and the limitation appears to be built into qmake.

softins avatar Mar 05 '22 18:03 softins

There's nothing like a system("lrelease") call I think? Could we run a bash script (or extend the autobuild scripts to run lrelease before compiling)

ann0see avatar Mar 05 '22 19:03 ann0see

There's nothing like a system("lrelease") call I think? Could we run a bash script (or extend the autobuild scripts to run lrelease before compiling)

We can run system commands from .pro files. However, embed_translations does more than what lrelease does. Integrating lrelease directly in .pro files for older Qt releases should be possible with the logic linked above.

hoffie avatar Mar 06 '22 08:03 hoffie

The problem with qmake on Qt < 5.12 is that it complains if the .qm files listed in a resource file do not already exist, and will refuse to add them to the resource .cpp file it generates. This all happens at the time of qmake Jamulus.pro, so if the .qm files are not generated until the subsequent run of make, they do not get embedded.

From 5.12 onwards, qmake doesn't insist that the .qm files exist already. This change was presumably part of adding the embed_translations functionality, as it wouldn't work without.

I haven't had time to think of a way forward yet for old Qt.

softins avatar Mar 06 '22 18:03 softins

The problem with qmake on Qt < 5.12 is that it complains if the .qm files listed in a resource file do not already exist, and will refuse to add them to the resource .cpp file it generates. This all happens at the time of qmake Jamulus.pro, so if the .qm files are not generated until the subsequent run of make, they do not get embedded.

How are the .qm files generated? Is there a way to run, e.g.

  • make distclean || true
  • qmake <args that ignore qm files>
  • make <but just the .qm files please>
  • qmake <args that require qm files>
  • make <main run>

pljones avatar Mar 06 '22 19:03 pljones

Managed to get it working on Qt 5.11, finally. Waiting to see whether the autobuilds all work, particularly Mac Legacy.

I will probably squash before merging, but for now the individual commits might be of interest.

softins avatar Mar 23 '22 23:03 softins

Managed to get the Windows build working, on a debug branch. Still working on the Android build. CONFIG += lrelease embed_translations doesn't appear to play nicely with multi-architecture builds. Unless there is another variable that needs setting.

softins avatar Mar 24 '22 23:03 softins

It looks like several problems mentioned here are related to the problems I discovered in this PR.

pgScorpio avatar Apr 10 '22 14:04 pgScorpio

@softins this needs to be rebased.

Yes, I still haven't worked out what to do about embedding translations for Qt < 5.12. I haven't yet got it to work, and the limitation appears to be built into qmake.

This is probably because in Jamulus.pro the path to the qm files does not match the path of the generated qm files, since lrelease creates subfolders in it's destination folder for debug_and_release or multitarget builds. (See Jamulus.pro of #2588 ) So if qmake also generates Makefile.Xxx files the lrelease output folder will also have Xxx subfolders!

pgScorpio avatar Apr 28 '22 20:04 pgScorpio

De-tagging from 3.9.0 - we can try to get it into 3.9.1.

pljones avatar Jun 18 '22 10:06 pljones

How are the .qm files generated? Is there a way to run, e.g.

make distclean || true qmake make <but just the .qm files please> qmake make

Could we do something like that in and just add the necessary commands to builds which use an older Qt version to the respective autobuild scripts? Then we‘d add documentation in COMPILING.md about what you need to do on old Qt versions

ann0see avatar Aug 06 '22 10:08 ann0see

Hi @softins,

Do you know if you'll be able to start looking at this again in the short term? We're looking to keep 3.9.1 "light" on translation impact anyway (although it introduces a whole new translation platform for the team) but we'd need to decide if this was in or out before going into the release cycle.

Thanks.

pljones avatar Aug 29 '22 16:08 pljones

I'm afraid I still have very little time over the next few weeks, so I think it's best to leave this as pending, and do the 3.9.1 release without it, unless someone else wants to pick it up.

softins avatar Aug 29 '22 17:08 softins

OK bumping to 3.10.0.

pljones avatar Sep 04 '22 10:09 pljones

As this is draft, moving to Triage and dropping the milestone.

pljones avatar Apr 19 '23 15:04 pljones

I mean, what speaks against executing the necessary commands in the mac.sh build script and documenting that Qt < 5.12 needs the files to be generated manually?

ann0see avatar Apr 26 '23 20:04 ann0see

I mean, what speaks against executing the necessary commands in the mac.sh build script and documenting that Qt < 5.12 needs the files to be generated manually?

That means we have different build processes depending on platform. I don't understand why lrelease can't be installed on the MacOS build environment if needed, though, as part of the build process (and cached, ideally).

pljones avatar Apr 28 '23 18:04 pljones

As I think we should be slowly deprecating macOS legacy, I think we could live with an English only legacy build. Would anyone reject that? It's not ideal, but still "ok"

ann0see avatar May 21 '23 16:05 ann0see

Ok. I've now rebased this PR.

ann0see avatar May 21 '23 18:05 ann0see

Closing as stale. If there's further development, please re-open.

ann0see avatar Jul 01 '23 19:07 ann0see