Re: Review Request 115351: Use kDebug instead of qDebug in QSpellEnchantDict
On Jan. 28, 2014, 5:49 p.m., Aleix Pol Gonzalez wrote: Should we even comment/remove them? Doesn't seem like anybody's going to read this output ever... Ping, can someone tell me what to do with this? Can I either push this change or remove the output altogether? - Milian --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115351/#review48493 --- On Jan. 28, 2014, 1 p.m., Milian Wolff wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115351/ --- (Updated Jan. 28, 2014, 1 p.m.) Review request for kdelibs. Repository: kdelibs Description --- Enabling automatic spell checking in Kate, I see a lot of debug output in the form of: Enchant dict for en_US 0x43a1480 This patch should fix this and only output the lines when debug output is enabled via kdebugdialog. Diffs - sonnet/plugins/enchant/enchantdict.cpp c289d26 Diff: https://git.reviewboard.kde.org/r/115351/diff/ Testing --- None actually, not even tested if it compiles. Thanks, Milian Wolff
Re: Review Request 115351: Use kDebug instead of qDebug in QSpellEnchantDict
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115351/#review52053 --- Heh, the Git history looks very sad in this part of kdelibs. Noone worked on this for years now. Personally I'd just get rid off the debug output, seems like noone cares anyway. (I'm also annoyed by the useless output) - Kevin Funk On Jan. 28, 2014, 1 p.m., Milian Wolff wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115351/ --- (Updated Jan. 28, 2014, 1 p.m.) Review request for kdelibs. Repository: kdelibs Description --- Enabling automatic spell checking in Kate, I see a lot of debug output in the form of: Enchant dict for en_US 0x43a1480 This patch should fix this and only output the lines when debug output is enabled via kdebugdialog. Diffs - sonnet/plugins/enchant/enchantdict.cpp c289d26 Diff: https://git.reviewboard.kde.org/r/115351/diff/ Testing --- None actually, not even tested if it compiles. Thanks, Milian Wolff
Re: Review Request 116523: Do not call ftpSendMimeType for empty files in Ftp::ftpGet
On March 4, 2014, 7:42 p.m., David Faure wrote: I don't get it. What's the problem with sending a mimetype for empty files? I would think this is actually expected - for all files, including empty ones. Why does this fix the bug? It fails and ends up sending an error message. See the if statement in while(true) loop in ftpSendMimeType. Even if that check does not fail and the while(true) loop completes successfully, the next statement if (!buffer.isEmpty()) will be false for empty files. Hence it is completely useless to call ftpSendMimeType from ftpGet for empty files. - Dawit --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/#review51922 --- On March 2, 2014, 2:20 p.m., Dawit Alemayehu wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116523/ --- (Updated March 2, 2014, 2:20 p.m.) Review request for kdelibs and David Faure. Bugs: 323491 http://bugs.kde.org/show_bug.cgi?id=323491 Repository: kdelibs Description --- The attached patch fixes a bug where copying empty files (size == 0) from an ftp server to any other remote server (sftp, ftp) results in an error message that states the file could not be opened. Diffs - kioslave/ftp/ftp.cpp 5bb2e8d Diff: https://git.reviewboard.kde.org/r/116523/diff/ Testing --- Attempt to copy an empty file from any ftp server to a remote destination, e.g. sftp server. Thanks, Dawit Alemayehu
Re: Review Request 116122: Do not add trailing slash on http DELETE requests
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116122/ --- (Updated March 5, 2014, 2:01 p.m.) Review request for kdelibs, Andrea Iacovitti and David Faure. Changes --- Updated patch. The problem was actually in DeleteJob where http_delete and not rmdir should have been used to delete http/webdav folder. Bugs: 331295 http://bugs.kde.org/show_bug.cgi?id=331295 Repository: kdelibs Description --- This patch fixes a prior commit, http://commits.kde.org/kdelibs/58294ac, which attempted to workaround webdav servers that do not accept delete requests on directories unless there is a trailing slash. It ensures that the fix is only applicable to webdav and the addition of the trailing slash is communicated to the client application. Diffs (updated) - kio/kio/deletejob.cpp b0268ef kioslave/http/http.cpp b4d64d4 Diff: https://git.reviewboard.kde.org/r/116122/diff/ Testing --- For HTTP delete: http://greenbytes.de/tech/tc/httpredirects/t301methods.html For WebDAV delete: http://boonfaya.com/sites/webdavapps.com/#targets Thanks, Dawit Alemayehu
Re: Review Request 116604: Allow directories with . as output for meinproc
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116604/#review52091 --- src/meinproc.cpp https://git.reviewboard.kde.org/r/116604/#comment36985 How does this affect this code in KHelpcenter: kde-runtime/khelpcenter/glossary.cpp:149:KProcess *meinproc = new KProcess; kde-runtime/khelpcenter/glossary.cpp:153:*meinproc KStandardDirs::locate( exe, QLatin1String( meinproc4 ) ); kde-runtime/khelpcenter/glossary.cpp:154:*meinproc QLatin1String( --output ) m_cacheFile; kde-runtime/khelpcenter/glossary.cpp:155:*meinproc QLatin1String( --stylesheet ) kde-runtime/khelpcenter/glossary.cpp:157:*meinproc m_sourceFile; kde-runtime/khelpcenter/glossary.cpp:176:KProcess *meinproc = static_castKProcess *(sender()); kde-runtime/khelpcenter/toc.cpp:148:KProcess *meinproc = new KProcess; kde-runtime/khelpcenter/toc.cpp:152:*meinproc KStandardDirs::locate(exe, meinproc4); kde-runtime/khelpcenter/toc.cpp:153:*meinproc --stylesheet KStandardDirs::locate( data, khelpcenter/table-of-contents.xslt ); kde-runtime/khelpcenter/toc.cpp:154:*meinproc --output m_cacheFile; kde-runtime/khelpcenter/toc.cpp:155:*meinproc m_sourceFile; kde-runtime/khelpcenter/toc.cpp:172:KProcess *meinproc = static_castKProcess *(sender()); About the issue with dot in Path see also http://lists.kde.org/?l=kde-doc-englishm=127421104303628w=2 - Burkhard Lück On March 5, 2014, 1:06 a.m., Luigi Toscano wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116604/ --- (Updated March 5, 2014, 1:06 a.m.) Review request for Documentation, KDE Frameworks, kdelibs, and Aleix Pol Gonzalez. Bugs: 246755 https://bugs.kde.org/show_bug.cgi?id=246755 Repository: kdoctools Description --- The outputFile parameter is not used by the stylesheets, so don't pass it. If a directory starts with ., it is interpreted in a wrong way by libxslt with an error like: --- XPath error : Invalid expression /home/kde-devel/.cache5/khelpcenter/help/__home__kde- devel__kde__share__doc__HTML__en__kioslave__file__index.docbook ^ runtime error Evaluating user parameter outputFile failed --- This is an old issue, it was solved on windows by not compiling that code, but I suspect that the issue has been in UNIX systems too for a long time. Another way to solve the bug is quoting the value of the parameter with '...', replacing: params.append(qstrdup(parser.value(QStringLiteral(output)).toLocal8Bit().constData())); with something like QString quotedOutput = ' + parser.value(QStringLiteral(output)) + '; params.append(qstrdup(quotedOutput.toLocal8Bit().constData())); but anyway in this case the name of output file is not used, or I can't find any occurrence in the stylesheets. The stylesheet is applied and the name of the file is used only after to write the generated XML (see tranform() function). A similar patch can be applied to kdelibs/kdoctools too (same codepath). Diffs - src/meinproc.cpp 95adcea Diff: https://git.reviewboard.kde.org/r/116604/diff/ Testing --- Run meinproc5 (and 4) with -o /something/with/a/.dotdir/myfile.txt (the directory must exist), no error anymore and the file is generated. Thanks, Luigi Toscano
Re: Review Request 115351: Use kDebug instead of qDebug in QSpellEnchantDict
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115351/#review52204 --- This review has been submitted with commit 42b779e3efca6cddf316b6df85a36363288a9db4 by Milian Wolff to branch KDE/4.13. - Commit Hook On Jan. 28, 2014, 1 p.m., Milian Wolff wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115351/ --- (Updated Jan. 28, 2014, 1 p.m.) Review request for kdelibs. Repository: kdelibs Description --- Enabling automatic spell checking in Kate, I see a lot of debug output in the form of: Enchant dict for en_US 0x43a1480 This patch should fix this and only output the lines when debug output is enabled via kdebugdialog. Diffs - sonnet/plugins/enchant/enchantdict.cpp c289d26 Diff: https://git.reviewboard.kde.org/r/115351/diff/ Testing --- None actually, not even tested if it compiles. Thanks, Milian Wolff
Re: Review Request 115351: Use kDebug instead of qDebug in QSpellEnchantDict
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115351/ --- (Updated March 5, 2014, 7:11 p.m.) Status -- This change has been marked as submitted. Review request for kdelibs. Repository: kdelibs Description --- Enabling automatic spell checking in Kate, I see a lot of debug output in the form of: Enchant dict for en_US 0x43a1480 This patch should fix this and only output the lines when debug output is enabled via kdebugdialog. Diffs - sonnet/plugins/enchant/enchantdict.cpp c289d26 Diff: https://git.reviewboard.kde.org/r/115351/diff/ Testing --- None actually, not even tested if it compiles. Thanks, Milian Wolff
Re: Review Request 116555: Add support for pam-kwallet in kwalletd
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116555/#review52218 --- Ship it! Ship It! - Valentin Rusu On March 2, 2014, 11:33 p.m., Àlex Fiestas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116555/ --- (Updated March 2, 2014, 11:33 p.m.) Review request for KDE Runtime, Release Team and Valentin Rusu. Repository: kde-runtime Description --- This patch adds support for pam-kwallet (in my scratch right now, to be released soon). This is how the new pam works, and why this patch is needed: In order to open the wallet in a secure way we have to try hard to not send the hash on an insecure manner. This is how we achieve that: -pam_kwallet creates a pipe. -pam_kwallet opens a local socket listening somewhere (/tmp/foo.socket for example). -pam_kwallet forks+execv kwallet, passing via arguments the sockets (pipe and local socket). -pam_kwallet sends the hash via the pipe. -kwalletd gets the hash and waits for the environment. -startkde uses socat to send the environment to kwalletd. -kwalletd setups the environment before any Qt code is executed. -kwalletd resumes execution. With this way of executing kwallet we get: -pam_kwallet knows to who it is sending the hash (its on child). -hash is never revealed on shared memory (dbus), since pipes are private to the apps. -ptrace is usually disabled so only root can see the hash on the app memory -no Qt code is executed without the proper environment (same as startkde) -if kwalletd is executed normally (not from pam_kwallet) then it is business as usual. The patch also comes with integration tests that simulate how kwalletd is executed in the pam module. For the release team, I would love to add this to 4.13, after all it is innocuous if kwalletd is not executed via pam_module. Diffs - kwalletd/CMakeLists.txt e9aef16 kwalletd/autotests/CMakeLists.txt PRE-CREATION kwalletd/autotests/kdewallet.kwl PRE-CREATION kwalletd/autotests/kwalletexecuter.h PRE-CREATION kwalletd/autotests/kwalletexecuter.cpp PRE-CREATION kwalletd/autotests/qtest_kwallet.h PRE-CREATION kwalletd/autotests/testpamopen.cpp PRE-CREATION kwalletd/autotests/testpamopennofile.cpp PRE-CREATION kwalletd/main.cpp f9af414 Diff: https://git.reviewboard.kde.org/r/116555/diff/ Testing --- Thanks, Àlex Fiestas
Re: Review Request 116555: Add support for pam-kwallet in kwalletd
On March 3, 2014, 11:57 p.m., Albert Astals Cid wrote: The Beta 1 of 4.13 is on Wednesday. Can the maintainer of the code affected by this give a evaluation of how dangerous this patch is and his recommendation for the Freeze exception? Oh, I only see this now. Is it too late? - Valentin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116555/#review51842 --- On March 2, 2014, 11:33 p.m., Àlex Fiestas wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/116555/ --- (Updated March 2, 2014, 11:33 p.m.) Review request for KDE Runtime, Release Team and Valentin Rusu. Repository: kde-runtime Description --- This patch adds support for pam-kwallet (in my scratch right now, to be released soon). This is how the new pam works, and why this patch is needed: In order to open the wallet in a secure way we have to try hard to not send the hash on an insecure manner. This is how we achieve that: -pam_kwallet creates a pipe. -pam_kwallet opens a local socket listening somewhere (/tmp/foo.socket for example). -pam_kwallet forks+execv kwallet, passing via arguments the sockets (pipe and local socket). -pam_kwallet sends the hash via the pipe. -kwalletd gets the hash and waits for the environment. -startkde uses socat to send the environment to kwalletd. -kwalletd setups the environment before any Qt code is executed. -kwalletd resumes execution. With this way of executing kwallet we get: -pam_kwallet knows to who it is sending the hash (its on child). -hash is never revealed on shared memory (dbus), since pipes are private to the apps. -ptrace is usually disabled so only root can see the hash on the app memory -no Qt code is executed without the proper environment (same as startkde) -if kwalletd is executed normally (not from pam_kwallet) then it is business as usual. The patch also comes with integration tests that simulate how kwalletd is executed in the pam module. For the release team, I would love to add this to 4.13, after all it is innocuous if kwalletd is not executed via pam_module. Diffs - kwalletd/CMakeLists.txt e9aef16 kwalletd/autotests/CMakeLists.txt PRE-CREATION kwalletd/autotests/kdewallet.kwl PRE-CREATION kwalletd/autotests/kwalletexecuter.h PRE-CREATION kwalletd/autotests/kwalletexecuter.cpp PRE-CREATION kwalletd/autotests/qtest_kwallet.h PRE-CREATION kwalletd/autotests/testpamopen.cpp PRE-CREATION kwalletd/autotests/testpamopennofile.cpp PRE-CREATION kwalletd/main.cpp f9af414 Diff: https://git.reviewboard.kde.org/r/116555/diff/ Testing --- Thanks, Àlex Fiestas
Re: KF5 Update Meeting Minutes 2014-w10
Hello, Catching up with KF5 progress... On Tuesday, March 04, 2014 04:59:58 PM Kevin Ottens wrote: * Repository merges for kwallet and kdnssd still reported as pending... What's that? Is that about merging 4.xx repository with the KF5 one? * afiestas has kwallet patches in need of review, valir being unresponsive help is welcome; Reviewed! Nice piece of work, thanks! Regards, Valentin signature.asc Description: This is a digitally signed message part.
Re: KF5 Update Meeting Minutes 2014-w10
Hello, On Thursday 06 March 2014 02:13:36 Valentin Rusu wrote: Catching up with KF5 progress... On Tuesday, March 04, 2014 04:59:58 PM Kevin Ottens wrote: * Repository merges for kwallet and kdnssd still reported as pending... What's that? Is that about merging 4.xx repository with the KF5 one? No, the repository merge which already happened. Somehow no one closed the task, but it's done now. 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.
