zzag / kwin-effects-yet-another-magic-lamp

Just Yet Another Magic Lamp effect
GNU General Public License v2.0
160 stars 27 forks source link

Builds for slightly older KDE packages fail on Launchpad #3

Closed krisives closed 5 years ago

krisives commented 5 years ago

Firstly, thanks for all your hard work related to KDE. You're a legend!

Relevant part of build failure:

/<<PKGBUILDDIR>>/src/Model.h:44:22: error: field ‘shapeCurve’ has incomplete type ‘QEasingCurve’
         QEasingCurve shapeCurve;
                      ^~~~~~~~~~
In file included from /usr/include/x86_64-linux-gnu/qt5/QtCore/qobject.h:54:0,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/qcoreapplication.h:46,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/QCoreApplication:1,
                 from /usr/include/kwinglobals.h:24,
                 from /usr/include/kwineffects.h:28,
                 from /<<PKGBUILDDIR>>/src/Model.h:24,
                 from /<<PKGBUILDDIR>>/src/Model.cc:19:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qmetatype.h:1881:1: note: forward declaration of ‘class QEasingCurve’
 QT_FOR_EACH_STATIC_CORE_CLASS(QT_FORWARD_DECLARE_STATIC_TYPES_ITER)
 ^
In file included from /<<PKGBUILDDIR>>/src/Model.cc:19:0:
/<<PKGBUILDDIR>>/src/Model.h:151:11: error: ‘TimeLine’ in namespace ‘KWin’ does not name a type
     KWin::TimeLine m_timeLine;
           ^~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc: In member function ‘void Model::start(Model::AnimationKind)’:
/<<PKGBUILDDIR>>/src/Model.cc:36:9: error: ‘m_timeLine’ was not declared in this scope
     if (m_timeLine.running()) {
         ^~~~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:36:9: note: suggested alternative: ‘__timezone’
     if (m_timeLine.running()) {
         ^~~~~~~~~~
         __timezone
/<<PKGBUILDDIR>>/src/Model.cc:49:13: error: ‘m_timeLine’ was not declared in this scope
             m_timeLine.reset();
             ^~~~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:49:13: note: suggested alternative: ‘__timezone’
             m_timeLine.reset();
             ^~~~~~~~~~
             __timezone
/<<PKGBUILDDIR>>/src/Model.cc:50:43: error: ‘KWin::TimeLine’ has not been declared
             m_timeLine.setDirection(KWin::TimeLine::Forward);
                                           ^~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:52:53: error: incomplete type ‘QEasingCurve’ used in nested name specifier
             m_timeLine.setEasingCurve(QEasingCurve::Linear);
                                                     ^~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:56:13: error: ‘m_timeLine’ was not declared in this scope
             m_timeLine.reset();
             ^~~~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:56:13: note: suggested alternative: ‘__timezone’
             m_timeLine.reset();
             ^~~~~~~~~~
             __timezone
/<<PKGBUILDDIR>>/src/Model.cc:57:43: error: ‘KWin::TimeLine’ has not been declared
             m_timeLine.setDirection(KWin::TimeLine::Forward);
                                           ^~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:60:53: error: incomplete type ‘QEasingCurve’ used in nested name specifier
             m_timeLine.setEasingCurve(QEasingCurve::Linear);
                                                     ^~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:67:9: error: ‘m_timeLine’ was not declared in this scope
         m_timeLine.reset();
         ^~~~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:67:9: note: suggested alternative: ‘__timezone’
         m_timeLine.reset();
         ^~~~~~~~~~
         __timezone
/<<PKGBUILDDIR>>/src/Model.cc:68:39: error: ‘KWin::TimeLine’ has not been declared
         m_timeLine.setDirection(KWin::TimeLine::Backward);
                                       ^~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:70:49: error: incomplete type ‘QEasingCurve’ used in nested name specifier
         m_timeLine.setEasingCurve(QEasingCurve::Linear);
                                                 ^~~~~~
/<<PKGBUILDDIR>>/src/Model.cc: In member function ‘void Model::step(std::chrono::milliseconds)’:
/<<PKGBUILDDIR>>/src/Model.cc:81:5: error: ‘m_timeLine’ was not declared in this scope
     m_timeLine.update(delta);
     ^~~~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:81:5: note: suggested alternative: ‘__timezone’
     m_timeLine.update(delta);
     ^~~~~~~~~~
     __timezone
/<<PKGBUILDDIR>>/src/Model.cc: In member function ‘void Model::updateMinimizeStage()’:
/<<PKGBUILDDIR>>/src/Model.cc:105:9: error: ‘m_timeLine’ was not declared in this scope
         m_timeLine.reset();
         ^~~~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:105:9: note: suggested alternative: ‘__timezone’
         m_timeLine.reset();
         ^~~~~~~~~~
         __timezone
/<<PKGBUILDDIR>>/src/Model.cc:106:39: error: ‘KWin::TimeLine’ has not been declared
         m_timeLine.setDirection(KWin::TimeLine::Forward);
                                       ^~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:109:49: error: incomplete type ‘QEasingCurve’ used in nested name specifier
         m_timeLine.setEasingCurve(QEasingCurve::Linear);
                                                 ^~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:118:39: error: ‘KWin::TimeLine’ has not been declared
         m_timeLine.setDirection(KWin::TimeLine::Forward);
                                       ^~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:120:49: error: incomplete type ‘QEasingCurve’ used in nested name specifier
         m_timeLine.setEasingCurve(QEasingCurve::Linear);
                                                 ^~~~~~
/<<PKGBUILDDIR>>/src/Model.cc: In member function ‘void Model::updateUnminimizeStage()’:
/<<PKGBUILDDIR>>/src/Model.cc:147:9: error: ‘m_timeLine’ was not declared in this scope
         m_timeLine.reset();
         ^~~~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:147:9: note: suggested alternative: ‘__timezone’
         m_timeLine.reset();
         ^~~~~~~~~~
         __timezone
/<<PKGBUILDDIR>>/src/Model.cc:148:39: error: ‘KWin::TimeLine’ has not been declared
         m_timeLine.setDirection(KWin::TimeLine::Backward);
                                       ^~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:150:49: error: incomplete type ‘QEasingCurve’ used in nested name specifier
         m_timeLine.setEasingCurve(QEasingCurve::Linear);
                                                 ^~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:162:39: error: ‘KWin::TimeLine’ has not been declared
         m_timeLine.setDirection(KWin::TimeLine::Backward);
                                       ^~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:165:49: error: incomplete type ‘QEasingCurve’ used in nested name specifier
         m_timeLine.setEasingCurve(QEasingCurve::Linear);
                                                 ^~~~~~
/<<PKGBUILDDIR>>/src/Model.cc: At global scope:
/<<PKGBUILDDIR>>/src/Model.cc:181:18: error: field ‘shapeCurve’ has incomplete type ‘QEasingCurve’
     QEasingCurve shapeCurve;
                  ^~~~~~~~~~
In file included from /usr/include/x86_64-linux-gnu/qt5/QtCore/qobject.h:54:0,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/qcoreapplication.h:46,
                 from /usr/include/x86_64-linux-gnu/qt5/QtCore/QCoreApplication:1,
                 from /usr/include/kwinglobals.h:24,
                 from /usr/include/kwineffects.h:28,
                 from /<<PKGBUILDDIR>>/src/Model.h:24,
                 from /<<PKGBUILDDIR>>/src/Model.cc:19:
/usr/include/x86_64-linux-gnu/qt5/QtCore/qmetatype.h:1881:1: note: forward declaration of ‘class QEasingCurve’
 QT_FOR_EACH_STATIC_CORE_CLASS(QT_FORWARD_DECLARE_STATIC_TYPES_ITER)
 ^
/<<PKGBUILDDIR>>/src/Model.cc: In member function ‘void Model::applyBump(QVector<WindowQuad>&) const’:
/<<PKGBUILDDIR>>/src/Model.cc:385:27: error: ‘m_timeLine’ was not declared in this scope
     params.bumpProgress = m_timeLine.value();
                           ^~~~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:385:27: note: suggested alternative: ‘__timezone’
     params.bumpProgress = m_timeLine.value();
                           ^~~~~~~~~~
                           __timezone
/<<PKGBUILDDIR>>/src/Model.cc: In member function ‘void Model::applyStretch1(QVector<WindowQuad>&) const’:
/<<PKGBUILDDIR>>/src/Model.cc:398:46: error: ‘m_timeLine’ was not declared in this scope
     params.stretchProgress = m_shapeFactor * m_timeLine.value();
                                              ^~~~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:398:46: note: suggested alternative: ‘__timezone’
     params.stretchProgress = m_shapeFactor * m_timeLine.value();
                                              ^~~~~~~~~~
                                              __timezone
/<<PKGBUILDDIR>>/src/Model.cc: In member function ‘void Model::applyStretch2(QVector<WindowQuad>&) const’:
/<<PKGBUILDDIR>>/src/Model.cc:412:46: error: ‘m_timeLine’ was not declared in this scope
     params.stretchProgress = m_shapeFactor * m_timeLine.value();
                                              ^~~~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:412:46: note: suggested alternative: ‘__timezone’
     params.stretchProgress = m_shapeFactor * m_timeLine.value();
                                              ^~~~~~~~~~
                                              __timezone
/<<PKGBUILDDIR>>/src/Model.cc: In member function ‘void Model::applySquash(QVector<WindowQuad>&) const’:
/<<PKGBUILDDIR>>/src/Model.cc:425:29: error: ‘m_timeLine’ was not declared in this scope
     params.squashProgress = m_timeLine.value();
                             ^~~~~~~~~~
/<<PKGBUILDDIR>>/src/Model.cc:425:29: note: suggested alternative: ‘__timezone’
     params.squashProgress = m_timeLine.value();
                             ^~~~~~~~~~
                             __timezone
zzag commented 5 years ago

The problem is this effect needs the TimeLine class, which I added in 5.14. So, one possible way to fix this issue is to add an ifdef and include our own implementation of TimeLine.

Also, I'd like to point out that libkwineffects is currently not ABI stable so after each KDE Plasma release it would be great to repackage this effect.

krisives commented 5 years ago

You're awesome. I will retry the build.

zzag commented 5 years ago

Please let me know whether it's successful now.

krisives commented 5 years ago

Yup it works now: https://code.launchpad.net/~krisives/+recipe/kwin-effects-yaml-daily

I will be testing it more by installing the packages etc. and let you know if anything goes wrong.

Thanks again.

zzag commented 5 years ago

Oh, cool, thanks! :-)

krisives commented 5 years ago

Tested the PPA packages and they work great. Would you accept a PR for adding the PPA instructions in the README.md so that Ubuntu/Debian/Neon users can use this without much hassle? The instructions would be:

sudo add-apt-repository ppa:krisives/kwin-effects-yaml
sudo apt install kwin-effects-yaml
zzag commented 5 years ago

Would you accept a PR for adding the PPA instructions in the README.md so that Ubuntu/Debian/Neon users can use this without much hassle?

Sure, go for it.

krisives commented 5 years ago

Sorry to bug you about this more. After some testing I found out that the packages that Launchpad is building don't work because -DCMAKE_BUILD_TYPE=None is a rule in Debian, for whatever reason, which when I build and run make install with this project causes it not to be seen as an effects plugin. It took me a while to figure this out because I had to get into the habit of logging out and logging into KDE to ensure the tests weren't giving me false positives with stale metadata.

Do you know any reason why the default for -DCMAKE_BUILD_TYPE=None makes it not work? I checked the produced .deb and files installed by make install and they seem to be the same file paths/names - although comparing the exact content difference is a bit tricky since they are binary files.

Thanks for any help.

zzag commented 5 years ago

If the effect is built against 5.13, but you use 5.14, then the effect won't work.

krisives commented 5 years ago

This is on a local development system building with 5.14 simply using the same cmake build commands from the log, which contains the -DCMAKE_BUILD_TYPE=None flag.

zzag commented 5 years ago

Works fine for me.

Just to be sure: have you deleted the old effect? For some obvious reasons, I changed the name of the plugin, sorry.

krisives commented 5 years ago

This is using a copy from Git before any changes you made at 4c1ec5a14db2554c877c31e4d8e48df40ec75779

Here is the build command being used to simulate what Launchpad was doing:

mkdir obj-x86_64-linux-gnu
cd obj-x86_64-linux-gnu

cmake .. -DCMAKE_INSTALL_PREFIX=/usr \
 -DCMAKE_VERBOSE_MAKEFILE=ON \
 -DCMAKE_BUILD_TYPE=None \
 -DCMAKE_INSTALL_SYSCONFDIR=/etc \
 -DCMAKE_INSTALL_LOCALSTATEDIR=/var \
 -DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=ON \
 -DCMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY=ON

make -j4 -O

When I change from using -DCMAKE_BUILD_TYPE=None to -DCMAKE_BUILD_TYPE=Release it installs and plasma sees it fine, but when I use the None version of the command (after uninstalling, deleting everything, logging out, and logging back in checking the Desktop Effects every step of the way to make sure it's not cached or using stale metadata)

I haven't looked at the name change yet but I don't think the packaging will care about that as long as cmake .. with make install works the same. I have been packaging a lot of CMake stuff for KDE over the last month and this is the first time I have seen a these flags cause a problem so I am confused.

I can work around this by manually specifying specific flags for cmake, but before I do that I want to understand why this is happening and if it's a problem.

krisives commented 5 years ago

I decided to override the build process and force -DCMAKE_BUILD_TYPE=Release on the debian/rules file which fixes the problem and it installs correctly, but then I hit the issue you described earlier with trying to use a 5.13 build on a 5.14 system.

It works if you manually specify using a new version for the PPA like disco, but that sucks since it's not default behavior and the end result is the PPA would seem to fail for most users out of the box. I don't know of a good way to make the build recipe bring in newer versions of KDE or if that's even possible.

The only solution I would have right now would be to grab the .deb files from the disco version of the PPA downloads page and upload them to GitHub as a releases, but that kinda defeats the purpose of having a recipe that automatically re-builds things.

It's unfortunate too because I think a lot of users that aren't experienced enough to manually compile things would really like to use this.