Re: Web Shortcuts KCM
Hello, On Monday 14 July 2014 12:46:15 David Faure wrote: On Monday 14 July 2014 12:34:44 Eike Hein wrote: I'd like to port the ebrowsing KCM and move it to plasma-desktop or -workspace, since it has plenty of users outside of Konq (e.g. Konvi, Konsole and Okular, the first two of which have ports now). Thoughts? The whole split between workspace and applications seems to generate more issues than we thought. we're going to miss a place for shared runtime deps. If someone uses konvi, konsole, okular, or konqueror outside of plasma- workspace, (e.g. in gnome, Mac or Windows), then they are not going to be able to configure cookies, internet keywords, useragent, proxies . if these things are part of the workspace. And as such shall be configured in the workspace no? I mean... cookies, proxies and so on, that looks wrong if they are configured by some runtime bits provided by a framework knowing the platforms have such settings. I also kills all hopes to see something like KIO have a lower tier for instance. Given that the kurifilter plugins themselves are in KIO for this same reason, maybe the ebrowsing and the kio (cookies,proxies,useragent) KCMs should go into KIO as well? Honestly to me it looks like a wrong move. A better move would be to have the framework (e.g. KIO) read the settings from the platform for its settings (e.g. proxies). We ought to treat our workspaces like one of the platforms our frameworks can be used with, not try to bring workspace bits in our frameworks. That would go counter to our initial goals. Regards. -- Kévin Ottens, http://ervin.ipsquad.net KDAB - proud supporter of KDE, http://www.kdab.com 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: Web Shortcuts KCM
On Monday 14 July 2014 13:15:32 John Layt wrote: Over on the Windows list we've been discussing about KCM's for configuring common services/frameworks like this when running apps under non-Plasma desktops, including Gnome, Windows, Mac, etc. General gist is that we don't want to have systemsettings installed in non-Plasma platforms to gain access to them, so they need to be accessed through the apps config dialog or help menu instead. This implies a few things: 1) That the KCM's are located somewhere easily packaged for stand-alone app installers to include 2) That an app can figure out what KCM's it needs access to when running non-native 3) That at runtime the app can detect it is running in non-native mode and find and load the required external KCMs that it needs [...] Thoughts? Or maybe it is that an application which depends on such a KCM to be present to be configured is not portable? After all, it is a sign of a direct dependency on a low level setting from the application. Maybe that's fine, maybe not all applications are meant to run outside of a Plasma workspace. What's not fine IMO is to turn a framework into a bit of workspace with UI to workaround that. From what I kept above, I got that feeling that we end up in that situation mainly because: we got some settings which should be application settings but end up appearing in systemsettings instead and we got applications/frameworks reading settings from our low level settings unconditionally while they should read from the platform settings instead (Plasma being one among several). Regards. -- Kévin Ottens, http://ervin.ipsquad.net KDAB - proud supporter of KDE, http://www.kdab.com 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 119280: Add the Web Shortcuts KCM from kde-baseapps/konq to the KIO framework
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119280/#review62372 --- Note that as mentioned in the Web Shortcuts KCM thread, I'm very much not in favor of such a code move. - Kevin Ottens On July 14, 2014, 8:36 p.m., Eike Hein wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119280/ --- (Updated July 14, 2014, 8:36 p.m.) Review request for KDE Frameworks and David Faure. Repository: kio Description --- As discussed, this adds the Web Shortcuts KCM, formerly shipped as part as Konqueror, to the KIO framework, where the URI Filters framework it configures resides as well. This makes more sense than stuffing it into workspace, since Web Shortcuts have many app level downstreams (e.g. Konversation, Okular and Konsole) which try to run the KCM via kcmshell5, and may not be running inside Plasma Desktop at the time. I've lightly modified the code to make it build, and made the naming more consistent - webshortcuts is now used throughout where previously was a mix of ebrowsing and kurifilt. This means .po name changed, but the KCM only contains a single string. More importantly it means apps calling the KCM via kcmshell5 need to be changed to use the new name - I promise to take care of that. I'm the least confident on the CMake stuff, especially the TRANSLATION_DOMAIN redefinition, so I'd be happy for review. Note the categorization is already what the recategorization effort proscribed for this KCM. Diffs - src/CMakeLists.txt 6f8373f src/kcms/CMakeLists.txt PRE-CREATION src/kcms/webshortcuts/CMakeLists.txt PRE-CREATION src/kcms/webshortcuts/Messages.sh PRE-CREATION src/kcms/webshortcuts/main.h PRE-CREATION src/kcms/webshortcuts/main.cpp PRE-CREATION src/kcms/webshortcuts/webshortcuts.desktop PRE-CREATION src/urifilters/ikws/CMakeLists.txt 4efe24e Diff: https://git.reviewboard.kde.org/r/119280/diff/ Testing --- Thanks, Eike Hein ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Web Shortcuts KCM
On Tuesday 15 July 2014 08:15:46 Kevin Ottens wrote: Honestly to me it looks like a wrong move. A better move would be to have the framework (e.g. KIO) read the settings from the platform for its settings (e.g. proxies). OK about proxies, but web shortcuts is a KIO-specific thing, you won't find any platform settings for these. I see nothing workspace-related or platform-related in web shortcuts. You type gg:foo in a KDE app, you get a google search - this requires a GUI for configuration, and I can't see why that configuration should be only available in a plasma workspace or on Unix. Nor do we want to see each one of the 10 apps that support web shortcuts to have to come up with their own GUI for configuring this -- 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: KConfig build fails
On Tuesday 15 July 2014 00:51:20 David Gil Oliva wrote: Hi! 2014-07-15 0:19 GMT+02:00 Frank Reininghaus frank7...@googlemail.com: Hi, 2014-07-14 23:21 GMT+02:00 David Gil Oliva: Hi! KConfig build fails with this messages, all of them related to QBasicAtomicInt. Are they KF5 bugs? probably this is the same issue that has been discussed in https://git.reviewboard.kde.org/r/119257/ ? According to David F., this might depend on the Qt version that you are using. Thank you so much. I'll try with Qt 5.3. No, please keep Qt 5.2 and add the missing load() and store() calls. We advertise Qt 5.2 compatibility, let's actually do it... -- 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: Web Shortcuts KCM
Hello, On Tuesday 15 July 2014 10:01:50 David Faure wrote: On Tuesday 15 July 2014 08:15:46 Kevin Ottens wrote: Honestly to me it looks like a wrong move. A better move would be to have the framework (e.g. KIO) read the settings from the platform for its settings (e.g. proxies). OK about proxies, but web shortcuts is a KIO-specific thing, you won't find any platform settings for these. I see nothing workspace-related or platform-related in web shortcuts. You type gg:foo in a KDE app, Which applications? I experience that mostly in krunner and the browsers. As a user I don't think I get to type those anywhere else. you get a google search - this requires a GUI for configuration, and I can't see why that configuration should be only available in a plasma workspace or on Unix. I have to admit I would have no problem having this kind of extra only controllable in our own workspace. I mean, it makes only a difference for end users, and I've no problem pointing them to our workspace if they want to get it in full. Nor do we want to see each one of the 10 apps that support web shortcuts to have to come up with their own GUI for configuring this At the same time they don't need a full fledged KCM either. A pre-made piece of GUI they can hook up is a much leaner answer to the problem which won't mandate a lot of dependencies. That could just be a widget in kio/widgets for instance. Then it would be usable by our browser and the workspace KCM. As I mentioned in the thread, some of those settings we're struggling with should be: - either application settings for with a widget can be more than enough (e.g. web shortcuts), I'm not even sure we want a widget in every cases (if considered power user could be just in the files with no pre-made GUI); - or workspace settings for which the frameworks should read them from the platform instead of by-passing them (e.g. proxies). In any case that doesn't ask for a KCM in a framework IMO. Cheers. -- Kévin Ottens, http://ervin.ipsquad.net KDAB - proud supporter of KDE, http://www.kdab.com 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
Review Request 119291: Use an input type=search for the search box.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119291/ --- Review request for KDE Frameworks, Alex Merry and Aurélien Gâteau. Repository: kapidox Description --- Use an `input` with a proper type and a placeholder: this allows the platform to style it differently if necessary, and the use of the placeholder attribute lets us to get rid of the custom script to clean up its value. Diffs - src/kapidox/data/templates/base.html 10f8aa6dae14e0ad6e649bdb28fcba81b7d39341 Diff: https://git.reviewboard.kde.org/r/119291/diff/ Testing --- Thanks, Raphael Kubo da Costa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119275: Fix: Same variable in camelCase and REQUIRED_HEADERS doesn't export all headers
On July 14, 2014, 10:57 p.m., Aleix Pol Gonzalez wrote: Is this because of the usage of list(APPEND)? Maybe using set(.. PARENT_SCOPE) for appending would do the trick as well? Andreas Xavier wrote: Can you confirm that you are seeing the same problem? This is my first time trying to compile KF5 and the problem is most likely to be with my setup. If I know that everyone is experiencing the problem, then I will try to fix it in ecm using set( .. PARENT_SCOPE). I remember running into something similar while building using kdesrc-build. I just suspected that it's something related to my setup since it went away once I manually ran 'make install' from the build directory created by kdesrc-build. While building on Windows using emerge I didn't see this though. - Cristian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/#review62358 --- On July 14, 2014, 6 p.m., Andreas Xavier wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/ --- (Updated July 14, 2014, 6 p.m.) Review request for KDE Frameworks, Alex Merry and Michael Pyne. Repository: kcoreaddons Description --- Using the same variable name for var1 and var2 in the new ecm_generate_headers() syntax, when it is called more than once only exports the headers from the first invokation of ecm_generate_headers(), where var1 and var2 are defined as follows. ecm_generate_headers(var1 ... REQUIRED_HEADERS var2 ) It doesn't show up in existing builds because cmake doesn't delete old header files. Steps to Replicate the Problem: 1. Delete the existing header files for KCoreAddons and the existing build files. rm -r $KF5/KcoreAddons rm -r your kcoreaddons/build directory 2. Re-build kcoreaddons from a new build dir cmake -DCMAKE_INSTALL_PREFIX=$KF5 .. 3. Check in $KF5/KcoreAddons and there should only be these headers: KAboutData kaboutdata.h kcoreaddons_export.h Solution: This patch solves the problem by changing the name of var2 to KCoreAddons_HEADERS_lowercase and exporting both KCoreAddons_HEADERS and KCoreAddons_HEADERS_lowercase. Extended Solution: If this patch is approved, then I will 1. Submit patches to the other frameworks using ecm_generate_headers() in this fashion. 2. submit a patch to extra-cmake-modules to warn when var1 and var2 have the same name. Diffs - src/lib/CMakeLists.txt 26eb5a1 Diff: https://git.reviewboard.kde.org/r/119275/diff/ Testing --- Compiled kcoreaddons, then checked that all headers generated and exported. Ran unittests. Thanks, Andreas Xavier ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Web Shortcuts KCM
On 07/15/2014 10:24 AM, Kevin Ottens wrote: Which applications? I experience that mostly in krunner and the browsers. As a user I don't think I get to type those anywhere else. It's not just typing. Konversation, Okular and Konsole let you select text, right-click it and search it using one of your pre- ferred Web Shortcuts. You can also see the KCM entry point there: http://wstaw.org/m/2014/07/15/webshortcuts.png And yes, we do care about having this work outside the KDE work- space, if only because it'd be annoying to keep track of users cannot configure this anyway, find a way to figure out if we're in KDE or not and hide this at runtime if not. It makes app code more complex. Cheers, Eike ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Web Shortcuts KCM
On Tuesday 15 July 2014 14:59:43 Eike Hein wrote: On 07/15/2014 10:24 AM, Kevin Ottens wrote: Which applications? I experience that mostly in krunner and the browsers. As a user I don't think I get to type those anywhere else. It's not just typing. Konversation, Okular and Konsole let you select text, right-click it and search it using one of your pre- ferred Web Shortcuts. You can also see the KCM entry point there: http://wstaw.org/m/2014/07/15/webshortcuts.png Ah I see. Never ever used that for some reason. :-) And yes, we do care about having this work outside the KDE work- space, if only because it'd be annoying to keep track of users cannot configure this anyway, find a way to figure out if we're in KDE or not and hide this at runtime if not. It makes app code more complex. Well, for a single entry menu, really? :-) Anyway, I think the rest of my previous email still stands (ie at most a widget would be enough for the app related settings, we should talk to the underlying platform for the other ones). Regards. -- Kévin Ottens, http://ervin.ipsquad.net KDAB - proud supporter of KDE, http://www.kdab.com 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
Review Request 119300: extra-cmake-modules: Fix using the same variable for camelCase and REQUIRED_HEADERS causes problems.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119300/ --- Review request for KDE Frameworks, Alex Merry and Michael Pyne. Repository: extra-cmake-modules Description --- First, this is my first attempt at building frameworks, so the problem is most likely on my end. However, following the instructions at https://community.kde.org/Frameworks/Building/Details I found this problem. Using the same variable name for camelCase var1 and REQUIRED_HEADERS var2 in the new ecm_generate_headers() syntax, when it is called more than once only exports the headers from the first invokation of ecm_generate_headers(), where var1 and var2 are defined as follows. ecm_generate_headers(var1 ... REQUIRED_HEADERS var2 ) It doesn't show up in existing builds because cmake doesn't delete old header files, so all accumulated headers remain in the build. There are some projects in kde-frameworks that use the same variable name for var1 and var2, for example kcoreaddons. Steps to Replicate Using KCoreAddons as an example: 1. Delete the existing header files for KCoreAddons and the existing build files. rm -r $KF5/KcoreAddons rm -r your kcoreaddons/build directory 2. Re-build kcoreaddons from a new build dir cmake -DCMAKE_INSTALL_PREFIX=$KF5 .. 3. Check in $KF5/KcoreAddons and there should only be these headers: KAboutData kaboutdata.h kcoreaddons_export.h Problem: If the same variable name is used for both var1 and var2 they overwrite each other at the PARENT level and the result is that only one invocation of ecm_generate_headers sticks. Solution: I assume that the CMakeList writer is intentionally using the same variable name for var1 and var2 and wants to accumulate both the camelCase and the lowercase.h names on the same variable. If the variable names are the same if accumulates both the camelCase list and the lowercase.h list on the single variable. If my assumption is incorrect and var1 and var2 must be different to separate the camelCase and lowercase.h lists, then a new patch should be submitted to generate either a warning or an error message when var1 and var2 are the same name. This review is linked with https://git.reviewboard.kde.org/r/119275/. If this review is accepted then https://git.reviewboard.kde.org/r/119275/ is unecessary. Diffs - modules/ECMGenerateHeaders.cmake bac5086 Diff: https://git.reviewboard.kde.org/r/119300/diff/ Testing --- make test Compiled kcoreaddons, checked that all the headers were exported. Then compiled kauth and checked that it found the exported headers. Thanks, Andreas Xavier ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119242: Fix QExplicitlySharedDataPointer usage
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119242/#review62402 --- Ship it! Ship It! - Milian Wolff On July 12, 2014, 9:43 a.m., Kevin Funk wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119242/ --- (Updated July 12, 2014, 9:43 a.m.) Review request for KDE Frameworks. Repository: kio Description --- Fix QExplicitlySharedDataPointer usage One of the constructors of QExplicitlySharedDataPointer got disabled in Qt 5.4 due to being to permissive. Also see: http://kfunk.org/2014/07/10/wrap-up-issues-with-qexplicitlyshareddatapointer-ksharedptr-successor/ Diffs - src/widgets/kopenwithdialog.cpp 8cb659fde2028892de82bad64e0ea3ff285b5e3a Diff: https://git.reviewboard.kde.org/r/119242/diff/ Testing --- Thanks, Kevin Funk ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119241: Fix QExplicitlySharedDataPointer usage
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119241/#review62401 --- Ship it! Ship It! - Milian Wolff On July 12, 2014, 9:41 a.m., Kevin Funk wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119241/ --- (Updated July 12, 2014, 9:41 a.m.) Review request for KDE Frameworks. Repository: kservice Description --- Fix QExplicitlySharedDataPointer usage One of the constructors of QExplicitlySharedDataPointer got disabled in Qt 5.4 due to being to permissive. Also see: http://kfunk.org/2014/07/10/wrap-up-issues-with-qexplicitlyshareddatapointer-ksharedptr-successor/ Diffs - src/kbuildsycoca/kbuildservicefactory.cpp bc36a22ced38892a92656d37a5d63e768dec6d40 src/kbuildsycoca/kbuildservicegroupfactory.cpp ea0d6ed094347c4bb3f0ce2bfd9a26445a25c545 src/kbuildsycoca/kbuildservicetypefactory.cpp 8f577a4267e8818e7e2cf5109068659439ca3221 src/kbuildsycoca/kbuildsycoca.cpp ed2929f4e60a1841807ce8490c8057d5a7b55827 src/services/kmimetypefactory.cpp a52d07d1dd9040c0a79f36f1665822e714d9b369 src/services/kservicefactory.cpp bb1e0f58396a358c3168c74a89be04315a295fcd src/services/kservicegroup.cpp 0bda1340ae0356c3d5aca9a9f0b3dd5e905638d8 src/services/kservicetypefactory.cpp 7599c4c73220b2aca366f41ac5cd7d05abfa8afc src/sycoca/ksycocadict.cpp a584f933bff10f44bc257ab996aaee3ad38cc79c tests/ksycocatest.cpp d51d80a691427fa4295dd06802de2fb87112f0ff Diff: https://git.reviewboard.kde.org/r/119241/diff/ Testing --- Thanks, Kevin Funk ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Web Shortcuts KCM
On 07/15/2014 03:16 PM, Kevin Ottens wrote: Well, for a single entry menu, really? :-) Yeah, I do care about every single menu entry not being broken, no matter where users use my app :). Of course admittedly the situ- ation in KDE 4 wasn't good either since it actually meant a runtime dep on kde-baseapps. I'm OK with spawning our own dialog and using a widget class, but I don't have time to work on that personally (I'm busy with Plasma and Konversation). This is a roadblock to keeping feature continuity in KF5 releases, so we do need to ship the KCM for now. Let me know where you guys want it put in the end. Regards. Cheers, Eike ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KConfig build fails
On Tuesday 15 of July 2014 12:09:43 David Gil Oliva wrote: El 15/07/2014 10:02, David Faure fa...@kde.org escribió: On Tuesday 15 July 2014 00:51:20 David Gil Oliva wrote: Hi! 2014-07-15 0:19 GMT+02:00 Frank Reininghaus frank7...@googlemail.com: Hi, 2014-07-14 23:21 GMT+02:00 David Gil Oliva: Hi! KConfig build fails with this messages, all of them related to QBasicAtomicInt. Are they KF5 bugs? probably this is the same issue that has been discussed in https://git.reviewboard.kde.org/r/119257/ ? According to David F., this might depend on the Qt version that you are using. Thank you so much. I'll try with Qt 5.3. No, please keep Qt 5.2 and add the missing load() and store() calls. The RR was discarded although you said it should go in. Yesterday night I asked the author why. I think I'll apply his patch locally and have it built. Let's hope I'll have a completely built KF5 someday :-D Hi, the same change is needed in KIO framework, too (/src/core/connectionbackend.cpp:141) Dan We advertise Qt 5.2 compatibility, let's actually do it... OK Cheers David -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 -- Daniel Vrátil | dvra...@redhat.com | dvratil on #kde-devel, #kontact, #akonadi KDE Desktop Team Associate Software Engineer, Red Hat GPG Key: 0xC59D614F6F4AE348 Fingerprint: 4EC1 86E3 C54E 0B39 5FDD B5FB C59D 614F 6F4A E348 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: KConfig build fails
Hi! 2014-07-15 17:16 GMT+02:00 Daniel Vrátil dvra...@redhat.com: On Tuesday 15 of July 2014 12:09:43 David Gil Oliva wrote: El 15/07/2014 10:02, David Faure fa...@kde.org escribió: On Tuesday 15 July 2014 00:51:20 David Gil Oliva wrote: Hi! 2014-07-15 0:19 GMT+02:00 Frank Reininghaus frank7...@googlemail.com: Hi, 2014-07-14 23:21 GMT+02:00 David Gil Oliva: Hi! KConfig build fails with this messages, all of them related to QBasicAtomicInt. Are they KF5 bugs? probably this is the same issue that has been discussed in https://git.reviewboard.kde.org/r/119257/ ? According to David F., this might depend on the Qt version that you are using. Thank you so much. I'll try with Qt 5.3. No, please keep Qt 5.2 and add the missing load() and store() calls. The RR was discarded although you said it should go in. Yesterday night I asked the author why. I think I'll apply his patch locally and have it built. Let's hope I'll have a completely built KF5 someday :-D Hi, the same change is needed in KIO framework, too (/src/core/connectionbackend.cpp:141) Dan If the change is needed, then let's submit it if someone gives a ship it to the RR. Or anyone knows why it was discarded? I am with my kid, so I could submit it not sooner than this evening. Cheers, David Gil We advertise Qt 5.2 compatibility, let's actually do it... OK Cheers David -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 -- Daniel Vrátil | dvra...@redhat.com | dvratil on #kde-devel, #kontact, #akonadi KDE Desktop Team Associate Software Engineer, Red Hat GPG Key: 0xC59D614F6F4AE348 Fingerprint: 4EC1 86E3 C54E 0B39 5FDD B5FB C59D 614F 6F4A E348 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119291: Use an input type=search for the search box.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119291/#review62407 --- Ship it! Ship It! - Alex Merry On July 15, 2014, 8:29 a.m., Raphael Kubo da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119291/ --- (Updated July 15, 2014, 8:29 a.m.) Review request for KDE Frameworks, Alex Merry and Aurélien Gâteau. Repository: kapidox Description --- Use an `input` with a proper type and a placeholder: this allows the platform to style it differently if necessary, and the use of the placeholder attribute lets us to get rid of the custom script to clean up its value. Diffs - src/kapidox/data/templates/base.html 10f8aa6dae14e0ad6e649bdb28fcba81b7d39341 Diff: https://git.reviewboard.kde.org/r/119291/diff/ Testing --- Thanks, Raphael Kubo da Costa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119291: Use an input type=search for the search box.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119291/ --- (Updated July 15, 2014, 6:20 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, Alex Merry and Aurélien Gâteau. Repository: kapidox Description --- Use an `input` with a proper type and a placeholder: this allows the platform to style it differently if necessary, and the use of the placeholder attribute lets us to get rid of the custom script to clean up its value. Diffs - src/kapidox/data/templates/base.html 10f8aa6dae14e0ad6e649bdb28fcba81b7d39341 Diff: https://git.reviewboard.kde.org/r/119291/diff/ Testing --- Thanks, Raphael Kubo da Costa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119289: Remove api_searchbox.html.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119289/ --- (Updated July 15, 2014, 9:21 p.m.) Review request for KDE Frameworks, Alex Merry and Aurélien Gâteau. Repository: kapidox Description --- It's been part of base.html itself since 07b35f28a334584114be89a5f293cd39ff003cd6. Diffs - src/kapidox/data/api_searchbox.html 9fd1ade894680f7f41ca498d0bc4a8dd684c0e98 Diff: https://git.reviewboard.kde.org/r/119289/diff/ Testing --- Thanks, Raphael Kubo da Costa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119275: Fix: Same variable in camelCase and REQUIRED_HEADERS doesn't export all headers
On July 14, 2014, 10:57 p.m., Aleix Pol Gonzalez wrote: Is this because of the usage of list(APPEND)? Maybe using set(.. PARENT_SCOPE) for appending would do the trick as well? Andreas Xavier wrote: Can you confirm that you are seeing the same problem? This is my first time trying to compile KF5 and the problem is most likely to be with my setup. If I know that everyone is experiencing the problem, then I will try to fix it in ecm using set( .. PARENT_SCOPE). Cristian Oneț wrote: I remember running into something similar while building using kdesrc-build. I just suspected that it's something related to my setup since it went away once I manually ran 'make install' from the build directory created by kdesrc-build. While building on Windows using emerge I didn't see this though. I can't reproduce this (and, additionally, the [CI system](http://build.kde.org) does a completely clean build and install every time). Can you post the output of `cmake --version`, please? - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/#review62358 --- On July 14, 2014, 6 p.m., Andreas Xavier wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/ --- (Updated July 14, 2014, 6 p.m.) Review request for KDE Frameworks, Alex Merry and Michael Pyne. Repository: kcoreaddons Description --- Using the same variable name for var1 and var2 in the new ecm_generate_headers() syntax, when it is called more than once only exports the headers from the first invokation of ecm_generate_headers(), where var1 and var2 are defined as follows. ecm_generate_headers(var1 ... REQUIRED_HEADERS var2 ) It doesn't show up in existing builds because cmake doesn't delete old header files. Steps to Replicate the Problem: 1. Delete the existing header files for KCoreAddons and the existing build files. rm -r $KF5/KcoreAddons rm -r your kcoreaddons/build directory 2. Re-build kcoreaddons from a new build dir cmake -DCMAKE_INSTALL_PREFIX=$KF5 .. 3. Check in $KF5/KcoreAddons and there should only be these headers: KAboutData kaboutdata.h kcoreaddons_export.h Solution: This patch solves the problem by changing the name of var2 to KCoreAddons_HEADERS_lowercase and exporting both KCoreAddons_HEADERS and KCoreAddons_HEADERS_lowercase. Extended Solution: If this patch is approved, then I will 1. Submit patches to the other frameworks using ecm_generate_headers() in this fashion. 2. submit a patch to extra-cmake-modules to warn when var1 and var2 have the same name. Diffs - src/lib/CMakeLists.txt 26eb5a1 Diff: https://git.reviewboard.kde.org/r/119275/diff/ Testing --- Compiled kcoreaddons, then checked that all headers generated and exported. Ran unittests. Thanks, Andreas Xavier ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119300: extra-cmake-modules: Fix using the same variable for camelCase and REQUIRED_HEADERS causes problems.
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119300/#review62408
---
I don't think this should be necessary. Let's keep discussion on
https://git.reviewboard.kde.org/r/119275/ for now, though.
modules/ECMGenerateHeaders.cmake
https://git.reviewboard.kde.org/r/119300/#comment43339
This test doesn't do what you think it does. Rather than comparing
${camelcase_headers_var} with ${EGH_REQUIRED_HEADERS}, it will compare
${${camelcase_headers_var}} with ${${EGH_REQUIRED_HEADERS}}. Unintuitive, I
know.
- Alex Merry
On July 15, 2014, 1:38 p.m., Andreas Xavier wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119300/
---
(Updated July 15, 2014, 1:38 p.m.)
Review request for KDE Frameworks, Alex Merry and Michael Pyne.
Repository: extra-cmake-modules
Description
---
First, this is my first attempt at building frameworks, so the problem is
most likely on my end. However, following the instructions at
https://community.kde.org/Frameworks/Building/Details I found this problem.
Using the same variable name for camelCase var1 and REQUIRED_HEADERS var2 in
the new ecm_generate_headers() syntax, when it is called more than once only
exports the headers from the first invokation of ecm_generate_headers(),
where var1 and var2 are defined as follows.
ecm_generate_headers(var1
...
REQUIRED_HEADERS var2
)
It doesn't show up in existing builds because cmake doesn't delete old header
files, so all accumulated headers remain in the build.
There are some projects in kde-frameworks that use the same variable name for
var1 and var2, for example kcoreaddons.
Steps to Replicate Using KCoreAddons as an example:
1. Delete the existing header files for KCoreAddons and the existing build
files.
rm -r $KF5/KcoreAddons
rm -r your kcoreaddons/build directory
2. Re-build kcoreaddons from a new build dir
cmake -DCMAKE_INSTALL_PREFIX=$KF5 ..
3. Check in $KF5/KcoreAddons and there should only be these headers:
KAboutData kaboutdata.h kcoreaddons_export.h
Problem:
If the same variable name is used for both var1 and var2 they overwrite each
other at the PARENT level and the result is that only one invocation of
ecm_generate_headers sticks.
Solution:
I assume that the CMakeList writer is intentionally using the same variable
name for var1 and var2 and wants to accumulate both the camelCase and the
lowercase.h names on the same variable. If the variable names are the same
if accumulates both the camelCase list and the lowercase.h list on the single
variable.
If my assumption is incorrect and var1 and var2 must be different to separate
the camelCase and lowercase.h lists, then a new patch should be submitted to
generate either a warning or an error message when var1 and var2 are the same
name.
This review is linked with https://git.reviewboard.kde.org/r/119275/. If
this review is accepted then https://git.reviewboard.kde.org/r/119275/ is
unecessary.
Diffs
-
modules/ECMGenerateHeaders.cmake bac5086
Diff: https://git.reviewboard.kde.org/r/119300/diff/
Testing
---
make test
Compiled kcoreaddons, checked that all the headers were exported. Then
compiled kauth and checked that it found the exported headers.
Thanks,
Andreas Xavier
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119289: Remove api_searchbox.html.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119289/#review62414 --- Ship it! Huh, guess I missed that :-) - Alex Merry On July 15, 2014, 6:21 p.m., Raphael Kubo da Costa wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119289/ --- (Updated July 15, 2014, 6:21 p.m.) Review request for KDE Frameworks, Alex Merry and Aurélien Gâteau. Repository: kapidox Description --- It's been part of base.html itself since 07b35f28a334584114be89a5f293cd39ff003cd6. Diffs - src/kapidox/data/api_searchbox.html 9fd1ade894680f7f41ca498d0bc4a8dd684c0e98 Diff: https://git.reviewboard.kde.org/r/119289/diff/ Testing --- Thanks, Raphael Kubo da Costa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119289: Remove api_searchbox.html.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119289/ --- (Updated July 15, 2014, 7:20 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, Alex Merry and Aurélien Gâteau. Repository: kapidox Description --- It's been part of base.html itself since 07b35f28a334584114be89a5f293cd39ff003cd6. Diffs - src/kapidox/data/api_searchbox.html 9fd1ade894680f7f41ca498d0bc4a8dd684c0e98 Diff: https://git.reviewboard.kde.org/r/119289/diff/ Testing --- Thanks, Raphael Kubo da Costa ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119257: kconfig: Fix QBasicAtomicInt being treated as int.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119257/#review62436 --- This patch fixes the build. Why did you discard it? - David Gil Oliva On jul. 13, 2014, 4:11 p.m., Andreas Xavier wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119257/ --- (Updated jul. 13, 2014, 4:11 p.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- While building frameworks following https://community.kde.org/Frameworks/Building/Details, I was unable to compile kconfig because QBasicAtomicInt is being treated as an int. This patch fixes that problem. Diffs - src/core/kconfig.cpp 14a5d39 Diff: https://git.reviewboard.kde.org/r/119257/diff/ Testing --- Compiled successfully. Thanks, Andreas Xavier ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119275: Fix: Same variable in camelCase and REQUIRED_HEADERS doesn't export all headers
On July 14, 2014, 10:57 p.m., Aleix Pol Gonzalez wrote: Is this because of the usage of list(APPEND)? Maybe using set(.. PARENT_SCOPE) for appending would do the trick as well? Andreas Xavier wrote: Can you confirm that you are seeing the same problem? This is my first time trying to compile KF5 and the problem is most likely to be with my setup. If I know that everyone is experiencing the problem, then I will try to fix it in ecm using set( .. PARENT_SCOPE). Cristian Oneț wrote: I remember running into something similar while building using kdesrc-build. I just suspected that it's something related to my setup since it went away once I manually ran 'make install' from the build directory created by kdesrc-build. While building on Windows using emerge I didn't see this though. Alex Merry wrote: I can't reproduce this (and, additionally, the [CI system](http://build.kde.org) does a completely clean build and install every time). Can you post the output of `cmake --version`, please? Entirely my mistake. All of frameworks compiled with no changes. Thank you for taking the time to look at this. I am now running with : cmake version 3.0.20140712-g2eadd1 One nameless Konsole tab over I was running with : cmake version 2.8.12.2 It might be worth noting that with the changes in this patch cmake 2.8.12.2 was able to compile frameworks all the way to KHTML with no complaints. Clearly, I am a cmake novice, but if this patch doesn't break the build with 3.0 and it makes it possible to build it with 2.8.12.2 it might make the overall build more robust. Once again thanks for your time. - Andreas --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/#review62358 --- On July 14, 2014, 6 p.m., Andreas Xavier wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/ --- (Updated July 14, 2014, 6 p.m.) Review request for KDE Frameworks, Alex Merry and Michael Pyne. Repository: kcoreaddons Description --- Using the same variable name for var1 and var2 in the new ecm_generate_headers() syntax, when it is called more than once only exports the headers from the first invokation of ecm_generate_headers(), where var1 and var2 are defined as follows. ecm_generate_headers(var1 ... REQUIRED_HEADERS var2 ) It doesn't show up in existing builds because cmake doesn't delete old header files. Steps to Replicate the Problem: 1. Delete the existing header files for KCoreAddons and the existing build files. rm -r $KF5/KcoreAddons rm -r your kcoreaddons/build directory 2. Re-build kcoreaddons from a new build dir cmake -DCMAKE_INSTALL_PREFIX=$KF5 .. 3. Check in $KF5/KcoreAddons and there should only be these headers: KAboutData kaboutdata.h kcoreaddons_export.h Solution: This patch solves the problem by changing the name of var2 to KCoreAddons_HEADERS_lowercase and exporting both KCoreAddons_HEADERS and KCoreAddons_HEADERS_lowercase. Extended Solution: If this patch is approved, then I will 1. Submit patches to the other frameworks using ecm_generate_headers() in this fashion. 2. submit a patch to extra-cmake-modules to warn when var1 and var2 have the same name. Diffs - src/lib/CMakeLists.txt 26eb5a1 Diff: https://git.reviewboard.kde.org/r/119275/diff/ Testing --- Compiled kcoreaddons, then checked that all headers generated and exported. Ran unittests. Thanks, Andreas Xavier ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119300: extra-cmake-modules: Fix using the same variable for camelCase and REQUIRED_HEADERS causes problems.
On July 15, 2014, 6:27 p.m., Alex Merry wrote:
modules/ECMGenerateHeaders.cmake, line 150
https://git.reviewboard.kde.org/r/119300/diff/1/?file=290331#file290331line150
This test doesn't do what you think it does. Rather than comparing
${camelcase_headers_var} with ${EGH_REQUIRED_HEADERS}, it will compare
${${camelcase_headers_var}} with ${${EGH_REQUIRED_HEADERS}}.
Unintuitive, I know.
Entirely my mistake. All of frameworks compiled with no changes.
Thank you for taking the time to look at this.
I am now running with :
cmake version 3.0.20140712-g2eadd1
One nameless Konsole tab over I was running with :
cmake version 2.8.12.2
It might be worth noting that with the changes in this patch cmake 2.8.12.2
was able to compile frameworks all the way to KHTML with no complaints.
Clearly, I am a cmake novice, but if this patch doesn't break the build with
3.0 and it makes it possible to build it with 2.8.12.2 it might make the
overall build more robust.
Once again thanks for your time.
- Andreas
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119300/#review62408
---
On July 15, 2014, 1:38 p.m., Andreas Xavier wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119300/
---
(Updated July 15, 2014, 1:38 p.m.)
Review request for KDE Frameworks, Alex Merry and Michael Pyne.
Repository: extra-cmake-modules
Description
---
First, this is my first attempt at building frameworks, so the problem is
most likely on my end. However, following the instructions at
https://community.kde.org/Frameworks/Building/Details I found this problem.
Using the same variable name for camelCase var1 and REQUIRED_HEADERS var2 in
the new ecm_generate_headers() syntax, when it is called more than once only
exports the headers from the first invokation of ecm_generate_headers(),
where var1 and var2 are defined as follows.
ecm_generate_headers(var1
...
REQUIRED_HEADERS var2
)
It doesn't show up in existing builds because cmake doesn't delete old header
files, so all accumulated headers remain in the build.
There are some projects in kde-frameworks that use the same variable name for
var1 and var2, for example kcoreaddons.
Steps to Replicate Using KCoreAddons as an example:
1. Delete the existing header files for KCoreAddons and the existing build
files.
rm -r $KF5/KcoreAddons
rm -r your kcoreaddons/build directory
2. Re-build kcoreaddons from a new build dir
cmake -DCMAKE_INSTALL_PREFIX=$KF5 ..
3. Check in $KF5/KcoreAddons and there should only be these headers:
KAboutData kaboutdata.h kcoreaddons_export.h
Problem:
If the same variable name is used for both var1 and var2 they overwrite each
other at the PARENT level and the result is that only one invocation of
ecm_generate_headers sticks.
Solution:
I assume that the CMakeList writer is intentionally using the same variable
name for var1 and var2 and wants to accumulate both the camelCase and the
lowercase.h names on the same variable. If the variable names are the same
if accumulates both the camelCase list and the lowercase.h list on the single
variable.
If my assumption is incorrect and var1 and var2 must be different to separate
the camelCase and lowercase.h lists, then a new patch should be submitted to
generate either a warning or an error message when var1 and var2 are the same
name.
This review is linked with https://git.reviewboard.kde.org/r/119275/. If
this review is accepted then https://git.reviewboard.kde.org/r/119275/ is
unecessary.
Diffs
-
modules/ECMGenerateHeaders.cmake bac5086
Diff: https://git.reviewboard.kde.org/r/119300/diff/
Testing
---
make test
Compiled kcoreaddons, checked that all the headers were exported. Then
compiled kauth and checked that it found the exported headers.
Thanks,
Andreas Xavier
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119275: Fix: Same variable in camelCase and REQUIRED_HEADERS doesn't export all headers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/ --- (Updated July 15, 2014, 9:36 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks, Alex Merry and Michael Pyne. Repository: kcoreaddons Description --- Using the same variable name for var1 and var2 in the new ecm_generate_headers() syntax, when it is called more than once only exports the headers from the first invokation of ecm_generate_headers(), where var1 and var2 are defined as follows. ecm_generate_headers(var1 ... REQUIRED_HEADERS var2 ) It doesn't show up in existing builds because cmake doesn't delete old header files. Steps to Replicate the Problem: 1. Delete the existing header files for KCoreAddons and the existing build files. rm -r $KF5/KcoreAddons rm -r your kcoreaddons/build directory 2. Re-build kcoreaddons from a new build dir cmake -DCMAKE_INSTALL_PREFIX=$KF5 .. 3. Check in $KF5/KcoreAddons and there should only be these headers: KAboutData kaboutdata.h kcoreaddons_export.h Solution: This patch solves the problem by changing the name of var2 to KCoreAddons_HEADERS_lowercase and exporting both KCoreAddons_HEADERS and KCoreAddons_HEADERS_lowercase. Extended Solution: If this patch is approved, then I will 1. Submit patches to the other frameworks using ecm_generate_headers() in this fashion. 2. submit a patch to extra-cmake-modules to warn when var1 and var2 have the same name. Diffs - src/lib/CMakeLists.txt 26eb5a1 Diff: https://git.reviewboard.kde.org/r/119275/diff/ Testing --- Compiled kcoreaddons, then checked that all headers generated and exported. Ran unittests. Thanks, Andreas Xavier ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119241: Fix QExplicitlySharedDataPointer usage
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119241/ --- (Updated July 15, 2014, 10:21 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kservice Description --- Fix QExplicitlySharedDataPointer usage One of the constructors of QExplicitlySharedDataPointer got disabled in Qt 5.4 due to being to permissive. Also see: http://kfunk.org/2014/07/10/wrap-up-issues-with-qexplicitlyshareddatapointer-ksharedptr-successor/ Diffs - src/kbuildsycoca/kbuildservicefactory.cpp bc36a22ced38892a92656d37a5d63e768dec6d40 src/kbuildsycoca/kbuildservicegroupfactory.cpp ea0d6ed094347c4bb3f0ce2bfd9a26445a25c545 src/kbuildsycoca/kbuildservicetypefactory.cpp 8f577a4267e8818e7e2cf5109068659439ca3221 src/kbuildsycoca/kbuildsycoca.cpp ed2929f4e60a1841807ce8490c8057d5a7b55827 src/services/kmimetypefactory.cpp a52d07d1dd9040c0a79f36f1665822e714d9b369 src/services/kservicefactory.cpp bb1e0f58396a358c3168c74a89be04315a295fcd src/services/kservicegroup.cpp 0bda1340ae0356c3d5aca9a9f0b3dd5e905638d8 src/services/kservicetypefactory.cpp 7599c4c73220b2aca366f41ac5cd7d05abfa8afc src/sycoca/ksycocadict.cpp a584f933bff10f44bc257ab996aaee3ad38cc79c tests/ksycocatest.cpp d51d80a691427fa4295dd06802de2fb87112f0ff Diff: https://git.reviewboard.kde.org/r/119241/diff/ Testing --- Thanks, Kevin Funk ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119242: Fix QExplicitlySharedDataPointer usage
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119242/ --- (Updated July 15, 2014, 10:22 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Repository: kio Description --- Fix QExplicitlySharedDataPointer usage One of the constructors of QExplicitlySharedDataPointer got disabled in Qt 5.4 due to being to permissive. Also see: http://kfunk.org/2014/07/10/wrap-up-issues-with-qexplicitlyshareddatapointer-ksharedptr-successor/ Diffs - src/widgets/kopenwithdialog.cpp 8cb659fde2028892de82bad64e0ea3ff285b5e3a Diff: https://git.reviewboard.kde.org/r/119242/diff/ Testing --- Thanks, Kevin Funk ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Jenkins build is back to stable : kio_master_qt5 #299
See http://build.kde.org/job/kio_master_qt5/299/changes ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[kwindowsystem] src: Preventing a crash in the KWindowInfo::Private destructor on OSX
Git commit 10398255165afda1a0ecddd1671bbb599846bb35 by Marko Käning.
Committed on 15/07/2014 at 22:51.
Pushed by kaning into branch 'master'.
Preventing a crash in the KWindowInfo::Private destructor on OSX
René J.V. Bertin found that Apple's developer documentation for
CFRelease mentions that its argument may not be a NULL pointer.
BUG: 337154
CCMAIL: kde-frameworks-devel@kde.org
REVIEW: 119240
M +3-1src/kwindowinfo_mac.cpp
http://commits.kde.org/kwindowsystem/10398255165afda1a0ecddd1671bbb599846bb35
diff --git a/src/kwindowinfo_mac.cpp b/src/kwindowinfo_mac.cpp
index ec4e9bb..328d6a9 100644
--- a/src/kwindowinfo_mac.cpp
+++ b/src/kwindowinfo_mac.cpp
@@ -48,7 +48,9 @@ void KWindowInfo::Private::setProcessSerialNumber(const
ProcessSerialNumber psn
KWindowInfo::Private::~Private()
{
-CFRelease(m_axWin);
+if (m_axWin) {
+CFRelease(m_axWin);
+}
}
void KWindowInfo::Private::updateData()
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119275: Fix: Same variable in camelCase and REQUIRED_HEADERS doesn't export all headers
On July 14, 2014, 10:57 p.m., Aleix Pol Gonzalez wrote: Is this because of the usage of list(APPEND)? Maybe using set(.. PARENT_SCOPE) for appending would do the trick as well? Andreas Xavier wrote: Can you confirm that you are seeing the same problem? This is my first time trying to compile KF5 and the problem is most likely to be with my setup. If I know that everyone is experiencing the problem, then I will try to fix it in ecm using set( .. PARENT_SCOPE). Cristian Oneț wrote: I remember running into something similar while building using kdesrc-build. I just suspected that it's something related to my setup since it went away once I manually ran 'make install' from the build directory created by kdesrc-build. While building on Windows using emerge I didn't see this though. Alex Merry wrote: I can't reproduce this (and, additionally, the [CI system](http://build.kde.org) does a completely clean build and install every time). Can you post the output of `cmake --version`, please? Andreas Xavier wrote: Entirely my mistake. All of frameworks compiled with no changes. Thank you for taking the time to look at this. I am now running with : cmake version 3.0.20140712-g2eadd1 One nameless Konsole tab over I was running with : cmake version 2.8.12.2 It might be worth noting that with the changes in this patch cmake 2.8.12.2 was able to compile frameworks all the way to KHTML with no complaints. Clearly, I am a cmake novice, but if this patch doesn't break the build with 3.0 and it makes it possible to build it with 2.8.12.2 it might make the overall build more robust. Once again thanks for your time. I'll look into whether kdesrc-build is failing on the first-ever-build use case, there's a recently-opened bug about it (bug 337446) so it's very possible. - Michael --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/#review62358 --- On July 15, 2014, 9:36 p.m., Andreas Xavier wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/ --- (Updated July 15, 2014, 9:36 p.m.) Review request for KDE Frameworks, Alex Merry and Michael Pyne. Repository: kcoreaddons Description --- Using the same variable name for var1 and var2 in the new ecm_generate_headers() syntax, when it is called more than once only exports the headers from the first invokation of ecm_generate_headers(), where var1 and var2 are defined as follows. ecm_generate_headers(var1 ... REQUIRED_HEADERS var2 ) It doesn't show up in existing builds because cmake doesn't delete old header files. Steps to Replicate the Problem: 1. Delete the existing header files for KCoreAddons and the existing build files. rm -r $KF5/KcoreAddons rm -r your kcoreaddons/build directory 2. Re-build kcoreaddons from a new build dir cmake -DCMAKE_INSTALL_PREFIX=$KF5 .. 3. Check in $KF5/KcoreAddons and there should only be these headers: KAboutData kaboutdata.h kcoreaddons_export.h Solution: This patch solves the problem by changing the name of var2 to KCoreAddons_HEADERS_lowercase and exporting both KCoreAddons_HEADERS and KCoreAddons_HEADERS_lowercase. Extended Solution: If this patch is approved, then I will 1. Submit patches to the other frameworks using ecm_generate_headers() in this fashion. 2. submit a patch to extra-cmake-modules to warn when var1 and var2 have the same name. Diffs - src/lib/CMakeLists.txt 26eb5a1 Diff: https://git.reviewboard.kde.org/r/119275/diff/ Testing --- Compiled kcoreaddons, then checked that all headers generated and exported. Ran unittests. Thanks, Andreas Xavier ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 119275: Fix: Same variable in camelCase and REQUIRED_HEADERS doesn't export all headers
On Iulie 14, 2014, 10:57 p.m., Aleix Pol Gonzalez wrote: Is this because of the usage of list(APPEND)? Maybe using set(.. PARENT_SCOPE) for appending would do the trick as well? Andreas Xavier wrote: Can you confirm that you are seeing the same problem? This is my first time trying to compile KF5 and the problem is most likely to be with my setup. If I know that everyone is experiencing the problem, then I will try to fix it in ecm using set( .. PARENT_SCOPE). Cristian Oneț wrote: I remember running into something similar while building using kdesrc-build. I just suspected that it's something related to my setup since it went away once I manually ran 'make install' from the build directory created by kdesrc-build. While building on Windows using emerge I didn't see this though. Alex Merry wrote: I can't reproduce this (and, additionally, the [CI system](http://build.kde.org) does a completely clean build and install every time). Can you post the output of `cmake --version`, please? Andreas Xavier wrote: Entirely my mistake. All of frameworks compiled with no changes. Thank you for taking the time to look at this. I am now running with : cmake version 3.0.20140712-g2eadd1 One nameless Konsole tab over I was running with : cmake version 2.8.12.2 It might be worth noting that with the changes in this patch cmake 2.8.12.2 was able to compile frameworks all the way to KHTML with no complaints. Clearly, I am a cmake novice, but if this patch doesn't break the build with 3.0 and it makes it possible to build it with 2.8.12.2 it might make the overall build more robust. Once again thanks for your time. Michael Pyne wrote: I'll look into whether kdesrc-build is failing on the first-ever-build use case, there's a recently-opened bug about it (bug 337446) so it's very possible. Now this is starting to make sense to me, altough kdesrc-build using extragear/utils/kdesrc-build/kf5-qt5-build-include builds cmake-git maybe it was using up cmake from my system which is 2.8.12.2 and was causing this problem. - Cristian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/#review62358 --- On Iulie 15, 2014, 9:36 p.m., Andreas Xavier wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119275/ --- (Updated Iulie 15, 2014, 9:36 p.m.) Review request for KDE Frameworks, Alex Merry and Michael Pyne. Repository: kcoreaddons Description --- Using the same variable name for var1 and var2 in the new ecm_generate_headers() syntax, when it is called more than once only exports the headers from the first invokation of ecm_generate_headers(), where var1 and var2 are defined as follows. ecm_generate_headers(var1 ... REQUIRED_HEADERS var2 ) It doesn't show up in existing builds because cmake doesn't delete old header files. Steps to Replicate the Problem: 1. Delete the existing header files for KCoreAddons and the existing build files. rm -r $KF5/KcoreAddons rm -r your kcoreaddons/build directory 2. Re-build kcoreaddons from a new build dir cmake -DCMAKE_INSTALL_PREFIX=$KF5 .. 3. Check in $KF5/KcoreAddons and there should only be these headers: KAboutData kaboutdata.h kcoreaddons_export.h Solution: This patch solves the problem by changing the name of var2 to KCoreAddons_HEADERS_lowercase and exporting both KCoreAddons_HEADERS and KCoreAddons_HEADERS_lowercase. Extended Solution: If this patch is approved, then I will 1. Submit patches to the other frameworks using ecm_generate_headers() in this fashion. 2. submit a patch to extra-cmake-modules to warn when var1 and var2 have the same name. Diffs - src/lib/CMakeLists.txt 26eb5a1 Diff: https://git.reviewboard.kde.org/r/119275/diff/ Testing --- Compiled kcoreaddons, then checked that all headers generated and exported. Ran unittests. Thanks, Andreas Xavier ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
