Re: Review Request 112722: Juggle around Kiosk include names to avoid incorrect library linkage and for better layering
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112722/#review40048 --- Ship it! Only some docu is missing: @since 5.0 in the new namespace (or its methods, rather), and porting instructions in KDE5PORTING.html - David Faure On Sept. 15, 2013, 12:41 a.m., Eike Hein wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112722/ --- (Updated Sept. 15, 2013, 12:41 a.m.) Review request for KDE Frameworks and David Faure. Description --- In KF5, KAuthorized was split into two: KCoreAuthorized, in KConfig, and KAuthorized, in KIO. Despite the name, KCoreAuthorized offers functions in the KAuthorized namespace, and KAuthorized extends it with additional functions that require KIO APIs. This causes the following problems: * The majority of users of the framework want to use the functions offered in KCoreAuthorized, but when they include kcoreauthorized.h, they have to use the KAuthorized namespace to access them. This sort of non-mapping is bad for the brain :). * While kauthorized.h remaining available via KIO succeds in preserving SC, it does so by muddled-up lib dependencies: Stuff that doesn't actually need KIO might end up linking to it just to avoid changing sources, or just because someone greps for KAuthorized and finds the copy in KIO (this is what happened to me in the Task Manager and made me investigate). The proposed patch does the following to address this: * KCoreAuthorized in KConfig is renamed to KAuthorized. This fixes the include-to-namespace mapping problem and continues to preserve SC for the majority of users. * KAuthorized in KIO is renamed to KUrlAuthorized to hold the URL/KIO-specific functions in the eponymous namespace. This means the few users of those functions (e.g. KHTML) need to be ported to new API. The bottom line is that this avoids users linking to the wrong API for no reason and continues to preserve SC for the majority of users while unfortunately causing porting effort for some users. Most existing users will be unaffected and gain the ability to stop linking KIO if they don't need it. New users won't be confused over why something as canonical-sounding as KAuthorized is in KIO. The alternative would have been to keep the KCoreAuthorized and KAuthorized names and locations, and add forwarders with deprecation notices to KAuthorized for the functions offered in KCoreAuthorized. However, this would mean duplication and a lot of porting churn away from deprecated API, and in the meantime not address the linking issue. This change was discussed with and tentatively agreed upon (pre-code) by dfaure. Diffs - includes/CMakeLists.txt bcd3789 kfile/kdirselectdialog.cpp ac4a194 kfile/kfilewidget.cpp 4901fcd khtml/java/kjavaappletviewer.cpp b10d993 khtml/khtml_part.cpp b0e31ce khtml/misc/loader.cpp cf451be khtml/xml/dom_docimpl.cpp 2b6125d kio/kfile/kpropertiesdialog.cpp 359ffab kio/misc/ktelnetservice.cpp 1c0d517 kutils/kcmultidialog.cpp 5286866 staging/kbookmarks/src/kbookmarkmenu.cc 6c1569c staging/kcompletion/src/klineedit.cpp 76ef377 staging/kde4support/src/kdeui/kapplication.cpp 12092e9 staging/kio/src/core/CMakeLists.txt 4897d5f staging/kio/src/core/job.cpp 511d1cc staging/kio/src/core/kauthorized.h 473725d staging/kio/src/core/kauthorized.cpp b930715 staging/kio/src/core/kurlauthorized.h PRE-CREATION staging/kio/src/core/kurlauthorized.cpp PRE-CREATION staging/kio/src/core/mkdirjob.cpp 9ed00b0 staging/kio/src/widgets/accessmanagerreply_p.cpp ad9b4f7 staging/kio/src/widgets/kfileitemactions.cpp 99d3209 staging/kio/src/widgets/kopenwithdialog.cpp b46e1ee staging/kio/src/widgets/krun.cpp aba4d7a staging/kio/src/widgets/kurlcompletion.cpp 80a4023 staging/kservice/src/services/kservice.cpp dea8618 staging/xmlgui/src/kactioncollection.cpp 5085f53 staging/xmlgui/src/khelpmenu.cpp cfc52a0 staging/xmlgui/src/ktoolbar.cpp 6a59874 staging/xmlgui/src/ktoolbarhandler.cpp 162de5a staging/xmlgui/src/kxmlguibuilder.cpp 6eb8a51 staging/xmlgui/src/kxmlguiclient.cpp 809033f tier1/kconfig/src/core/CMakeLists.txt 51e21ff tier1/kconfig/src/core/kauthorized.h PRE-CREATION tier1/kconfig/src/core/kauthorized.cpp PRE-CREATION tier1/kconfig/src/core/kcoreauthorized.h 2a3d79e tier1/kconfig/src/core/kcoreauthorized.cpp 97cbc79 tier1/kconfig/src/core/kdesktopfile.cpp a0d5cb8 Diff: http://git.reviewboard.kde.org/r/112722/diff/ Testing --- Thanks, Eike Hein
Re: Review Request 112722: Juggle around Kiosk include names to avoid incorrect library linkage and for better layering
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112722/#review40085 --- This review has been submitted with commit 9319f78c6ad63a377aeb7ab713265d402ddce7a3 by Eike Hein to branch frameworks. - Commit Hook On Sept. 15, 2013, 12:41 a.m., Eike Hein wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112722/ --- (Updated Sept. 15, 2013, 12:41 a.m.) Review request for KDE Frameworks and David Faure. Description --- In KF5, KAuthorized was split into two: KCoreAuthorized, in KConfig, and KAuthorized, in KIO. Despite the name, KCoreAuthorized offers functions in the KAuthorized namespace, and KAuthorized extends it with additional functions that require KIO APIs. This causes the following problems: * The majority of users of the framework want to use the functions offered in KCoreAuthorized, but when they include kcoreauthorized.h, they have to use the KAuthorized namespace to access them. This sort of non-mapping is bad for the brain :). * While kauthorized.h remaining available via KIO succeds in preserving SC, it does so by muddled-up lib dependencies: Stuff that doesn't actually need KIO might end up linking to it just to avoid changing sources, or just because someone greps for KAuthorized and finds the copy in KIO (this is what happened to me in the Task Manager and made me investigate). The proposed patch does the following to address this: * KCoreAuthorized in KConfig is renamed to KAuthorized. This fixes the include-to-namespace mapping problem and continues to preserve SC for the majority of users. * KAuthorized in KIO is renamed to KUrlAuthorized to hold the URL/KIO-specific functions in the eponymous namespace. This means the few users of those functions (e.g. KHTML) need to be ported to new API. The bottom line is that this avoids users linking to the wrong API for no reason and continues to preserve SC for the majority of users while unfortunately causing porting effort for some users. Most existing users will be unaffected and gain the ability to stop linking KIO if they don't need it. New users won't be confused over why something as canonical-sounding as KAuthorized is in KIO. The alternative would have been to keep the KCoreAuthorized and KAuthorized names and locations, and add forwarders with deprecation notices to KAuthorized for the functions offered in KCoreAuthorized. However, this would mean duplication and a lot of porting churn away from deprecated API, and in the meantime not address the linking issue. This change was discussed with and tentatively agreed upon (pre-code) by dfaure. Diffs - includes/CMakeLists.txt bcd3789 kfile/kdirselectdialog.cpp ac4a194 kfile/kfilewidget.cpp 4901fcd khtml/java/kjavaappletviewer.cpp b10d993 khtml/khtml_part.cpp b0e31ce khtml/misc/loader.cpp cf451be khtml/xml/dom_docimpl.cpp 2b6125d kio/kfile/kpropertiesdialog.cpp 359ffab kio/misc/ktelnetservice.cpp 1c0d517 kutils/kcmultidialog.cpp 5286866 staging/kbookmarks/src/kbookmarkmenu.cc 6c1569c staging/kcompletion/src/klineedit.cpp 76ef377 staging/kde4support/src/kdeui/kapplication.cpp 12092e9 staging/kio/src/core/CMakeLists.txt 4897d5f staging/kio/src/core/job.cpp 511d1cc staging/kio/src/core/kauthorized.h 473725d staging/kio/src/core/kauthorized.cpp b930715 staging/kio/src/core/kurlauthorized.h PRE-CREATION staging/kio/src/core/kurlauthorized.cpp PRE-CREATION staging/kio/src/core/mkdirjob.cpp 9ed00b0 staging/kio/src/widgets/accessmanagerreply_p.cpp ad9b4f7 staging/kio/src/widgets/kfileitemactions.cpp 99d3209 staging/kio/src/widgets/kopenwithdialog.cpp b46e1ee staging/kio/src/widgets/krun.cpp aba4d7a staging/kio/src/widgets/kurlcompletion.cpp 80a4023 staging/kservice/src/services/kservice.cpp dea8618 staging/xmlgui/src/kactioncollection.cpp 5085f53 staging/xmlgui/src/khelpmenu.cpp cfc52a0 staging/xmlgui/src/ktoolbar.cpp 6a59874 staging/xmlgui/src/ktoolbarhandler.cpp 162de5a staging/xmlgui/src/kxmlguibuilder.cpp 6eb8a51 staging/xmlgui/src/kxmlguiclient.cpp 809033f tier1/kconfig/src/core/CMakeLists.txt 51e21ff tier1/kconfig/src/core/kauthorized.h PRE-CREATION tier1/kconfig/src/core/kauthorized.cpp PRE-CREATION tier1/kconfig/src/core/kcoreauthorized.h 2a3d79e tier1/kconfig/src/core/kcoreauthorized.cpp 97cbc79 tier1/kconfig/src/core/kdesktopfile.cpp a0d5cb8 Diff: http://git.reviewboard.kde.org/r/112722/diff/ Testing --- Thanks, Eike Hein ___
Re: Review Request 112722: Juggle around Kiosk include names to avoid incorrect library linkage and for better layering
On Sept. 15, 2013, 8:37 a.m., David Faure wrote: Only some docu is missing: @since 5.0 in the new namespace (or its methods, rather), and porting instructions in KDE5PORTING.html Thanks :). The pushed version includes the requested docs changes. - Eike --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112722/#review40048 --- On Sept. 15, 2013, 11:33 p.m., Eike Hein wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112722/ --- (Updated Sept. 15, 2013, 11:33 p.m.) Review request for KDE Frameworks and David Faure. Description --- In KF5, KAuthorized was split into two: KCoreAuthorized, in KConfig, and KAuthorized, in KIO. Despite the name, KCoreAuthorized offers functions in the KAuthorized namespace, and KAuthorized extends it with additional functions that require KIO APIs. This causes the following problems: * The majority of users of the framework want to use the functions offered in KCoreAuthorized, but when they include kcoreauthorized.h, they have to use the KAuthorized namespace to access them. This sort of non-mapping is bad for the brain :). * While kauthorized.h remaining available via KIO succeds in preserving SC, it does so by muddled-up lib dependencies: Stuff that doesn't actually need KIO might end up linking to it just to avoid changing sources, or just because someone greps for KAuthorized and finds the copy in KIO (this is what happened to me in the Task Manager and made me investigate). The proposed patch does the following to address this: * KCoreAuthorized in KConfig is renamed to KAuthorized. This fixes the include-to-namespace mapping problem and continues to preserve SC for the majority of users. * KAuthorized in KIO is renamed to KUrlAuthorized to hold the URL/KIO-specific functions in the eponymous namespace. This means the few users of those functions (e.g. KHTML) need to be ported to new API. The bottom line is that this avoids users linking to the wrong API for no reason and continues to preserve SC for the majority of users while unfortunately causing porting effort for some users. Most existing users will be unaffected and gain the ability to stop linking KIO if they don't need it. New users won't be confused over why something as canonical-sounding as KAuthorized is in KIO. The alternative would have been to keep the KCoreAuthorized and KAuthorized names and locations, and add forwarders with deprecation notices to KAuthorized for the functions offered in KCoreAuthorized. However, this would mean duplication and a lot of porting churn away from deprecated API, and in the meantime not address the linking issue. This change was discussed with and tentatively agreed upon (pre-code) by dfaure. Diffs - includes/CMakeLists.txt bcd3789 kfile/kdirselectdialog.cpp ac4a194 kfile/kfilewidget.cpp 4901fcd khtml/java/kjavaappletviewer.cpp b10d993 khtml/khtml_part.cpp b0e31ce khtml/misc/loader.cpp cf451be khtml/xml/dom_docimpl.cpp 2b6125d kio/kfile/kpropertiesdialog.cpp 359ffab kio/misc/ktelnetservice.cpp 1c0d517 kutils/kcmultidialog.cpp 5286866 staging/kbookmarks/src/kbookmarkmenu.cc 6c1569c staging/kcompletion/src/klineedit.cpp 76ef377 staging/kde4support/src/kdeui/kapplication.cpp 12092e9 staging/kio/src/core/CMakeLists.txt 4897d5f staging/kio/src/core/job.cpp 511d1cc staging/kio/src/core/kauthorized.h 473725d staging/kio/src/core/kauthorized.cpp b930715 staging/kio/src/core/kurlauthorized.h PRE-CREATION staging/kio/src/core/kurlauthorized.cpp PRE-CREATION staging/kio/src/core/mkdirjob.cpp 9ed00b0 staging/kio/src/widgets/accessmanagerreply_p.cpp ad9b4f7 staging/kio/src/widgets/kfileitemactions.cpp 99d3209 staging/kio/src/widgets/kopenwithdialog.cpp b46e1ee staging/kio/src/widgets/krun.cpp aba4d7a staging/kio/src/widgets/kurlcompletion.cpp 80a4023 staging/kservice/src/services/kservice.cpp dea8618 staging/xmlgui/src/kactioncollection.cpp 5085f53 staging/xmlgui/src/khelpmenu.cpp cfc52a0 staging/xmlgui/src/ktoolbar.cpp 6a59874 staging/xmlgui/src/ktoolbarhandler.cpp 162de5a staging/xmlgui/src/kxmlguibuilder.cpp 6eb8a51 staging/xmlgui/src/kxmlguiclient.cpp 809033f tier1/kconfig/src/core/CMakeLists.txt 51e21ff tier1/kconfig/src/core/kauthorized.h PRE-CREATION tier1/kconfig/src/core/kauthorized.cpp PRE-CREATION tier1/kconfig/src/core/kcoreauthorized.h 2a3d79e tier1/kconfig/src/core/kcoreauthorized.cpp 97cbc79 tier1/kconfig/src/core/kdesktopfile.cpp a0d5cb8 Diff:
Re: Review Request 112722: Juggle around Kiosk include names to avoid incorrect library linkage and for better layering
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112722/ --- (Updated Sept. 15, 2013, 11:33 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and David Faure. Description --- In KF5, KAuthorized was split into two: KCoreAuthorized, in KConfig, and KAuthorized, in KIO. Despite the name, KCoreAuthorized offers functions in the KAuthorized namespace, and KAuthorized extends it with additional functions that require KIO APIs. This causes the following problems: * The majority of users of the framework want to use the functions offered in KCoreAuthorized, but when they include kcoreauthorized.h, they have to use the KAuthorized namespace to access them. This sort of non-mapping is bad for the brain :). * While kauthorized.h remaining available via KIO succeds in preserving SC, it does so by muddled-up lib dependencies: Stuff that doesn't actually need KIO might end up linking to it just to avoid changing sources, or just because someone greps for KAuthorized and finds the copy in KIO (this is what happened to me in the Task Manager and made me investigate). The proposed patch does the following to address this: * KCoreAuthorized in KConfig is renamed to KAuthorized. This fixes the include-to-namespace mapping problem and continues to preserve SC for the majority of users. * KAuthorized in KIO is renamed to KUrlAuthorized to hold the URL/KIO-specific functions in the eponymous namespace. This means the few users of those functions (e.g. KHTML) need to be ported to new API. The bottom line is that this avoids users linking to the wrong API for no reason and continues to preserve SC for the majority of users while unfortunately causing porting effort for some users. Most existing users will be unaffected and gain the ability to stop linking KIO if they don't need it. New users won't be confused over why something as canonical-sounding as KAuthorized is in KIO. The alternative would have been to keep the KCoreAuthorized and KAuthorized names and locations, and add forwarders with deprecation notices to KAuthorized for the functions offered in KCoreAuthorized. However, this would mean duplication and a lot of porting churn away from deprecated API, and in the meantime not address the linking issue. This change was discussed with and tentatively agreed upon (pre-code) by dfaure. Diffs - includes/CMakeLists.txt bcd3789 kfile/kdirselectdialog.cpp ac4a194 kfile/kfilewidget.cpp 4901fcd khtml/java/kjavaappletviewer.cpp b10d993 khtml/khtml_part.cpp b0e31ce khtml/misc/loader.cpp cf451be khtml/xml/dom_docimpl.cpp 2b6125d kio/kfile/kpropertiesdialog.cpp 359ffab kio/misc/ktelnetservice.cpp 1c0d517 kutils/kcmultidialog.cpp 5286866 staging/kbookmarks/src/kbookmarkmenu.cc 6c1569c staging/kcompletion/src/klineedit.cpp 76ef377 staging/kde4support/src/kdeui/kapplication.cpp 12092e9 staging/kio/src/core/CMakeLists.txt 4897d5f staging/kio/src/core/job.cpp 511d1cc staging/kio/src/core/kauthorized.h 473725d staging/kio/src/core/kauthorized.cpp b930715 staging/kio/src/core/kurlauthorized.h PRE-CREATION staging/kio/src/core/kurlauthorized.cpp PRE-CREATION staging/kio/src/core/mkdirjob.cpp 9ed00b0 staging/kio/src/widgets/accessmanagerreply_p.cpp ad9b4f7 staging/kio/src/widgets/kfileitemactions.cpp 99d3209 staging/kio/src/widgets/kopenwithdialog.cpp b46e1ee staging/kio/src/widgets/krun.cpp aba4d7a staging/kio/src/widgets/kurlcompletion.cpp 80a4023 staging/kservice/src/services/kservice.cpp dea8618 staging/xmlgui/src/kactioncollection.cpp 5085f53 staging/xmlgui/src/khelpmenu.cpp cfc52a0 staging/xmlgui/src/ktoolbar.cpp 6a59874 staging/xmlgui/src/ktoolbarhandler.cpp 162de5a staging/xmlgui/src/kxmlguibuilder.cpp 6eb8a51 staging/xmlgui/src/kxmlguiclient.cpp 809033f tier1/kconfig/src/core/CMakeLists.txt 51e21ff tier1/kconfig/src/core/kauthorized.h PRE-CREATION tier1/kconfig/src/core/kauthorized.cpp PRE-CREATION tier1/kconfig/src/core/kcoreauthorized.h 2a3d79e tier1/kconfig/src/core/kcoreauthorized.cpp 97cbc79 tier1/kconfig/src/core/kdesktopfile.cpp a0d5cb8 Diff: http://git.reviewboard.kde.org/r/112722/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: Review Request 112722: Juggle around Kiosk include names to avoid incorrect library linkage and for better layering
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112722/#review40013 --- The description is wrong because it talks about KUrlAuthorized which doesn't exist. But let's make it exist, for a full namespace-to-include mapping. The internal exported function is definitely no reason against doing that. Just put the extern definitions in KIO inside a namespace KAuthorized. And everything else in KIO goes in the namespace KUrlAuthorized. - David Faure On Sept. 13, 2013, 11:38 p.m., Eike Hein wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112722/ --- (Updated Sept. 13, 2013, 11:38 p.m.) Review request for KDE Frameworks and David Faure. Description --- In KF5, KAuthorized was split into two: KCoreAuthorized, in KConfig, and KAuthorized, in KIO. Despite the name, KCoreAuthorized offers functions in the KAuthorized namespace, and KAuthorized extends it with additional functions that require KIO APIs. This causes the following problems: * The majority of users of the framework want to use the functions offered in KCoreAuthorized, but when they include kcoreauthorized.h, they have to use the KAuthorized namespace to access them. This sort of non-mapping is bad for the brain :). * While kauthorized.h remaining available via KIO succeds in preserving SC, it does so by muddled-up lib dependencies: Stuff that doesn't actually need KIO might end up linking to it just to avoid changing sources, or just because someone greps for KAuthorized and finds the copy in KIO (this is what happened to me in the Task Manager and made me investigate). The proposed patch does the following to address this: * KCoreAuthorized in KConfig is renamed to KAuthorized. This fixes the include-to-namespace mapping problem and continues to preserve SC for the majority of users. * KAuthorized in KIO is renamed to KUrlAuthorized to hold the URL/KIO-specific functions. This means the few users of those functions (e.g. KHTML) need to be ported to a new include name, but SC is retained via the same namespace. (I think it would have been nice to offer them in a KUrlAuthorized namespace to avoid the include-to-namespace mapping problem here too, but this is complicated by wanting to call non-public symbols expoted by K(Core)Authorized ... ideas welcome.) The bottom line is that this avoids users linking to the wrong API for no reason and continues to preserve SC for the majority of users while unfortunately causing porting effort for some users. Most existing users will be unaffected and gain the ability to stop linking KIO if they don't need it. New users won't be confused over why something as canonical-sounding as KAuthorized is in KIO. The alternative would have been to keep the KCoreAuthorized and KAuthorized names and locations, and add forwarders with deprecation notices to KAuthorized for the functions offered in KCoreAuthorized. However, this would mean duplication and a lot of porting churn away from deprecated API, and in the meantime not address the linking issue. This change was discussed with and tentatively agreed upon (pre-code) by dfaure. Diffs - includes/CMakeLists.txt bcd3789 kfile/kdirselectdialog.cpp ac4a194 kfile/kfilewidget.cpp 4901fcd khtml/java/kjavaappletviewer.cpp b10d993 khtml/khtml_part.cpp b0e31ce khtml/misc/loader.cpp cf451be khtml/xml/dom_docimpl.cpp 2b6125d kio/kfile/kpropertiesdialog.cpp 359ffab kio/misc/ktelnetservice.cpp 1c0d517 kutils/kcmultidialog.cpp 5286866 staging/kbookmarks/src/kbookmarkmenu.cc 6c1569c staging/kcompletion/src/klineedit.cpp 76ef377 staging/kde4support/src/kdeui/kapplication.cpp 12092e9 staging/kio/src/core/CMakeLists.txt 4897d5f staging/kio/src/core/job.cpp 511d1cc staging/kio/src/core/kauthorized.h 473725d staging/kio/src/core/kauthorized.cpp b930715 staging/kio/src/core/kurlauthorized.h PRE-CREATION staging/kio/src/core/kurlauthorized.cpp PRE-CREATION staging/kio/src/core/mkdirjob.cpp 9ed00b0 staging/kio/src/widgets/accessmanagerreply_p.cpp ad9b4f7 staging/kio/src/widgets/kfileitemactions.cpp 99d3209 staging/kio/src/widgets/kopenwithdialog.cpp b46e1ee staging/kio/src/widgets/krun.cpp aba4d7a staging/kio/src/widgets/kurlcompletion.cpp 80a4023 staging/kservice/src/services/kservice.cpp dea8618 staging/xmlgui/src/kactioncollection.cpp 5085f53 staging/xmlgui/src/khelpmenu.cpp cfc52a0 staging/xmlgui/src/ktoolbar.cpp 6a59874 staging/xmlgui/src/ktoolbarhandler.cpp 162de5a staging/xmlgui/src/kxmlguibuilder.cpp 6eb8a51
Re: Review Request 112722: Juggle around Kiosk include names to avoid incorrect library linkage and for better layering
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112722/ --- (Updated Sept. 15, 2013, 12:41 a.m.) Review request for KDE Frameworks and David Faure. Changes --- Add KUrlAuthorized namespace; update description. Description (updated) --- In KF5, KAuthorized was split into two: KCoreAuthorized, in KConfig, and KAuthorized, in KIO. Despite the name, KCoreAuthorized offers functions in the KAuthorized namespace, and KAuthorized extends it with additional functions that require KIO APIs. This causes the following problems: * The majority of users of the framework want to use the functions offered in KCoreAuthorized, but when they include kcoreauthorized.h, they have to use the KAuthorized namespace to access them. This sort of non-mapping is bad for the brain :). * While kauthorized.h remaining available via KIO succeds in preserving SC, it does so by muddled-up lib dependencies: Stuff that doesn't actually need KIO might end up linking to it just to avoid changing sources, or just because someone greps for KAuthorized and finds the copy in KIO (this is what happened to me in the Task Manager and made me investigate). The proposed patch does the following to address this: * KCoreAuthorized in KConfig is renamed to KAuthorized. This fixes the include-to-namespace mapping problem and continues to preserve SC for the majority of users. * KAuthorized in KIO is renamed to KUrlAuthorized to hold the URL/KIO-specific functions in the eponymous namespace. This means the few users of those functions (e.g. KHTML) need to be ported to new API. The bottom line is that this avoids users linking to the wrong API for no reason and continues to preserve SC for the majority of users while unfortunately causing porting effort for some users. Most existing users will be unaffected and gain the ability to stop linking KIO if they don't need it. New users won't be confused over why something as canonical-sounding as KAuthorized is in KIO. The alternative would have been to keep the KCoreAuthorized and KAuthorized names and locations, and add forwarders with deprecation notices to KAuthorized for the functions offered in KCoreAuthorized. However, this would mean duplication and a lot of porting churn away from deprecated API, and in the meantime not address the linking issue. This change was discussed with and tentatively agreed upon (pre-code) by dfaure. Diffs (updated) - includes/CMakeLists.txt bcd3789 kfile/kdirselectdialog.cpp ac4a194 kfile/kfilewidget.cpp 4901fcd khtml/java/kjavaappletviewer.cpp b10d993 khtml/khtml_part.cpp b0e31ce khtml/misc/loader.cpp cf451be khtml/xml/dom_docimpl.cpp 2b6125d kio/kfile/kpropertiesdialog.cpp 359ffab kio/misc/ktelnetservice.cpp 1c0d517 kutils/kcmultidialog.cpp 5286866 staging/kbookmarks/src/kbookmarkmenu.cc 6c1569c staging/kcompletion/src/klineedit.cpp 76ef377 staging/kde4support/src/kdeui/kapplication.cpp 12092e9 staging/kio/src/core/CMakeLists.txt 4897d5f staging/kio/src/core/job.cpp 511d1cc staging/kio/src/core/kauthorized.h 473725d staging/kio/src/core/kauthorized.cpp b930715 staging/kio/src/core/kurlauthorized.h PRE-CREATION staging/kio/src/core/kurlauthorized.cpp PRE-CREATION staging/kio/src/core/mkdirjob.cpp 9ed00b0 staging/kio/src/widgets/accessmanagerreply_p.cpp ad9b4f7 staging/kio/src/widgets/kfileitemactions.cpp 99d3209 staging/kio/src/widgets/kopenwithdialog.cpp b46e1ee staging/kio/src/widgets/krun.cpp aba4d7a staging/kio/src/widgets/kurlcompletion.cpp 80a4023 staging/kservice/src/services/kservice.cpp dea8618 staging/xmlgui/src/kactioncollection.cpp 5085f53 staging/xmlgui/src/khelpmenu.cpp cfc52a0 staging/xmlgui/src/ktoolbar.cpp 6a59874 staging/xmlgui/src/ktoolbarhandler.cpp 162de5a staging/xmlgui/src/kxmlguibuilder.cpp 6eb8a51 staging/xmlgui/src/kxmlguiclient.cpp 809033f tier1/kconfig/src/core/CMakeLists.txt 51e21ff tier1/kconfig/src/core/kauthorized.h PRE-CREATION tier1/kconfig/src/core/kauthorized.cpp PRE-CREATION tier1/kconfig/src/core/kcoreauthorized.h 2a3d79e tier1/kconfig/src/core/kcoreauthorized.cpp 97cbc79 tier1/kconfig/src/core/kdesktopfile.cpp a0d5cb8 Diff: http://git.reviewboard.kde.org/r/112722/diff/ Testing --- Thanks, Eike Hein ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 112722: Juggle around Kiosk include names to avoid incorrect library linkage and for better layering
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112722/ --- Review request for KDE Frameworks and David Faure. Description --- In KF5, KAuthorized was split into two: KCoreAuthorized, in KConfig, and KAuthorized, in KIO. Despite the name, KCoreAuthorized offers functions in the KAuthorized namespace, and KAuthorized extends it with additional functions that require KIO APIs. This causes the following problems: * The majority of users of the framework want to use the functions offered in KCoreAuthorized, but when they include kcoreauthorized.h, they have to use the KAuthorized namespace to access them. This sort of non-mapping is bad for the brain :). * While kauthorized.h remaining available via KIO succeds in preserving SC, it does so by muddled-up lib dependencies: Stuff that doesn't actually need KIO might end up linking to it just to avoid changing sources, or just because someone greps for KAuthorized and finds the copy in KIO (this is what happened to me in the Task Manager and made me investigate). The proposed patch does the following to address this: * KCoreAuthorized in KConfig is renamed to KAuthorized. This fixes the include-to-namespace mapping problem and continues to preserve SC for the majority of users. * KAuthorized in KIO is renamed to KUrlAuthorized to hold the URL/KIO-specific functions. This means the few users of those functions (e.g. KHTML) need to be ported to a new include name, but SC is retained via the same namespace. (I think it would have been nice to offer them in a KUrlAuthorized namespace to avoid the include-to-namespace mapping problem here too, but this is complicated by wanting to call non-public symbols expoted by K(Core)Authorized ... ideas welcome.) The bottom line is that this avoids users linking to the wrong API for no reason and continues to preserve SC for the majority of users while unfortunately causing porting effort for some users. Most existing users will be unaffected and gain the ability to stop linking KIO if they don't need it. New users won't be confused over why something as canonical-sounding as KAuthorized is in KIO. The alternative would have been to keep the KCoreAuthorized and KAuthorized names and locations, and add forwarders with deprecation notices to KAuthorized for the functions offered in KCoreAuthorized. However, this would mean duplication and a lot of porting churn away from deprecated API, and in the meantime not address the linking issue. This change was discussed with and tentatively agreed upon (pre-code) by dfaure. Diffs - includes/CMakeLists.txt bcd3789 kfile/kdirselectdialog.cpp ac4a194 kfile/kfilewidget.cpp 4901fcd khtml/java/kjavaappletviewer.cpp b10d993 khtml/khtml_part.cpp b0e31ce khtml/misc/loader.cpp cf451be khtml/xml/dom_docimpl.cpp 2b6125d kio/kfile/kpropertiesdialog.cpp 359ffab kio/misc/ktelnetservice.cpp 1c0d517 kutils/kcmultidialog.cpp 5286866 staging/kbookmarks/src/kbookmarkmenu.cc 6c1569c staging/kcompletion/src/klineedit.cpp 76ef377 staging/kde4support/src/kdeui/kapplication.cpp 12092e9 staging/kio/src/core/CMakeLists.txt 4897d5f staging/kio/src/core/job.cpp 511d1cc staging/kio/src/core/kauthorized.h 473725d staging/kio/src/core/kauthorized.cpp b930715 staging/kio/src/core/kurlauthorized.h PRE-CREATION staging/kio/src/core/kurlauthorized.cpp PRE-CREATION staging/kio/src/core/mkdirjob.cpp 9ed00b0 staging/kio/src/widgets/accessmanagerreply_p.cpp ad9b4f7 staging/kio/src/widgets/kfileitemactions.cpp 99d3209 staging/kio/src/widgets/kopenwithdialog.cpp b46e1ee staging/kio/src/widgets/krun.cpp aba4d7a staging/kio/src/widgets/kurlcompletion.cpp 80a4023 staging/kservice/src/services/kservice.cpp dea8618 staging/xmlgui/src/kactioncollection.cpp 5085f53 staging/xmlgui/src/khelpmenu.cpp cfc52a0 staging/xmlgui/src/ktoolbar.cpp 6a59874 staging/xmlgui/src/ktoolbarhandler.cpp 162de5a staging/xmlgui/src/kxmlguibuilder.cpp 6eb8a51 staging/xmlgui/src/kxmlguiclient.cpp 809033f tier1/kconfig/src/core/CMakeLists.txt 51e21ff tier1/kconfig/src/core/kauthorized.h PRE-CREATION tier1/kconfig/src/core/kauthorized.cpp PRE-CREATION tier1/kconfig/src/core/kcoreauthorized.h 2a3d79e tier1/kconfig/src/core/kcoreauthorized.cpp 97cbc79 tier1/kconfig/src/core/kdesktopfile.cpp a0d5cb8 Diff: http://git.reviewboard.kde.org/r/112722/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: Review Request 112722: Juggle around Kiosk include names to avoid incorrect library linkage and for better layering
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112722/ --- (Updated Sept. 13, 2013, 11:38 p.m.) Review request for KDE Frameworks and David Faure. Changes --- Whoops, one replace too many. Description --- In KF5, KAuthorized was split into two: KCoreAuthorized, in KConfig, and KAuthorized, in KIO. Despite the name, KCoreAuthorized offers functions in the KAuthorized namespace, and KAuthorized extends it with additional functions that require KIO APIs. This causes the following problems: * The majority of users of the framework want to use the functions offered in KCoreAuthorized, but when they include kcoreauthorized.h, they have to use the KAuthorized namespace to access them. This sort of non-mapping is bad for the brain :). * While kauthorized.h remaining available via KIO succeds in preserving SC, it does so by muddled-up lib dependencies: Stuff that doesn't actually need KIO might end up linking to it just to avoid changing sources, or just because someone greps for KAuthorized and finds the copy in KIO (this is what happened to me in the Task Manager and made me investigate). The proposed patch does the following to address this: * KCoreAuthorized in KConfig is renamed to KAuthorized. This fixes the include-to-namespace mapping problem and continues to preserve SC for the majority of users. * KAuthorized in KIO is renamed to KUrlAuthorized to hold the URL/KIO-specific functions. This means the few users of those functions (e.g. KHTML) need to be ported to a new include name, but SC is retained via the same namespace. (I think it would have been nice to offer them in a KUrlAuthorized namespace to avoid the include-to-namespace mapping problem here too, but this is complicated by wanting to call non-public symbols expoted by K(Core)Authorized ... ideas welcome.) The bottom line is that this avoids users linking to the wrong API for no reason and continues to preserve SC for the majority of users while unfortunately causing porting effort for some users. Most existing users will be unaffected and gain the ability to stop linking KIO if they don't need it. New users won't be confused over why something as canonical-sounding as KAuthorized is in KIO. The alternative would have been to keep the KCoreAuthorized and KAuthorized names and locations, and add forwarders with deprecation notices to KAuthorized for the functions offered in KCoreAuthorized. However, this would mean duplication and a lot of porting churn away from deprecated API, and in the meantime not address the linking issue. This change was discussed with and tentatively agreed upon (pre-code) by dfaure. Diffs (updated) - includes/CMakeLists.txt bcd3789 kfile/kdirselectdialog.cpp ac4a194 kfile/kfilewidget.cpp 4901fcd khtml/java/kjavaappletviewer.cpp b10d993 khtml/khtml_part.cpp b0e31ce khtml/misc/loader.cpp cf451be khtml/xml/dom_docimpl.cpp 2b6125d kio/kfile/kpropertiesdialog.cpp 359ffab kio/misc/ktelnetservice.cpp 1c0d517 kutils/kcmultidialog.cpp 5286866 staging/kbookmarks/src/kbookmarkmenu.cc 6c1569c staging/kcompletion/src/klineedit.cpp 76ef377 staging/kde4support/src/kdeui/kapplication.cpp 12092e9 staging/kio/src/core/CMakeLists.txt 4897d5f staging/kio/src/core/job.cpp 511d1cc staging/kio/src/core/kauthorized.h 473725d staging/kio/src/core/kauthorized.cpp b930715 staging/kio/src/core/kurlauthorized.h PRE-CREATION staging/kio/src/core/kurlauthorized.cpp PRE-CREATION staging/kio/src/core/mkdirjob.cpp 9ed00b0 staging/kio/src/widgets/accessmanagerreply_p.cpp ad9b4f7 staging/kio/src/widgets/kfileitemactions.cpp 99d3209 staging/kio/src/widgets/kopenwithdialog.cpp b46e1ee staging/kio/src/widgets/krun.cpp aba4d7a staging/kio/src/widgets/kurlcompletion.cpp 80a4023 staging/kservice/src/services/kservice.cpp dea8618 staging/xmlgui/src/kactioncollection.cpp 5085f53 staging/xmlgui/src/khelpmenu.cpp cfc52a0 staging/xmlgui/src/ktoolbar.cpp 6a59874 staging/xmlgui/src/ktoolbarhandler.cpp 162de5a staging/xmlgui/src/kxmlguibuilder.cpp 6eb8a51 staging/xmlgui/src/kxmlguiclient.cpp 809033f tier1/kconfig/src/core/CMakeLists.txt 51e21ff tier1/kconfig/src/core/kauthorized.h PRE-CREATION tier1/kconfig/src/core/kauthorized.cpp PRE-CREATION tier1/kconfig/src/core/kcoreauthorized.h 2a3d79e tier1/kconfig/src/core/kcoreauthorized.cpp 97cbc79 tier1/kconfig/src/core/kdesktopfile.cpp a0d5cb8 Diff: http://git.reviewboard.kde.org/r/112722/diff/ Testing --- Thanks, Eike Hein ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
