ECM uninstall target
alohas, I just ported phonon/five to ECM and noticed that apparently ECM does not have an include or something to introduce an uninstall target. Are there plans to introduce such a thing or am I simply blind and not seeing the already existing include? HS
Review Request 116783: Change ftpFileExists to use relative paths for SIZE requests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/ --- Review request for kdelibs and David Faure. Repository: kdelibs Description --- I completely missed ftpFileExists issued a SIZE command to determine the existence of a file when addressing bug# 326292. See https://git.reviewboard.kde.org/r/116524/ review request. The attached patch addresses that oversight to insure renaming files work properly on the android ftp server listed in the bug report. Diffs - kioslave/ftp/ftp.h cbcd096 kioslave/ftp/ftp.cpp b9d90e6 Diff: https://git.reviewboard.kde.org/r/116783/diff/ Testing --- Rerun all the tests run for 116524. Thanks, Dawit Alemayehu
Re: Review Request 116783: Change ftpFileExists to use relative paths for SIZE requests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/#review52887 --- Looks good, apart from method name and coding style (spaces in all sorts of wrong places, for historic reasons - OK to leave as it here, but run astyle-kdelibs or clean by hand after patching the kio framework, which has been made consistent style-wise). kioslave/ftp/ftp.cpp https://git.reviewboard.kde.org/r/116783/#comment37232 It's a bit confusing to have two methods called ftpSize. Maybe call this one ftpSizeCmd() or something? (check existing method names for consistency) - David Faure On March 13, 2014, 12:33 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/ --- (Updated March 13, 2014, 12:33 p.m.) Review request for kdelibs and David Faure. Repository: kdelibs Description --- I completely missed ftpFileExists issued a SIZE command to determine the existence of a file when addressing bug# 326292. See https://git.reviewboard.kde.org/r/116524/ review request. The attached patch addresses that oversight to insure renaming files work properly on the android ftp server listed in the bug report. Diffs - kioslave/ftp/ftp.h cbcd096 kioslave/ftp/ftp.cpp b9d90e6 Diff: https://git.reviewboard.kde.org/r/116783/diff/ Testing --- Rerun all the tests run for 116524. Thanks, Dawit Alemayehu
Review Request 116784: Fix incorrect use of KDateTime.toTime_t in kio_http
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116784/ --- Review request for kdelibs, Andreas Hartmetz and David Faure. Repository: kdelibs Description --- The attached patch does the following: - It corrects a mistake in assumption that KDateTime.toTime_t() will return -1 for invalidate dates. It does not. The result is an overflow which is interpreted in kio_http as a timestamp in the distant future which obviously is wrong. See https://bugs.kde.org/show_bug.cgi?id=331774 for example. This assumption also affects the timestamp variables used for cache management. - It converts cache management timestamp variables to 64 bits so they can accomodates dates beyond Feb 7, 2106. Diffs - kioslave/http/http.h dd85622 kioslave/http/http.cpp e4f1eba Diff: https://git.reviewboard.kde.org/r/116784/diff/ Testing --- Thanks, Dawit Alemayehu
Re: Review Request 116783: Change ftpFileExists to use relative paths for SIZE requests
On March 13, 2014, 12:40 p.m., David Faure wrote: kioslave/ftp/ftp.cpp, line 2260 https://git.reviewboard.kde.org/r/116783/diff/2/?file=253641#file253641line2260 It's a bit confusing to have two methods called ftpSize. Maybe call this one ftpSizeCmd() or something? (check existing method names for consistency) Ok. I will rename it ftpSendSizeCmd for consistency and adjust the spaces to 2 for this branches and correct it properly for the frameworks branch. - Dawit --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/#review52887 --- On March 13, 2014, 12:33 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/ --- (Updated March 13, 2014, 12:33 p.m.) Review request for kdelibs and David Faure. Repository: kdelibs Description --- I completely missed ftpFileExists issued a SIZE command to determine the existence of a file when addressing bug# 326292. See https://git.reviewboard.kde.org/r/116524/ review request. The attached patch addresses that oversight to insure renaming files work properly on the android ftp server listed in the bug report. Diffs - kioslave/ftp/ftp.h cbcd096 kioslave/ftp/ftp.cpp b9d90e6 Diff: https://git.reviewboard.kde.org/r/116783/diff/ Testing --- Rerun all the tests run for 116524. Thanks, Dawit Alemayehu
Re: Review Request 116783: Change ftpFileExists to use relative paths for SIZE requests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/ --- (Updated March 13, 2014, 12:58 p.m.) Review request for kdelibs and David Faure. Changes --- Updated patch. Repository: kdelibs Description --- I completely missed ftpFileExists issued a SIZE command to determine the existence of a file when addressing bug# 326292. See https://git.reviewboard.kde.org/r/116524/ review request. The attached patch addresses that oversight to insure renaming files work properly on the android ftp server listed in the bug report. Diffs (updated) - kioslave/ftp/ftp.h cbcd096 kioslave/ftp/ftp.cpp b9d90e6 Diff: https://git.reviewboard.kde.org/r/116783/diff/ Testing --- Rerun all the tests run for 116524. Thanks, Dawit Alemayehu
Re: Review Request 116783: Change ftpFileExists to use relative paths for SIZE requests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/#review52891 --- kioslave/ftp/ftp.cpp https://git.reviewboard.kde.org/r/116783/#comment37234 Misunderstanding, I wasn't talking about indentation (which indeed ends up being 4 spaces everywhere in frameworks). The issue is more: no space after this if, one space too many after psz on the previous line, too many spaces inside the parenthesis on the line Ftp::ftpSize( ... ), etc. My suggestion was to just do this automatically in frameworks using a tool, otherwise you have to do it correctly by hand :-) - David Faure On March 13, 2014, 12:58 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/ --- (Updated March 13, 2014, 12:58 p.m.) Review request for kdelibs and David Faure. Repository: kdelibs Description --- I completely missed ftpFileExists issued a SIZE command to determine the existence of a file when addressing bug# 326292. See https://git.reviewboard.kde.org/r/116524/ review request. The attached patch addresses that oversight to insure renaming files work properly on the android ftp server listed in the bug report. Diffs - kioslave/ftp/ftp.h cbcd096 kioslave/ftp/ftp.cpp b9d90e6 Diff: https://git.reviewboard.kde.org/r/116783/diff/ Testing --- Rerun all the tests run for 116524. Thanks, Dawit Alemayehu
Re: Review Request 116784: Fix incorrect use of KDateTime.toTime_t in kio_http
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116784/#review52897 --- The handling of return values from KDateTime::toTime_t() in the existing kio_http code is not correct, because the return value's type is implicitly cast to other types before being checked. For example, in one place it is cast to qint64, which will result in a value of 0x instead of 0x (= -1). This type of error will mask the fact that the error value is being returned. Instead of changing the calling code to detect invalid dates using other methods, it should be fixed to properly cast the uint value returned from KDateTime::toTime_t(). For types other than int, it needs to specifically check for uint(-1) and set the cast value to -1 in that case. For example: uint t = KDateTime::toTime_t(...); // Set the qint64 to be -1 if an error occurred: qint64 result = (t == uint(-1)) ? -1 : t; Note: KDateTime::toTime_t() is *supposed* to return uint(-1) to indicate an error. If it doesn't always do this, *it* should be fixed instead of changing code elsewhere, since kio_http is unlikely to be the only module that will have trouble if that is happening. - David Jarvie On March 13, 2014, 12:49 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116784/ --- (Updated March 13, 2014, 12:49 p.m.) Review request for kdelibs, Andreas Hartmetz and David Faure. Repository: kdelibs Description --- The attached patch does the following: - It corrects a mistake in assumption that KDateTime.toTime_t() will return -1 for invalidate dates. It does not. The result is an overflow which is interpreted in kio_http as a timestamp in the distant future which obviously is wrong. See https://bugs.kde.org/show_bug.cgi?id=331774 for example. This assumption also affects the timestamp variables used for cache management. - It converts cache management timestamp variables to 64 bits so they can accomodates dates beyond Feb 7, 2106. Diffs - kioslave/http/http.h dd85622 kioslave/http/http.cpp e4f1eba Diff: https://git.reviewboard.kde.org/r/116784/diff/ Testing --- Thanks, Dawit Alemayehu
Re: Review Request 109675: Make sure that the KDE prefix comes first in XDG_DATA_DIRS
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/109675/#review52898 --- introduced regression'y behavior, see https://bugs.kde.org/show_bug.cgi?id=332107 - Rex Dieter On March 25, 2013, 7:14 p.m., Andreas Hartmetz wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/109675/ --- (Updated March 25, 2013, 7:14 p.m.) Review request for kdelibs and Vishesh Handa. Repository: kde-workspace Description --- Planned commit message: Make sure that the KDE prefix comes first in XDG_DATA_DIRS. I tracked down a Nepomuk problem to this. Nepomuk file indexing didn't work because the ontologies were too old. Nepomuk loaded ontologies from /usr/share instead of my KDE prefix /opt/kde4/share, because /opt/kde4 was the very last entry in the respective search list in KStandardDirs. The first entries in that search list all came from XDG_DATA_DIRS, which in my case (Kubuntu) is set by the X session initialization scripts. That is before startkde runs, so startkde never touched XDG_DATA_DIRS. But it should, and now it does. Diffs - startkde.cmake 8361fe0 Diff: https://git.reviewboard.kde.org/r/109675/diff/ Testing --- Thanks, Andreas Hartmetz
Re: Review Request 116783: Change ftpFileExists to use relative paths for SIZE requests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/ --- (Updated March 13, 2014, 10:15 p.m.) Review request for kdelibs and David Faure. Changes --- updated patch Repository: kdelibs Description --- I completely missed ftpFileExists issued a SIZE command to determine the existence of a file when addressing bug# 326292. See https://git.reviewboard.kde.org/r/116524/ review request. The attached patch addresses that oversight to insure renaming files work properly on the android ftp server listed in the bug report. Diffs (updated) - kioslave/ftp/ftp.h cbcd096 kioslave/ftp/ftp.cpp b9d90e6 Diff: https://git.reviewboard.kde.org/r/116783/diff/ Testing --- Rerun all the tests run for 116524. Thanks, Dawit Alemayehu
Re: Review Request 116783: Change ftpFileExists to use relative paths for SIZE requests
On March 13, 2014, 1:10 p.m., David Faure wrote: kioslave/ftp/ftp.cpp, line 2302 https://git.reviewboard.kde.org/r/116783/diff/3/?file=253646#file253646line2302 Misunderstanding, I wasn't talking about indentation (which indeed ends up being 4 spaces everywhere in frameworks). The issue is more: no space after this if, one space too many after psz on the previous line, too many spaces inside the parenthesis on the line Ftp::ftpSize( ... ), etc. My suggestion was to just do this automatically in frameworks using a tool, otherwise you have to do it correctly by hand :-) I will use the tool you mentioned for frameworks, but for this one I have tried to fix all the glaring ones you mentioned - Dawit --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/#review52891 --- On March 13, 2014, 10:15 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/ --- (Updated March 13, 2014, 10:15 p.m.) Review request for kdelibs and David Faure. Repository: kdelibs Description --- I completely missed ftpFileExists issued a SIZE command to determine the existence of a file when addressing bug# 326292. See https://git.reviewboard.kde.org/r/116524/ review request. The attached patch addresses that oversight to insure renaming files work properly on the android ftp server listed in the bug report. Diffs - kioslave/ftp/ftp.h cbcd096 kioslave/ftp/ftp.cpp b9d90e6 Diff: https://git.reviewboard.kde.org/r/116783/diff/ Testing --- Rerun all the tests run for 116524. Thanks, Dawit Alemayehu
Re: Review Request 116783: Change ftpFileExists to use relative paths for SIZE requests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/#review52916 --- Ship it! Ship It! - David Faure On March 13, 2014, 10:15 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/ --- (Updated March 13, 2014, 10:15 p.m.) Review request for kdelibs and David Faure. Repository: kdelibs Description --- I completely missed ftpFileExists issued a SIZE command to determine the existence of a file when addressing bug# 326292. See https://git.reviewboard.kde.org/r/116524/ review request. The attached patch addresses that oversight to insure renaming files work properly on the android ftp server listed in the bug report. Diffs - kioslave/ftp/ftp.h cbcd096 kioslave/ftp/ftp.cpp b9d90e6 Diff: https://git.reviewboard.kde.org/r/116783/diff/ Testing --- Rerun all the tests run for 116524. Thanks, Dawit Alemayehu
Re: Review Request 116784: Fix incorrect use of KDateTime.toTime_t in kio_http
On March 13, 2014, 5:30 p.m., David Jarvie wrote: The handling of return values from KDateTime::toTime_t() in the existing kio_http code is not correct, because the return value's type is implicitly cast to other types before being checked. For example, in one place it is cast to qint64, which will result in a value of 0x instead of 0x (= -1). This type of error will mask the fact that the error value is being returned. Instead of changing the calling code to detect invalid dates using other methods, it should be fixed to properly cast the uint value returned from KDateTime::toTime_t(). For types other than int, it needs to specifically check for uint(-1) and set the cast value to -1 in that case. For example: uint t = KDateTime::toTime_t(...); // Set the qint64 to be -1 if an error occurred: qint64 result = (t == uint(-1)) ? -1 : t; Note: KDateTime::toTime_t() is *supposed* to return uint(-1) to indicate an error. If it doesn't always do this, *it* should be fixed instead of changing code elsewhere, since kio_http is unlikely to be the only module that will have trouble if that is happening. Perhaps it was not clear from the description, but I am not implying nor have I implied there to be a bug in KDateTime. As I have clearly stated the problem is with the assumption the code in kio_http makes about what KDateTime::toTime_t returns for an invalid date. No matter how you see it the toTime_t() function can not and does not return a literal -1, which is exactly what the code in kio_http assumes! Of course that is clearly wrong. Anyhow, this patch is specifically intended to fix that issue and nothing else. - Dawit --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116784/#review52897 --- On March 13, 2014, 12:49 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116784/ --- (Updated March 13, 2014, 12:49 p.m.) Review request for kdelibs, Andreas Hartmetz and David Faure. Repository: kdelibs Description --- The attached patch does the following: - It corrects a mistake in assumption that KDateTime.toTime_t() will return -1 for invalidate dates. It does not. The result is an overflow which is interpreted in kio_http as a timestamp in the distant future which obviously is wrong. See https://bugs.kde.org/show_bug.cgi?id=331774 for example. This assumption also affects the timestamp variables used for cache management. - It converts cache management timestamp variables to 64 bits so they can accomodates dates beyond Feb 7, 2106. Diffs - kioslave/http/http.h dd85622 kioslave/http/http.cpp e4f1eba Diff: https://git.reviewboard.kde.org/r/116784/diff/ Testing --- Thanks, Dawit Alemayehu
Re: Review Request 116783: Change ftpFileExists to use relative paths for SIZE requests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/ --- (Updated March 13, 2014, 11:30 p.m.) Status -- This change has been marked as submitted. Review request for kdelibs and David Faure. Repository: kdelibs Description --- I completely missed ftpFileExists issued a SIZE command to determine the existence of a file when addressing bug# 326292. See https://git.reviewboard.kde.org/r/116524/ review request. The attached patch addresses that oversight to insure renaming files work properly on the android ftp server listed in the bug report. Diffs - kioslave/ftp/ftp.h cbcd096 kioslave/ftp/ftp.cpp b9d90e6 Diff: https://git.reviewboard.kde.org/r/116783/diff/ Testing --- Rerun all the tests run for 116524. Thanks, Dawit Alemayehu
Re: Review Request 116783: Change ftpFileExists to use relative paths for SIZE requests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/#review52918 --- This review has been submitted with commit 0d8913d2b544c16043262ac5380fa899b711d59b by Dawit Alemayehu to branch KDE/4.12. - Commit Hook On March 13, 2014, 10:15 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116783/ --- (Updated March 13, 2014, 10:15 p.m.) Review request for kdelibs and David Faure. Repository: kdelibs Description --- I completely missed ftpFileExists issued a SIZE command to determine the existence of a file when addressing bug# 326292. See https://git.reviewboard.kde.org/r/116524/ review request. The attached patch addresses that oversight to insure renaming files work properly on the android ftp server listed in the bug report. Diffs - kioslave/ftp/ftp.h cbcd096 kioslave/ftp/ftp.cpp b9d90e6 Diff: https://git.reviewboard.kde.org/r/116783/diff/ Testing --- Rerun all the tests run for 116524. Thanks, Dawit Alemayehu
