-
Notifications
You must be signed in to change notification settings - Fork 223
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 #3288
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pljones
reviewed
Jun 10, 2024
pljones
reviewed
Jun 10, 2024
pljones
reviewed
Jun 10, 2024
softins
force-pushed
the
embed-translations
branch
2 times, most recently
from
June 10, 2024 22:03
384213c
to
e257ad0
Compare
pljones
approved these changes
Jun 11, 2024
ann0see
reviewed
Jun 11, 2024
Previously, translations were embedded manually, via src/resources.qrc Now they are compiled and embedded automatically, using embed_translations, which allows them to be correctly located for multiple targets or archs. Note that RCC_DIR and LRELEASE_DIR can no longer be specified, because the system defaults are necessary for multiple targets/archs to build correctly. The version of Qt must be at least 5.12 for lrelease and embed_translations.
softins
force-pushed
the
embed-translations
branch
from
June 16, 2024 17:14
e257ad0
to
9d0f77c
Compare
The resources generated by embed_translations include the .qm from the filename in the resource name, so it is necessary to remove it. Also, the prefix generated by qmake is /i18n, not /translations.
Now that the .qm files are generated on the fly by the Makefile, a manual lrelease step is no longer needed and wouldn't work correctly.
softins
force-pushed
the
embed-translations
branch
from
June 16, 2024 17:19
9d0f77c
to
05789c6
Compare
Once this update is merged, I can follow it up with a PR to easily solve #3092 |
pljones
approved these changes
Jun 17, 2024
ann0see
reviewed
Jun 20, 2024
ann0see
reviewed
Jun 20, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems ok. I will test on windows
ann0see
added
the
needs documentation
PRs requiring documentation changes or additions
label
Jun 20, 2024
ann0see
approved these changes
Jun 22, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks
5 tasks
This was referenced Jul 23, 2024
Merged
Closed
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
needs documentation
PRs requiring documentation changes or additions
tooling
Changes to the automated build system
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Short description of changes
Generate the translation .qm files during build and remove them from Github.
CHANGELOG: Build: Generate qm files and embedded resource during build
Context: Fixes an issue?
This replaces previous closed PRs #2393 and #1303
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. After a couple of years of intermittent experimentation, this PR is completely working.
The breakthrough was recently discovered in a Qt bug report. Builds for multiple architectures (e.g. on Android) or targets (Windows defaults to
debug_and_release
) only work ifRCC_DIR
andLRELEASE_DIR
are allowed to default and not specified inJamulus.pro
.Does this change need documentation? What needs to be documented and how?
It needs to be made conspicuous that Jamulus will now only build on Qt 5.12 or later.
Jamulus.pro
will causeqmake
to exit with an error on earlier (obsolete) versions of Qt.Status of this Pull Request
Working and ready for review.
What is missing until this pull request can be merged?
Just review and general testing.
Checklist