Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build: Generate qm files and embedded resource during build #2393

Closed
wants to merge 7 commits into from

Conversation

softins
Copy link
Member

@softins softins commented Feb 14, 2022

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

  • I've verified that this Pull Request follows the general code principles
  • I tested my code and it does what I want
  • 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 softins self-assigned this Feb 14, 2022
@softins
Copy link
Member Author

softins commented Feb 14, 2022

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

@pljones
Copy link
Collaborator

pljones commented Feb 15, 2022

Is the legacy MacOS build affected?

@softins
Copy link
Member Author

softins commented Feb 15, 2022

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.

@ann0see
Copy link
Member

ann0see commented Feb 15, 2022

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

@hoffie
Copy link
Member

hoffie commented Feb 15, 2022

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:

@ann0see
Copy link
Member

ann0see commented Mar 5, 2022

@softins this needs to be rebased.

@softins
Copy link
Member Author

softins commented Mar 5, 2022

@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.

@ann0see
Copy link
Member

ann0see commented Mar 5, 2022

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)

@hoffie
Copy link
Member

hoffie commented Mar 6, 2022

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.

@softins
Copy link
Member Author

softins commented Mar 6, 2022

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.

@pljones
Copy link
Collaborator

pljones commented Mar 6, 2022

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>

@softins softins added this to the Release 3.9.0 milestone Mar 19, 2022
@softins softins force-pushed the embed-translations branch 2 times, most recently from d147b2c to 4b6c907 Compare March 23, 2022 23:15
@softins
Copy link
Member Author

softins commented Mar 23, 2022

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 softins marked this pull request as ready for review March 23, 2022 23:19
@softins softins marked this pull request as draft March 24, 2022 23:01
@softins
Copy link
Member Author

softins commented Mar 24, 2022

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.

@pgScorpio
Copy link
Contributor

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

hoffie referenced this pull request in pgScorpio/jamulus Apr 14, 2022
@pgScorpio
Copy link
Contributor

@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!

@pljones
Copy link
Collaborator

pljones commented Jun 18, 2022

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

@pljones pljones modified the milestones: Release 3.9.0, Release 3.9.1 Jun 18, 2022
@pljones
Copy link
Collaborator

pljones commented Sep 4, 2022

OK bumping to 3.10.0.

@pljones pljones modified the milestones: Release 3.9.1, Release 3.10.0 Sep 4, 2022
@pljones pljones changed the title Generate qm files and embedded resource during build Build: Generate qm files and embedded resource during build Oct 5, 2022
@ann0see ann0see changed the base branch from master to main December 26, 2022 19:10
@pljones
Copy link
Collaborator

pljones commented Apr 19, 2023

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

@pljones pljones removed this from the Release 3.10.0 milestone Apr 19, 2023
@ann0see
Copy link
Member

ann0see commented Apr 26, 2023

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?

@pljones
Copy link
Collaborator

pljones commented Apr 28, 2023

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).

@ann0see
Copy link
Member

ann0see commented May 21, 2023

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
Copy link
Member

ann0see commented May 21, 2023

Ok. I've now rebased this PR.

@ann0see
Copy link
Member

ann0see commented Jul 1, 2023

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

@softins
Copy link
Member Author

softins commented Jun 6, 2024

After two years and a lot of experimentation and head scratching, I have finally solved this for all builds. I will open a fresh PR for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants