Differential D8577

Fix: Missing dependencies for ktexeditor autotests
ClosedPublic
Actions

Authored by hgoebel on Oct 31 2017, 4:04 PM.

Details

Reviewers
dfaure
Group Reviewers
KTextEditor
Commits
R39:d6fc0b782046: Fix: Missing dependencies for ktexeditor autotests
Summary

When building ktexeditor autotests for guix[1], building the following autotests fail:

  • autotests/src/scripting_test.cpp
  • autotests/src/vimode/base.cpp

The reasons are missing dependencies in autotests/CMakeLists.txt resp. autotests/src/vimode/CMakeLists.txt – and related missing dependencies in CMakeLists.txt.

See https://phabricator.kde.org/D8112 (already merged) for the same kind of problem for kwin.

[1] https://www.gnu.org/software/guix/

Detailed description and Analysis
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

guix (like nix) used a separate prefix for each package. Thus the header files for qtdeclarative are in /gnu/store/…-qtdeclarative-5.9.1/include/qt5/ while the ones for qtbase are in /gnu/store/…-base-5.9.1/include/qt5/. This means that *each* dependency's include directory must be specified. This is in contrast to a "normal" Unix-system, where all includes end up in /usr/include/qt5 and missing to define some include-dependencies does often not raise an error.

Diff Detail

Repository
R39 KTextEditor
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
hgoebel created this revision.Oct 31 2017, 4:04 PM
hgoebel created this object with visibility "No One".
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptOct 31 2017, 4:04 PM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
hgoebel updated this revision to Diff 21656.Oct 31 2017, 4:26 PM
hgoebel changed the visibility from "No One" to "Public (No Login Required)".
hgoebel added a reviewer: KTextEditor.
hgoebel added a comment.Oct 31 2017, 4:31 PM

Please note that I do not have commit rights, so please somebody else commit.

dfaure accepted this revision.Nov 5 2017, 9:01 PM
This revision is now accepted and ready to land.Nov 5 2017, 9:01 PM
Closed by commit R39:d6fc0b782046: Fix: Missing dependencies for ktexeditor autotests (authored by hgoebel, committed by dfaure). · Explain WhyNov 5 2017, 9:01 PM
This revision was automatically updated to reflect the committed changes.
dhaumann added a subscriber: dhaumann.Nov 5 2017, 10:10 PM

I am a bit surprised by this commit: we just removed the dependency on QtScript in favor of QtQML and now it's back. I don't think this hurts much, but the goal is to not depend on it... ?!

hgoebel added a comment.Nov 5 2017, 11:44 PM

QtScript is only required for the tests. Maybe just a left-over #include?

cullmann added a subscriber: cullmann.Nov 6 2017, 7:09 AM

Hmm, I would rather like to have this fixed than having that dependency just for that.
I thought during the port it compiled completely without QtScript.

hgoebel added a comment.Nov 6 2017, 9:35 AM

Well, the currently the dependency is still needed, see https://cgit.kde.org/ktexteditor.git/tree/autotests/src/scripting_test.cpp#n51:

#include <QtScript/QScriptEngine>

If the code is not compiled in an isolated and controlled environment, and QtScript is installed on the system, the compiler might silently get the includes installed elsewhere. The same is true, if some QT package requires QtScript and QtScript is installed alongside that package.

cullmann added a comment.Nov 6 2017, 9:36 AM

Could you just remove the problematic includes and try to compile without the dependency, IMHO I see no use of QtScript stuff.

hgoebel added a comment.Nov 6 2017, 10:12 AM

My aim is to bring Plasma into GuixSD, not to develop ktexteditor. So please understand that I'm not going to work on ktexteditor.

kfunk added a subscriber: kfunk.Nov 6 2017, 11:01 AM
In D8577#164815, @cullmann wrote:

Could you just remove the problematic includes and try to compile without the dependency, IMHO I see no use of QtScript stuff.

Done with: 631b1447c97f1d81de2e81d2556474f0bc3109dd

Revision Contents
Changeset List

Diff 21931

View Options

CMakeLists.txt

Loading...
View Options

autotests/CMakeLists.txt

Loading...
View Options

autotests/src/vimode/CMakeLists.txt

Loading...