Re: Review Request for KWayland for inclusion in frameworks
On Saturday, May 7, 2016 7:45:20 PM CEST Michael Pyne wrote: > On Sat, May 7, 2016 13:50:05 David Faure wrote: > > Hi guys, > > > > Can you check the CI for KWayland? ASAN detects an invalid memory usage > > in KWayland::Client::OutputDevice::Private::doneCallback() called after > > KWayland::Client::OutputDevice::~OutputDevice(). > > > > https://build.kde.org/view/Frameworks%20kf5-qt5/job/kwayland%20master%20kf > > 5-> > > qt5/28/PLATFORM=Linux,compiler=gcc/testReport/junit/%28root%29/TestSuite/ > > kwa yland_testWaylandOutputDevice/ > > I find that report kind of confusing. The line 323 of > test_wayland_outputdevice.cpp is just a QSignalSpy constructor, it shouldn't > involve destruction of an OutputDevice (what ASAN is warning about here). I > wonder if it maybe has something to do with the modern signal/slot > connection syntax in use? > > For what it's worth there's a relevant Coverity entry (CID 1340943), noting > that the TestWaylandOutputDevice class does not have an initializer for > m_queue within the constructor (or in any of the other class members it > looked at). But I don't see how that would be related either. This test has been flaky for some time. A rerun normally succeeds (as it just did) and whenever I tried to reproduce locally I failed. Sorry I have no idea (yet) about the test and so far just triggered rebuild whenever the test failed. Cheers Martin signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request for KWayland for inclusion in frameworks
On Sat, May 7, 2016 13:50:05 David Faure wrote: > Hi guys, > > Can you check the CI for KWayland? ASAN detects an invalid memory usage > in KWayland::Client::OutputDevice::Private::doneCallback() called after > KWayland::Client::OutputDevice::~OutputDevice(). > > https://build.kde.org/view/Frameworks%20kf5-qt5/job/kwayland%20master%20kf5-> > qt5/28/PLATFORM=Linux,compiler=gcc/testReport/junit/%28root%29/TestSuite/kwa > yland_testWaylandOutputDevice/ I find that report kind of confusing. The line 323 of test_wayland_outputdevice.cpp is just a QSignalSpy constructor, it shouldn't involve destruction of an OutputDevice (what ASAN is warning about here). I wonder if it maybe has something to do with the modern signal/slot connection syntax in use? For what it's worth there's a relevant Coverity entry (CID 1340943), noting that the TestWaylandOutputDevice class does not have an initializer for m_queue within the constructor (or in any of the other class members it looked at). But I don't see how that would be related either. Regards, - Michael Pyne ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request for KWayland for inclusion in frameworks
Hi guys, Can you check the CI for KWayland? ASAN detects an invalid memory usage in KWayland::Client::OutputDevice::Private::doneCallback() called after KWayland::Client::OutputDevice::~OutputDevice(). https://build.kde.org/view/Frameworks%20kf5-qt5/job/kwayland%20master%20kf5-qt5/28/PLATFORM=Linux,compiler=gcc/testReport/junit/%28root%29/TestSuite/kwayland_testWaylandOutputDevice/ -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request for KWayland for inclusion in frameworks
On Tuesday 03 May 2016 07:59:29 Martin Graesslin wrote: > Hi David, > > did you forget to update the version number of KWayland to 5.22 or is > something still missing in KWayland? Neither, kwayland just wasn't in the list of modules yet, that's further in the process. The step1 version upgrade that just happened was supposed to happen right after 5.21 was out, I simply forgot. At that time new frameworks are not there yet. At 5.22 release time I would have noticed it: [...] * In case this detected new frameworks, make sure to increase its version numbers (step1/step2 before missed it) [...] Anyway, done now ;) -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request for KWayland for inclusion in frameworks
Hi David, did you forget to update the version number of KWayland to 5.22 or is something still missing in KWayland? Cheers Martin On Sunday, April 17, 2016 10:42:08 AM CEST David Faure wrote: > Hi Martin, > > Very nice run down of the check list! > > I approve the move of kwayland to frameworks, please proceed > (preferrably earlier than in two weeks, since that would be close to release > date). > > Cheers, > David. > > On Friday 15 April 2016 10:08:57 Martin Graesslin wrote: > > Hi frameworks-developers, > > > > based on the thread "KWayland as framework" [1] I want to start the review > > process for including KWayland into frameworks. > > > > To make it easier I'm giving an overview: > > Policies [2]: > > * Tier 1/integration - dependencies on Qt::Gui (5.4) and Wayland (1.7) > > * directory structure > > > > ** name of toplevel directory: kwayland > > ** README.md: check > > ** COPYING.LIB: check > > ** metadata.yaml: check > > ** docs subdirectory: no additional documentation > > ** src with dedicated subdirectories: check, there is src/client and src/ > > > > server > > > > ** examples subdirectory: no dedicated examples > > ** autotests subdirectory: check > > ** tests subdirectory: check > > ** cmake subdirectory: not needed > > > > * automatic unit test: the current line coverage for client library is 86 > > %, the line coverage for server library is 80 % > > * binary compatibility: KWayland has been ABI stable since it got added to > > Plasma > > * documentation: yes, the API is documented > > * localization: not needed, no user visible strings > > > > * buildsystem: > > ** KF5WaylandConfig.cmake is generated > > ** KF5WaylandConfigVersion.cmake is generated > > ** no other cmake files are installed > > ** library names are camel case: KF5WaylandServer and KF5WaylandClient > > ** dependencies to other frameworks: it's tier 1, doesn't apply > > > > * commits are reviewed: yes, but KWayland is already using phabricator > > * CI failures are treated as stop the line events: I like my view green > > ;-) > > > > * compiler requirements: > > ** gcc 4.5: I don't know, the minimum gcc version my distribution offers > > is > > > > 4.7 > > > > ** clang 3.1: I don't know, the minimum clang version my distribution > > offers> > > is 3.5 > > > > ** VS2012: doesn't apply, it's Linux only [5] > > ** constraints for other C++11 features: it compiles on build.kde.org, so > > > > should be fine > > > > Guidelines [3]: > > * Policies: see above > > * translations: doesn't apply > > * split repository: yes, all commits were extracted when creating the > > project * astyle: the code follows the kdelibs coding style from the > > start, didn't run astyle again > > * Dependencies on deprecated API: none > > * module in frameworks: not yet, currently still in Plasma [4] > > * kde-build-metadata: not yet, currently still in Plasma [4] > > * CI Jobs: currently still for Plasma [4]: > > https://build.kde.org/job/kwayland %20master%20kf5-qt5/ > > * bugs.kde.org project: currently still in Plasma [4] > > * reviewboard: yes > > * README.md: yes > > > > If you have any further questions regarding the move, please ask. If I > > don't get any blocking feedback in the next two weeks, I'm going to > > request the move and hope to get it into next frameworks release if that > > works with David ;-) > > > > Thanks, > > Martin > > > > [1] > > https://mail.kde.org/pipermail/kde-frameworks-devel/2016-March/032116.htm > > l [2] https://community.kde.org/Frameworks/Policies > > [3] https://community.kde.org/Frameworks/CreationGuidelines > > [4] I'll do those tasks if we get the green light for move to frameworks, > > otherwise it'll stay in Plasma > > [5] some code uses linux includes, to support other Unix-systems porting > > is > > needed signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request for KWayland for inclusion in frameworks
On Friday, April 15, 2016 10:08:57 AM CEST Martin Graesslin wrote: > Hi frameworks-developers, short update > * module in frameworks: not yet, currently still in Plasma [4] done > * kde-build-metadata: not yet, currently still in Plasma [4] done > * CI Jobs: currently still for Plasma [4]: > https://build.kde.org/job/kwayland %20master%20kf5-qt5/ done > * bugs.kde.org project: currently still in Plasma [4] done I think we have everything in place now. Cheers Martin signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request for KWayland for inclusion in frameworks
Hi Martin, Very nice run down of the check list! I approve the move of kwayland to frameworks, please proceed (preferrably earlier than in two weeks, since that would be close to release date). Cheers, David. On Friday 15 April 2016 10:08:57 Martin Graesslin wrote: > Hi frameworks-developers, > > based on the thread "KWayland as framework" [1] I want to start the review > process for including KWayland into frameworks. > > To make it easier I'm giving an overview: > Policies [2]: > * Tier 1/integration - dependencies on Qt::Gui (5.4) and Wayland (1.7) > * directory structure > ** name of toplevel directory: kwayland > ** README.md: check > ** COPYING.LIB: check > ** metadata.yaml: check > ** docs subdirectory: no additional documentation > ** src with dedicated subdirectories: check, there is src/client and src/ > server > ** examples subdirectory: no dedicated examples > ** autotests subdirectory: check > ** tests subdirectory: check > ** cmake subdirectory: not needed > * automatic unit test: the current line coverage for client library is 86 %, > the line coverage for server library is 80 % > * binary compatibility: KWayland has been ABI stable since it got added to > Plasma > * documentation: yes, the API is documented > * localization: not needed, no user visible strings > * buildsystem: > ** KF5WaylandConfig.cmake is generated > ** KF5WaylandConfigVersion.cmake is generated > ** no other cmake files are installed > ** library names are camel case: KF5WaylandServer and KF5WaylandClient > ** dependencies to other frameworks: it's tier 1, doesn't apply > * commits are reviewed: yes, but KWayland is already using phabricator > * CI failures are treated as stop the line events: I like my view green ;-) > * compiler requirements: > ** gcc 4.5: I don't know, the minimum gcc version my distribution offers is > 4.7 > ** clang 3.1: I don't know, the minimum clang version my distribution > offers > is 3.5 > ** VS2012: doesn't apply, it's Linux only [5] > ** constraints for other C++11 features: it compiles on build.kde.org, so > should be fine > > Guidelines [3]: > * Policies: see above > * translations: doesn't apply > * split repository: yes, all commits were extracted when creating the project > * astyle: the code follows the kdelibs coding style from the start, didn't > run > astyle again > * Dependencies on deprecated API: none > * module in frameworks: not yet, currently still in Plasma [4] > * kde-build-metadata: not yet, currently still in Plasma [4] > * CI Jobs: currently still for Plasma [4]: https://build.kde.org/job/kwayland > %20master%20kf5-qt5/ > * bugs.kde.org project: currently still in Plasma [4] > * reviewboard: yes > * README.md: yes > > If you have any further questions regarding the move, please ask. If I don't > get any blocking feedback in the next two weeks, I'm going to request the > move > and hope to get it into next frameworks release if that works with David ;-) > > Thanks, > Martin > > [1] https://mail.kde.org/pipermail/kde-frameworks-devel/2016-March/032116.html > [2] https://community.kde.org/Frameworks/Policies > [3] https://community.kde.org/Frameworks/CreationGuidelines > [4] I'll do those tasks if we get the green light for move to frameworks, > otherwise it'll stay in Plasma > [5] some code uses linux includes, to support other Unix-systems porting is > needed -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request for KWayland for inclusion in frameworks
Hi frameworks-developers, based on the thread "KWayland as framework" [1] I want to start the review process for including KWayland into frameworks. To make it easier I'm giving an overview: Policies [2]: * Tier 1/integration - dependencies on Qt::Gui (5.4) and Wayland (1.7) * directory structure ** name of toplevel directory: kwayland ** README.md: check ** COPYING.LIB: check ** metadata.yaml: check ** docs subdirectory: no additional documentation ** src with dedicated subdirectories: check, there is src/client and src/ server ** examples subdirectory: no dedicated examples ** autotests subdirectory: check ** tests subdirectory: check ** cmake subdirectory: not needed * automatic unit test: the current line coverage for client library is 86 %, the line coverage for server library is 80 % * binary compatibility: KWayland has been ABI stable since it got added to Plasma * documentation: yes, the API is documented * localization: not needed, no user visible strings * buildsystem: ** KF5WaylandConfig.cmake is generated ** KF5WaylandConfigVersion.cmake is generated ** no other cmake files are installed ** library names are camel case: KF5WaylandServer and KF5WaylandClient ** dependencies to other frameworks: it's tier 1, doesn't apply * commits are reviewed: yes, but KWayland is already using phabricator * CI failures are treated as stop the line events: I like my view green ;-) * compiler requirements: ** gcc 4.5: I don't know, the minimum gcc version my distribution offers is 4.7 ** clang 3.1: I don't know, the minimum clang version my distribution offers is 3.5 ** VS2012: doesn't apply, it's Linux only [5] ** constraints for other C++11 features: it compiles on build.kde.org, so should be fine Guidelines [3]: * Policies: see above * translations: doesn't apply * split repository: yes, all commits were extracted when creating the project * astyle: the code follows the kdelibs coding style from the start, didn't run astyle again * Dependencies on deprecated API: none * module in frameworks: not yet, currently still in Plasma [4] * kde-build-metadata: not yet, currently still in Plasma [4] * CI Jobs: currently still for Plasma [4]: https://build.kde.org/job/kwayland %20master%20kf5-qt5/ * bugs.kde.org project: currently still in Plasma [4] * reviewboard: yes * README.md: yes If you have any further questions regarding the move, please ask. If I don't get any blocking feedback in the next two weeks, I'm going to request the move and hope to get it into next frameworks release if that works with David ;-) Thanks, Martin [1] https://mail.kde.org/pipermail/kde-frameworks-devel/2016-March/032116.html [2] https://community.kde.org/Frameworks/Policies [3] https://community.kde.org/Frameworks/CreationGuidelines [4] I'll do those tasks if we get the green light for move to frameworks, otherwise it'll stay in Plasma [5] some code uses linux includes, to support other Unix-systems porting is needed signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
