-
-
Notifications
You must be signed in to change notification settings - Fork 163
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 system related fixes #479
Merged
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
localization/*.ts files are XML (data) files, there is no need for them to be marked executable, but two of them were.
Commits 29e65d7 ("Fix html links color and NL translation building error") and df43cec ("Localization normalized and minor (auto) updates") removed localization/localization_nl.ts file from the source tree and qtpass resources but forgot to remove it from qtpass.pro so it got automatically recreated (nearly empty) every time qmake was run. Remove it from this file, too.
(At least) Linux uses tests/auto/ui/tst_ui and tests/auto/util/tst_util as output file names for built tests, so add these names to .gitignore so they don't pop up as untracked files.
Having compiler_updateqm_make_all target in PRE_TARGETDEPS in src.pro causes make to always consider libqtpass.a out of date and rebuild it. Together with the next commit this will cause main/qtpass to always be considered out of date and so relinked unnecessarily on each make invocation. It looks like this PRE_TARGETDEPS entry isn't actually required as qmake generates proper localization/*.ts -> localization/*.qm dependencies without it anyway, so let's just remove it.
Currently, if one changes some source file in src and then runs make the libqtpass.a gets correctly rebuilt, however main/qtpass isn't actually relinked to make use of this new library version. According to the qmake project file snippet at the very bottom of https://doc.qt.io/qtcreator/creator-project-qmake-libraries.html internal static libraries need to be manually added to their users PRE_TARGETDEPS, too. The same goes for tests that link to libqtpass.a.
Codecov Report
@@ Coverage Diff @@
## master #479 +/- ##
=========================================
- Coverage 7.46% 7.46% -0.01%
=========================================
Files 44 44
Lines 2812 2813 +1
=========================================
Hits 210 210
- Misses 2602 2603 +1
Continue to review full report at Codecov.
|
Currently, the project resources are built twice: the first time in the "main" directory and the second time in the "src" directory. This is because they are specified both in main.pro and src.pro qmake project files (and in qtpass.pro, too). They should be specified in just one such project file instead so they are built just once. Let's leave them in src.pro since that's where localization *.ts -> *.qm dependencies are provided. For this, a Q_INIT_RESOURCE() statement is needed so they are properly linked to by the main application.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR contains the following build system related fixes:
Currently, if one changes some source file in src and then runs make the libqtpass.a gets correctly rebuilt, however main/qtpass isn't actually relinked to make use of this new library version.
According to the qmake project file snippet at the very bottom of https://doc.qt.io/qtcreator/creator-project-qmake-libraries.html internal static libraries need to be manually added to their users PRE_TARGETDEPS, too.
Fix the issue that make always considers libqtpass.a out of date and rebuilds it.
Make sure that resources are built just once.
Small fixes like localization/*.ts files unnecessairly maked executable or a forgotten localization file left over in qtpass.pro.