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 system related fixes #479

Merged
merged 6 commits into from Sep 29, 2019

Conversation

@maciejsszmigiero
Copy link
Contributor

maciejsszmigiero commented Sep 28, 2019

This PR contains the following build system related fixes:

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

  2. Fix the issue that make always considers libqtpass.a out of date and rebuilds it.

  3. Make sure that resources are built just once.

  4. Small fixes like localization/*.ts files unnecessairly maked executable or a forgotten localization file left over in qtpass.pro.

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

This comment has been minimized.

Copy link

coveralls commented Sep 28, 2019

Coverage Status

Coverage remained the same at 7.049% when pulling 65aa495 on maciejsszmigiero:build-fixes into 8f2b87b on IJHack:master.

@codecov

This comment has been minimized.

Copy link

codecov bot commented Sep 28, 2019

Codecov Report

Merging #479 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
main/main.cpp 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f2b87b...65aa495. Read the comment docs.

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.
@annejan annejan merged commit 9a06a24 into IJHack:master Sep 29, 2019
4 of 6 checks passed
4 of 6 checks passed
codecov/patch 0% of diff hit (target 7.46%)
Details
codecov/project 7.46% (-0.01%) compared to 8f2b87b
Details
CodeFactor No issues found.
Details
LGTM analysis: C/C++ No new or fixed alerts
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.