Re: Review Request 120627: Remove kdelibs4support.

2014-10-18 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120627/#review68660
---



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120627/#comment47858

This use of QTemporaryFile is not secure. There is a hole between the 
creation time of the temporary file and the start time of KIO::file_copy in 
which an attacker could create the file. I don't know whether KIO would follow 
malicious symlinks, but at the very least the attacker could make the download 
fail.

Needless to say, the revision 4 solution tempFileName = QDir::tempPath() + 
/kompare. + url.fileName(); is also insecure.

What about using QTemporaryDir also in this situation? Then you have a 
secure temporary directory that you own, inside which you can safely use even 
predictable file names, and you can also reuse the existing cleanup code.


- Kevin Kofler


On Okt. 18, 2014, 5:01 vorm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120627/
 ---
 
 (Updated Okt. 18, 2014, 5:01 vorm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: kompare
 
 
 Description
 ---
 
 Change KUrl to QUrl.
 Use QLayout/QFrame instead of KVBox (seems broken though somehow)
 Use QFileDialog instead of KFileDialog.
 
 
 Diffs
 -
 
   libdialogpages/pagebase.cpp ba1574aed7124ede49e1c5908a8fe693cf7bc5d3 
   libdialogpages/viewpage.h b5b770d1441650564106e1cc7ef7e587f6ee142d 
   libdialogpages/viewpage.cpp 07bdba5e1edf55a6dcd02e5deef58d30c07660c2 
   libdialogpages/filessettings.h dc3306e34fe1b4eb7cb6a9d2b598f91932bedda0 
   libdialogpages/filessettings.cpp 0e19dc00f22a2f6e9588bf2d110dbde682888472 
   libdialogpages/pagebase.h 0cef46feaa2cc81deff12c2c5f739e6be6df1b49 
   libdialogpages/CMakeLists.txt 769a1154c56e8eb8aa42f1bc6d84e0f9a4154fd0 
   libdialogpages/dialogpagesexport.h b2de57f6616739d353d4889ef4965ab07f1191aa 
   libdialogpages/diffpage.h 37490b1ebb245e9648530429da63a9240010 
   libdialogpages/diffpage.cpp 7800b486e023cffe41e1fa3e9e60781250ea4199 
   libdialogpages/filespage.h 42afafcd0fc8bc0a01e32b79d414742937d791fb 
   libdialogpages/filespage.cpp 6a87fe36abd57bdaa09b516de38969db6c6f2298 
   kompareurldialog.h dc50c588e70835ad9292da1baf5222f58f512f67 
   kompareurldialog.cpp 7de050bc44770a79f8f7d789cabd95d6707a40f1 
   komparepart/komparesplitter.cpp 8d496bf279caa7cb9a305c2d15131f591c48818d 
   komparepart/komparelistview.cpp 35bbab849d8b7938cba518e97a00ed50cae35612 
   komparepart/kompareprefdlg.cpp 118485663390e9563a77741b490a9cdf8bf6d464 
   komparepart/komparesaveoptionswidget.cpp 
 4c9acba6a7f9c6dda04130946faac37138422875 
   interfaces/kompareinterface.h 53b19d944b2a4a65c14ea41b8f1c0997581933db 
   kompare_shell.h 8549fcdc4d1536c58734f2bc3a78b9ebc42c6c5f 
   kompare_shell.cpp dcc45513f3f9f5f94869046989b6b4f5b1c0995e 
   komparenavtreepart/CMakeLists.txt 53e8e670e70629afac9197fc108d844733ec5c07 
   komparenavtreepart/komparenavtreepart.cpp 
 3faceff78fbbd2f083cd0a7837c74f50fe543474 
   komparepart/CMakeLists.txt 09b61e6ca0cdce391fc759be49a672a050cc16cd 
   komparepart/kompare_part.h 24475f1b0ccf7fbeda56860a9a69955cd0b82808 
   komparepart/kompare_part.cpp 4d40be0dedcfb91b77ee239de11188b328f8bc13 
   CMakeLists.txt 38167c2099d0ea1600bd5a6893982e809902fa3a 
   doc/index.docbook 578d12a41d9a6afed441ffd38c39bff16c096ab2 
   libdialogpages/viewsettings.h dbf6afe0d0c70e548e32dfc09391d67ef595cdba 
   libdialogpages/viewsettings.cpp 5a69d0bd9a49f7a3881940c4ea8ad407be56adc1 
   main.cpp 4132c8442f8546ee7d365051dda0e32196249217 
 
 Diff: https://git.reviewboard.kde.org/r/120627/diff/
 
 
 Testing
 ---
 
 It builds and runs. The compare dialog ui looks squished though and doesn't 
 resize like it used to, must be something I did wrong when porting away from 
 KVBox
 
 
 Thanks,
 
 Jeremy Whiting
 




Re: QUrl from a string (porting KUrl constructor or KCmdLineArgs::url)

2014-10-18 Thread Kevin Kofler
Alex Merry wrote:
 Could you add this to the porting guide, please?

Done, also added KUrl::prettyUrl → QUrl::toDisplayString as per the other 
thread.

Kevin Kofler



Re: Review Request 120627: Remove kdelibs4support.

2014-10-18 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120627/#review68677
---

Ship it!


Looks good now.

- Kevin Kofler


On Okt. 18, 2014, 9:03 nachm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120627/
 ---
 
 (Updated Okt. 18, 2014, 9:03 nachm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: kompare
 
 
 Description
 ---
 
 Change KUrl to QUrl.
 Use QLayout/QFrame instead of KVBox (seems broken though somehow)
 Use QFileDialog instead of KFileDialog.
 
 
 Diffs
 -
 
   main.cpp 4132c8442f8546ee7d365051dda0e32196249217 
   libdialogpages/pagebase.h 0cef46feaa2cc81deff12c2c5f739e6be6df1b49 
   libdialogpages/pagebase.cpp ba1574aed7124ede49e1c5908a8fe693cf7bc5d3 
   libdialogpages/viewpage.h b5b770d1441650564106e1cc7ef7e587f6ee142d 
   libdialogpages/viewpage.cpp 07bdba5e1edf55a6dcd02e5deef58d30c07660c2 
   libdialogpages/viewsettings.h dbf6afe0d0c70e548e32dfc09391d67ef595cdba 
   libdialogpages/viewsettings.cpp 5a69d0bd9a49f7a3881940c4ea8ad407be56adc1 
   komparepart/komparesaveoptionswidget.cpp 
 4c9acba6a7f9c6dda04130946faac37138422875 
   komparepart/komparesplitter.cpp 8d496bf279caa7cb9a305c2d15131f591c48818d 
   kompareurldialog.h dc50c588e70835ad9292da1baf5222f58f512f67 
   kompareurldialog.cpp 7de050bc44770a79f8f7d789cabd95d6707a40f1 
   libdialogpages/CMakeLists.txt 769a1154c56e8eb8aa42f1bc6d84e0f9a4154fd0 
   libdialogpages/dialogpagesexport.h b2de57f6616739d353d4889ef4965ab07f1191aa 
   libdialogpages/diffpage.h 37490b1ebb245e9648530429da63a9240010 
   libdialogpages/diffpage.cpp 7800b486e023cffe41e1fa3e9e60781250ea4199 
   libdialogpages/filespage.h 42afafcd0fc8bc0a01e32b79d414742937d791fb 
   libdialogpages/filespage.cpp 6a87fe36abd57bdaa09b516de38969db6c6f2298 
   libdialogpages/filessettings.h dc3306e34fe1b4eb7cb6a9d2b598f91932bedda0 
   libdialogpages/filessettings.cpp 0e19dc00f22a2f6e9588bf2d110dbde682888472 
   komparepart/kompare_part.h 24475f1b0ccf7fbeda56860a9a69955cd0b82808 
   komparepart/kompare_part.cpp 4d40be0dedcfb91b77ee239de11188b328f8bc13 
   komparepart/komparelistview.cpp 35bbab849d8b7938cba518e97a00ed50cae35612 
   komparepart/kompareprefdlg.cpp 118485663390e9563a77741b490a9cdf8bf6d464 
   CMakeLists.txt 38167c2099d0ea1600bd5a6893982e809902fa3a 
   doc/index.docbook 578d12a41d9a6afed441ffd38c39bff16c096ab2 
   interfaces/kompareinterface.h 53b19d944b2a4a65c14ea41b8f1c0997581933db 
   kompare_shell.h 8549fcdc4d1536c58734f2bc3a78b9ebc42c6c5f 
   kompare_shell.cpp dcc45513f3f9f5f94869046989b6b4f5b1c0995e 
   komparenavtreepart/CMakeLists.txt 53e8e670e70629afac9197fc108d844733ec5c07 
   komparenavtreepart/komparenavtreepart.cpp 
 3faceff78fbbd2f083cd0a7837c74f50fe543474 
   komparepart/CMakeLists.txt 09b61e6ca0cdce391fc759be49a672a050cc16cd 
 
 Diff: https://git.reviewboard.kde.org/r/120627/diff/
 
 
 Testing
 ---
 
 It builds and runs. The compare dialog ui looks squished though and doesn't 
 resize like it used to, must be something I did wrong when porting away from 
 KVBox
 
 
 Thanks,
 
 Jeremy Whiting
 




Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120627/#review68622
---


 Use QLayout/QFrame instead of KVBox (seems broken though somehow)

Then I suggest you either fix it, or submit only the parts of the port that 
work.

We have time until KF6 to port away from kdelibs4support, that's years ahead. 
(The last use of Qt3Support and kde3support in Kompare was eliminated in 
November 2011, after Qt 5 was announced, and almost 4 years after KDE 4.0.0.) 
There is really no need to commit a half-broken port just to eliminate 
kdelibs4support now.

- Kevin Kofler


On Okt. 17, 2014, 4:17 nachm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120627/
 ---
 
 (Updated Okt. 17, 2014, 4:17 nachm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: kompare
 
 
 Description
 ---
 
 Change KUrl to QUrl.
 Use QLayout/QFrame instead of KVBox (seems broken though somehow)
 Use QFileDialog instead of KFileDialog.
 
 
 Diffs
 -
 
   libdialogpages/filespage.cpp 6a87fe36abd57bdaa09b516de38969db6c6f2298 
   libdialogpages/filessettings.h dc3306e34fe1b4eb7cb6a9d2b598f91932bedda0 
   libdialogpages/filessettings.cpp 0e19dc00f22a2f6e9588bf2d110dbde682888472 
   libdialogpages/pagebase.h 0cef46feaa2cc81deff12c2c5f739e6be6df1b49 
   libdialogpages/pagebase.cpp ba1574aed7124ede49e1c5908a8fe693cf7bc5d3 
   libdialogpages/viewpage.h b5b770d1441650564106e1cc7ef7e587f6ee142d 
   libdialogpages/viewpage.cpp 07bdba5e1edf55a6dcd02e5deef58d30c07660c2 
   libdialogpages/viewsettings.h dbf6afe0d0c70e548e32dfc09391d67ef595cdba 
   libdialogpages/viewsettings.cpp 5a69d0bd9a49f7a3881940c4ea8ad407be56adc1 
   main.cpp 4132c8442f8546ee7d365051dda0e32196249217 
   libdialogpages/filespage.h 42afafcd0fc8bc0a01e32b79d414742937d791fb 
   libdialogpages/dialogpagesexport.h b2de57f6616739d353d4889ef4965ab07f1191aa 
   libdialogpages/diffpage.h 37490b1ebb245e9648530429da63a9240010 
   libdialogpages/diffpage.cpp 7800b486e023cffe41e1fa3e9e60781250ea4199 
   komparepart/CMakeLists.txt 09b61e6ca0cdce391fc759be49a672a050cc16cd 
   komparepart/kompare_part.h 24475f1b0ccf7fbeda56860a9a69955cd0b82808 
   komparepart/kompare_part.cpp 4d40be0dedcfb91b77ee239de11188b328f8bc13 
   komparepart/komparelistview.cpp 35bbab849d8b7938cba518e97a00ed50cae35612 
   komparepart/kompareprefdlg.cpp 118485663390e9563a77741b490a9cdf8bf6d464 
   komparepart/komparesaveoptionswidget.cpp 
 4c9acba6a7f9c6dda04130946faac37138422875 
   komparepart/komparesplitter.cpp 8d496bf279caa7cb9a305c2d15131f591c48818d 
   kompareurldialog.h dc50c588e70835ad9292da1baf5222f58f512f67 
   kompareurldialog.cpp 7de050bc44770a79f8f7d789cabd95d6707a40f1 
   libdialogpages/CMakeLists.txt 769a1154c56e8eb8aa42f1bc6d84e0f9a4154fd0 
   komparenavtreepart/komparenavtreepart.cpp 
 3faceff78fbbd2f083cd0a7837c74f50fe543474 
   CMakeLists.txt 38167c2099d0ea1600bd5a6893982e809902fa3a 
   doc/index.docbook 578d12a41d9a6afed441ffd38c39bff16c096ab2 
   interfaces/kompareinterface.h 53b19d944b2a4a65c14ea41b8f1c0997581933db 
   kompare_shell.h 8549fcdc4d1536c58734f2bc3a78b9ebc42c6c5f 
   kompare_shell.cpp dcc45513f3f9f5f94869046989b6b4f5b1c0995e 
   komparenavtreepart/CMakeLists.txt 53e8e670e70629afac9197fc108d844733ec5c07 
 
 Diff: https://git.reviewboard.kde.org/r/120627/diff/
 
 
 Testing
 ---
 
 It builds and runs. The compare dialog ui looks squished though and doesn't 
 resize like it used to, must be something I did wrong when porting away from 
 KVBox
 
 
 Thanks,
 
 Jeremy Whiting
 




Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Kevin Kofler


 On Okt. 17, 2014, 6:03 nachm., Kevin Kofler wrote:
   Use QLayout/QFrame instead of KVBox (seems broken though somehow)
  
  Then I suggest you either fix it, or submit only the parts of the port that 
  work.
  
  We have time until KF6 to port away from kdelibs4support, that's years 
  ahead. (The last use of Qt3Support and kde3support in Kompare was 
  eliminated in November 2011, after Qt 5 was announced, and almost 4 years 
  after KDE 4.0.0.) There is really no need to commit a half-broken port just 
  to eliminate kdelibs4support now.

(and the port away from Qt3Support/kde3support only became really needed now 
that you're porting to KF5, again almost 3 years after I actually did the last 
parts – in total, we would have had almost 7 years of time)


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120627/#review68622
---


On Okt. 17, 2014, 4:17 nachm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120627/
 ---
 
 (Updated Okt. 17, 2014, 4:17 nachm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: kompare
 
 
 Description
 ---
 
 Change KUrl to QUrl.
 Use QLayout/QFrame instead of KVBox (seems broken though somehow)
 Use QFileDialog instead of KFileDialog.
 
 
 Diffs
 -
 
   libdialogpages/filespage.cpp 6a87fe36abd57bdaa09b516de38969db6c6f2298 
   libdialogpages/filessettings.h dc3306e34fe1b4eb7cb6a9d2b598f91932bedda0 
   libdialogpages/filessettings.cpp 0e19dc00f22a2f6e9588bf2d110dbde682888472 
   libdialogpages/pagebase.h 0cef46feaa2cc81deff12c2c5f739e6be6df1b49 
   libdialogpages/pagebase.cpp ba1574aed7124ede49e1c5908a8fe693cf7bc5d3 
   libdialogpages/viewpage.h b5b770d1441650564106e1cc7ef7e587f6ee142d 
   libdialogpages/viewpage.cpp 07bdba5e1edf55a6dcd02e5deef58d30c07660c2 
   libdialogpages/viewsettings.h dbf6afe0d0c70e548e32dfc09391d67ef595cdba 
   libdialogpages/viewsettings.cpp 5a69d0bd9a49f7a3881940c4ea8ad407be56adc1 
   main.cpp 4132c8442f8546ee7d365051dda0e32196249217 
   libdialogpages/filespage.h 42afafcd0fc8bc0a01e32b79d414742937d791fb 
   libdialogpages/dialogpagesexport.h b2de57f6616739d353d4889ef4965ab07f1191aa 
   libdialogpages/diffpage.h 37490b1ebb245e9648530429da63a9240010 
   libdialogpages/diffpage.cpp 7800b486e023cffe41e1fa3e9e60781250ea4199 
   komparepart/CMakeLists.txt 09b61e6ca0cdce391fc759be49a672a050cc16cd 
   komparepart/kompare_part.h 24475f1b0ccf7fbeda56860a9a69955cd0b82808 
   komparepart/kompare_part.cpp 4d40be0dedcfb91b77ee239de11188b328f8bc13 
   komparepart/komparelistview.cpp 35bbab849d8b7938cba518e97a00ed50cae35612 
   komparepart/kompareprefdlg.cpp 118485663390e9563a77741b490a9cdf8bf6d464 
   komparepart/komparesaveoptionswidget.cpp 
 4c9acba6a7f9c6dda04130946faac37138422875 
   komparepart/komparesplitter.cpp 8d496bf279caa7cb9a305c2d15131f591c48818d 
   kompareurldialog.h dc50c588e70835ad9292da1baf5222f58f512f67 
   kompareurldialog.cpp 7de050bc44770a79f8f7d789cabd95d6707a40f1 
   libdialogpages/CMakeLists.txt 769a1154c56e8eb8aa42f1bc6d84e0f9a4154fd0 
   komparenavtreepart/komparenavtreepart.cpp 
 3faceff78fbbd2f083cd0a7837c74f50fe543474 
   CMakeLists.txt 38167c2099d0ea1600bd5a6893982e809902fa3a 
   doc/index.docbook 578d12a41d9a6afed441ffd38c39bff16c096ab2 
   interfaces/kompareinterface.h 53b19d944b2a4a65c14ea41b8f1c0997581933db 
   kompare_shell.h 8549fcdc4d1536c58734f2bc3a78b9ebc42c6c5f 
   kompare_shell.cpp dcc45513f3f9f5f94869046989b6b4f5b1c0995e 
   komparenavtreepart/CMakeLists.txt 53e8e670e70629afac9197fc108d844733ec5c07 
 
 Diff: https://git.reviewboard.kde.org/r/120627/diff/
 
 
 Testing
 ---
 
 It builds and runs. The compare dialog ui looks squished though and doesn't 
 resize like it used to, must be something I did wrong when porting away from 
 KVBox
 
 
 Thanks,
 
 Jeremy Whiting
 




Re: Review Request 120630: Port from KTempDir to QTemporaryDir. Port from KUrl to QUrl.

2014-10-17 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120630/#review68624
---


This one looks OK, but it changes the API, so it needs to go together with a 
matching Kompare change, doesn't it? (One more reason to do the Kompare parts 
in smaller chunks.)

- Kevin Kofler


On Okt. 17, 2014, 4:21 nachm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120630/
 ---
 
 (Updated Okt. 17, 2014, 4:21 nachm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: libkomparediff2
 
 
 Description
 ---
 
 Port from KTempDir to QTemporaryDir.
 
 
 Diffs
 -
 
   kompare.h a8f3c5a71995c8416f9089f3c263f3691c9bfab5 
   kompare.cpp e44b8a3dcf59a43de426b28c39412d4331949412 
 
 Diff: https://git.reviewboard.kde.org/r/120630/diff/
 
 
 Testing
 ---
 
 It builds and an updated kompare frameworks branch builds against it and runs.
 
 
 Thanks,
 
 Jeremy Whiting
 




Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Kevin Kofler
::fromLocalFile(args.at(0)) : 
QUrl(args.at(0))



main.cpp
https://git.reviewboard.kde.org/r/120627/#comment47814

Here too:
QDir::isAbsolutePath(args.at(1)) ? QUrl::fromLocalFile(args.at(1)) : 
QUrl(args.at(1))



main.cpp
https://git.reviewboard.kde.org/r/120627/#comment47815

Here too:
QDir::isAbsolutePath(args.at(0)) ? QUrl::fromLocalFile(args.at(0)) : 
QUrl(args.at(0))



main.cpp
https://git.reviewboard.kde.org/r/120627/#comment47816

Here too:
QDir::isAbsolutePath(args.at(0)) ? QUrl::fromLocalFile(args.at(0)) : 
QUrl(args.at(0))



main.cpp
https://git.reviewboard.kde.org/r/120627/#comment47817

Here too:
QDir::isAbsolutePath(args.at(1)) ? QUrl::fromLocalFile(args.at(1)) : 
QUrl(args.at(1))


- Kevin Kofler


On Okt. 17, 2014, 9:38 nachm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120627/
 ---
 
 (Updated Okt. 17, 2014, 9:38 nachm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: kompare
 
 
 Description
 ---
 
 Change KUrl to QUrl.
 Use QLayout/QFrame instead of KVBox (seems broken though somehow)
 Use QFileDialog instead of KFileDialog.
 
 
 Diffs
 -
 
   libdialogpages/viewpage.cpp 07bdba5e1edf55a6dcd02e5deef58d30c07660c2 
   libdialogpages/viewsettings.h dbf6afe0d0c70e548e32dfc09391d67ef595cdba 
   libdialogpages/viewsettings.cpp 5a69d0bd9a49f7a3881940c4ea8ad407be56adc1 
   main.cpp 4132c8442f8546ee7d365051dda0e32196249217 
   libdialogpages/diffpage.h 37490b1ebb245e9648530429da63a9240010 
   libdialogpages/diffpage.cpp 7800b486e023cffe41e1fa3e9e60781250ea4199 
   libdialogpages/filespage.h 42afafcd0fc8bc0a01e32b79d414742937d791fb 
   libdialogpages/filespage.cpp 6a87fe36abd57bdaa09b516de38969db6c6f2298 
   libdialogpages/filessettings.h dc3306e34fe1b4eb7cb6a9d2b598f91932bedda0 
   libdialogpages/filessettings.cpp 0e19dc00f22a2f6e9588bf2d110dbde682888472 
   libdialogpages/pagebase.h 0cef46feaa2cc81deff12c2c5f739e6be6df1b49 
   libdialogpages/pagebase.cpp ba1574aed7124ede49e1c5908a8fe693cf7bc5d3 
   libdialogpages/viewpage.h b5b770d1441650564106e1cc7ef7e587f6ee142d 
   komparepart/komparesplitter.cpp 8d496bf279caa7cb9a305c2d15131f591c48818d 
   kompareurldialog.h dc50c588e70835ad9292da1baf5222f58f512f67 
   kompareurldialog.cpp 7de050bc44770a79f8f7d789cabd95d6707a40f1 
   libdialogpages/CMakeLists.txt 769a1154c56e8eb8aa42f1bc6d84e0f9a4154fd0 
   libdialogpages/dialogpagesexport.h b2de57f6616739d353d4889ef4965ab07f1191aa 
   komparenavtreepart/komparenavtreepart.cpp 
 3faceff78fbbd2f083cd0a7837c74f50fe543474 
   komparepart/CMakeLists.txt 09b61e6ca0cdce391fc759be49a672a050cc16cd 
   komparepart/kompare_part.h 24475f1b0ccf7fbeda56860a9a69955cd0b82808 
   komparepart/kompare_part.cpp 4d40be0dedcfb91b77ee239de11188b328f8bc13 
   komparepart/komparelistview.cpp 35bbab849d8b7938cba518e97a00ed50cae35612 
   komparepart/kompareprefdlg.cpp 118485663390e9563a77741b490a9cdf8bf6d464 
   komparepart/komparesaveoptionswidget.cpp 
 4c9acba6a7f9c6dda04130946faac37138422875 
   CMakeLists.txt 38167c2099d0ea1600bd5a6893982e809902fa3a 
   doc/index.docbook 578d12a41d9a6afed441ffd38c39bff16c096ab2 
   interfaces/kompareinterface.h 53b19d944b2a4a65c14ea41b8f1c0997581933db 
   kompare_shell.h 8549fcdc4d1536c58734f2bc3a78b9ebc42c6c5f 
   kompare_shell.cpp dcc45513f3f9f5f94869046989b6b4f5b1c0995e 
   komparenavtreepart/CMakeLists.txt 53e8e670e70629afac9197fc108d844733ec5c07 
 
 Diff: https://git.reviewboard.kde.org/r/120627/diff/
 
 
 Testing
 ---
 
 It builds and runs. The compare dialog ui looks squished though and doesn't 
 resize like it used to, must be something I did wrong when porting away from 
 KVBox
 
 
 Thanks,
 
 Jeremy Whiting
 




Re: Porting KUrl::prettyUrl: please do not reintroduce CVE-2013-2074!

2014-10-17 Thread Kevin Kofler
I wrote:
 just a small public service announcement: The correct replacement for:
 url.prettyUrl()
 in Qt 5 is NOT:
 url.toString() // BAD!
 but:
 url.toString(QUrl::RemovePassword)
or, even better:
url.toDisplayString()
as pointed out by Andrea Iacovitti. (I guess his message is pending 
moderation.)

Kevin Kofler



QUrl from a string (porting KUrl constructor or KCmdLineArgs::url)

2014-10-17 Thread Kevin Kofler
Hi,

even after reading the porting guide and listening to David Faure's Akademy 
talk, I am still unsure about what the best way is to get a URL from a 
string, supporting:
* absolute URL (with explicit scheme:// only)
* absolute local file path
* relative path: everything neither an absolute URL nor an absolute local
  file path is assumed to be a relative path (relative to the local CWD)
as both KUrl::KUrl(QString) and KCmdLineArgs::url did (in slightly 
different, but similar ways).

This is needed, e.g., when processing command-line arguments.

I see the following APIs available:
* QUrl::QUrl(QString): accepts an absolute URL or a relative URL, but the
   documentation explicitly warns that it must not be
   used on absolute local file paths
* QUrl::fromLocalFile: accepts an absolute local file path or a relative
   local file path, but does not accept a URL
* QUrl::fromUserInput: seems closest to what I want, but assumes a web
   browser context and in particular assumes http:// in
   situations that should really be treated as a
   relative path in most contexts

So, is there any function anywhere that does what I want? Or do I have to 
write something like:
QDir::isAbsolutePath(str) ? QUrl::fromLocalFile(str) : QUrl(str)
? (And is QUrl(str) even OK for relative file paths or is there a difference 
between a relative URL and a relative local file path that I'm missing?)

Kevin Kofler



Re: Review Request 120630: Port from KTempDir to QTemporaryDir. Port from KUrl to QUrl.

2014-10-17 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120630/#review68635
---

Ship it!


I'm approving this one, but please only push it when the Kompare patch is also 
ready to be pushed.

- Kevin Kofler


On Okt. 17, 2014, 4:21 nachm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120630/
 ---
 
 (Updated Okt. 17, 2014, 4:21 nachm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: libkomparediff2
 
 
 Description
 ---
 
 Port from KTempDir to QTemporaryDir.
 
 
 Diffs
 -
 
   kompare.h a8f3c5a71995c8416f9089f3c263f3691c9bfab5 
   kompare.cpp e44b8a3dcf59a43de426b28c39412d4331949412 
 
 Diff: https://git.reviewboard.kde.org/r/120630/diff/
 
 
 Testing
 ---
 
 It builds and an updated kompare frameworks branch builds against it and runs.
 
 
 Thanks,
 
 Jeremy Whiting
 




Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120627/#review68636
---


Since the KVBox issue should be fixed in revision 2, I'm lifting that 
particular objection. But please address the issues I found while reviewing 
revision 1, which still apply.

For the QUrl from string issue, I also brought it up at: 
http://lists.kde.org/?l=kde-core-develm=141358589725749w=2 – let's see what 
they have to say.

If you aren't using PageBase anymore, shouldn't the class be removed?

- Kevin Kofler


On Okt. 17, 2014, 9:38 nachm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120627/
 ---
 
 (Updated Okt. 17, 2014, 9:38 nachm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: kompare
 
 
 Description
 ---
 
 Change KUrl to QUrl.
 Use QLayout/QFrame instead of KVBox (seems broken though somehow)
 Use QFileDialog instead of KFileDialog.
 
 
 Diffs
 -
 
   libdialogpages/viewpage.cpp 07bdba5e1edf55a6dcd02e5deef58d30c07660c2 
   libdialogpages/viewsettings.h dbf6afe0d0c70e548e32dfc09391d67ef595cdba 
   libdialogpages/viewsettings.cpp 5a69d0bd9a49f7a3881940c4ea8ad407be56adc1 
   main.cpp 4132c8442f8546ee7d365051dda0e32196249217 
   libdialogpages/diffpage.h 37490b1ebb245e9648530429da63a9240010 
   libdialogpages/diffpage.cpp 7800b486e023cffe41e1fa3e9e60781250ea4199 
   libdialogpages/filespage.h 42afafcd0fc8bc0a01e32b79d414742937d791fb 
   libdialogpages/filespage.cpp 6a87fe36abd57bdaa09b516de38969db6c6f2298 
   libdialogpages/filessettings.h dc3306e34fe1b4eb7cb6a9d2b598f91932bedda0 
   libdialogpages/filessettings.cpp 0e19dc00f22a2f6e9588bf2d110dbde682888472 
   libdialogpages/pagebase.h 0cef46feaa2cc81deff12c2c5f739e6be6df1b49 
   libdialogpages/pagebase.cpp ba1574aed7124ede49e1c5908a8fe693cf7bc5d3 
   libdialogpages/viewpage.h b5b770d1441650564106e1cc7ef7e587f6ee142d 
   komparepart/komparesplitter.cpp 8d496bf279caa7cb9a305c2d15131f591c48818d 
   kompareurldialog.h dc50c588e70835ad9292da1baf5222f58f512f67 
   kompareurldialog.cpp 7de050bc44770a79f8f7d789cabd95d6707a40f1 
   libdialogpages/CMakeLists.txt 769a1154c56e8eb8aa42f1bc6d84e0f9a4154fd0 
   libdialogpages/dialogpagesexport.h b2de57f6616739d353d4889ef4965ab07f1191aa 
   komparenavtreepart/komparenavtreepart.cpp 
 3faceff78fbbd2f083cd0a7837c74f50fe543474 
   komparepart/CMakeLists.txt 09b61e6ca0cdce391fc759be49a672a050cc16cd 
   komparepart/kompare_part.h 24475f1b0ccf7fbeda56860a9a69955cd0b82808 
   komparepart/kompare_part.cpp 4d40be0dedcfb91b77ee239de11188b328f8bc13 
   komparepart/komparelistview.cpp 35bbab849d8b7938cba518e97a00ed50cae35612 
   komparepart/kompareprefdlg.cpp 118485663390e9563a77741b490a9cdf8bf6d464 
   komparepart/komparesaveoptionswidget.cpp 
 4c9acba6a7f9c6dda04130946faac37138422875 
   CMakeLists.txt 38167c2099d0ea1600bd5a6893982e809902fa3a 
   doc/index.docbook 578d12a41d9a6afed441ffd38c39bff16c096ab2 
   interfaces/kompareinterface.h 53b19d944b2a4a65c14ea41b8f1c0997581933db 
   kompare_shell.h 8549fcdc4d1536c58734f2bc3a78b9ebc42c6c5f 
   kompare_shell.cpp dcc45513f3f9f5f94869046989b6b4f5b1c0995e 
   komparenavtreepart/CMakeLists.txt 53e8e670e70629afac9197fc108d844733ec5c07 
 
 Diff: https://git.reviewboard.kde.org/r/120627/diff/
 
 
 Testing
 ---
 
 It builds and runs. The compare dialog ui looks squished though and doesn't 
 resize like it used to, must be something I did wrong when porting away from 
 KVBox
 
 
 Thanks,
 
 Jeremy Whiting
 




Re: QUrl from a string (porting KUrl constructor or KCmdLineArgs::url)

2014-10-17 Thread Kevin Kofler
Hi,

I wrote:
 even after reading the porting guide and listening to David Faure's
 Akademy talk, I am still unsure about what the best way is to get a URL
 from a string, supporting:
 * absolute URL (with explicit scheme:// only)
 * absolute local file path
 * relative path: everything neither an absolute URL nor an absolute local
   file path is assumed to be a relative path (relative to the local CWD)
 as both KUrl::KUrl(QString) and KCmdLineArgs::url did (in slightly
 different, but similar ways).
 
 This is needed, e.g., when processing command-line arguments.
 
 I see the following APIs available:
 * QUrl::QUrl(QString): accepts an absolute URL or a relative URL, but the
documentation explicitly warns that it must not be
used on absolute local file paths
 * QUrl::fromLocalFile: accepts an absolute local file path or a relative
local file path, but does not accept a URL
 * QUrl::fromUserInput: seems closest to what I want, but assumes a web
browser context and in particular assumes http://
in situations that should really be treated as a
relative path in most contexts
 
 So, is there any function anywhere that does what I want? Or do I have to
 write something like:
 QDir::isAbsolutePath(str) ? QUrl::fromLocalFile(str) : QUrl(str)
 ? (And is QUrl(str) even OK for relative file paths or is there a
 difference between a relative URL and a relative local file path that I'm
 missing?)

So, Lukáš Tinkl answered me on #fedora-kde IRC. For posterity:
* Qt 5.4 introduces a new overload:
  http://doc-snapshot.qt-project.org/qt5-5.4/qurl.html#fromUserInput-2
  with a third parameter that solves this issue.
* This is how Okular solves the problem without hard-depending on Qt 5.4:
  
http://quickgit.kde.org/?p=okular.gita=commith=d98b4d920037422fe052ffa2633349d41fdbe02e

Kevin Kofler



Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Kevin Kofler


 On Okt. 17, 2014, 10:12 nachm., Kevin Kofler wrote:
  main.cpp, line 132
  https://git.reviewboard.kde.org/r/120627/diff/1/?file=320275#file320275line132
 
  I think this now needs something like:
  QDir::isAbsolutePath(args.at(0)) ? QUrl::fromLocalFile(args.at(0)) : 
  QUrl(args.at(0))
  
  QUrl really needs a static method for that, neither the constructor, 
  nor fromLocalFile, nor fromUserInput do the right thing in this context. 
  I'm going to ask on kde-core-devel if I'm missing something.
  
  This shows up so often in this function that it might be worth putting 
  it into a helper function.
 
 Kevin Kofler wrote:
 So, we should definitely put whatever we use in a wrapper function, 
 because:
 1. That way, it's easy to fix whatever we wrote if we run into some 
 corner case where it doesn't work.
 2. From Qt 5.4 onwards, one can write:
QUrl::fromUserInput(arg, QDir::currentPath(), QUrl::AssumeLocalFile)
(see 
 http://doc-snapshot.qt-project.org/qt5-5.4/qurl.html#fromUserInput-2)
which should do exactly what we want. (Unfortunately, that overload is 
 new in 5.4. Thanks to Lukáš Tinkl for pointing me to it.)
 
 The exact code Qt 5.4 uses is:
 
 https://qt.gitorious.org/qt/qtbase/commit/31ce6f50c679e61dc53f09ee1b1637918da38a82
 If we're writing a function anyway, we may as well use that logic, as in:
 ```c++
 QUrl urlFromArg(const QString arg)
 {
 #if QT_VERSION = 0x050400
 return QUrl::fromUserInput(arg, QDir::currentPath(), 
 QUrl::AssumeLocalFile);
 #else
 // Logic from QUrl::fromUserInput(QString, QString, 
 UserInputResolutionOptions)
 QUrl testUrl = QUrl(arg, QUrl::TolerantMode);
 if (testUrl.isRelative()  !QDir::isAbsolutePath(arg)) {
 QFileInfo fileInfo(QDir::current(), arg);
 return QUrl::fromLocalFile(fileInfo.absoluteFilePath());
 }
 return QUrl::fromUserInput(arg);
 #endif
 }
 ```
 (Okular is doing something similar, as pointed out by Lukáš.)
 
 Please note that I definitely want AssumeLocalFile, even where we don't 
 need to accept files that don't exist, because I don't want Kompare to do a 
 hostname lookup if it's passed a nonexisting file, I think it doesn't make 
 any sense to assume a URL if no http:// is given.

Actually, we don't need the QFileInfo in this case. (Qt 5.4's QUrl uses it 
because it also checks existence.) So the function simplifies to:
```c++
QUrl urlFromArg(const QString arg)
{
#if QT_VERSION = 0x050400
return QUrl::fromUserInput(arg, QDir::currentPath(), QUrl::AssumeLocalFile);
#else
// Logic from QUrl::fromUserInput(QString, QString, 
UserInputResolutionOptions)
return (QUrl(arg, QUrl::TolerantMode).isRelative()  
!QDir::isAbsolutePath(arg))
   ? QUrl::fromLocalFile(QDir::current().absoluteFilePath(arg))
   : QUrl::fromUserInput(arg);
#endif
}
```


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120627/#review68628
---


On Okt. 17, 2014, 9:38 nachm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120627/
 ---
 
 (Updated Okt. 17, 2014, 9:38 nachm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: kompare
 
 
 Description
 ---
 
 Change KUrl to QUrl.
 Use QLayout/QFrame instead of KVBox (seems broken though somehow)
 Use QFileDialog instead of KFileDialog.
 
 
 Diffs
 -
 
   libdialogpages/viewpage.cpp 07bdba5e1edf55a6dcd02e5deef58d30c07660c2 
   libdialogpages/viewsettings.h dbf6afe0d0c70e548e32dfc09391d67ef595cdba 
   libdialogpages/viewsettings.cpp 5a69d0bd9a49f7a3881940c4ea8ad407be56adc1 
   main.cpp 4132c8442f8546ee7d365051dda0e32196249217 
   libdialogpages/diffpage.h 37490b1ebb245e9648530429da63a9240010 
   libdialogpages/diffpage.cpp 7800b486e023cffe41e1fa3e9e60781250ea4199 
   libdialogpages/filespage.h 42afafcd0fc8bc0a01e32b79d414742937d791fb 
   libdialogpages/filespage.cpp 6a87fe36abd57bdaa09b516de38969db6c6f2298 
   libdialogpages/filessettings.h dc3306e34fe1b4eb7cb6a9d2b598f91932bedda0 
   libdialogpages/filessettings.cpp 0e19dc00f22a2f6e9588bf2d110dbde682888472 
   libdialogpages/pagebase.h 0cef46feaa2cc81deff12c2c5f739e6be6df1b49 
   libdialogpages/pagebase.cpp ba1574aed7124ede49e1c5908a8fe693cf7bc5d3 
   libdialogpages/viewpage.h b5b770d1441650564106e1cc7ef7e587f6ee142d 
   komparepart/komparesplitter.cpp 8d496bf279caa7cb9a305c2d15131f591c48818d 
   kompareurldialog.h dc50c588e70835ad9292da1baf5222f58f512f67 
   kompareurldialog.cpp 7de050bc44770a79f8f7d789cabd95d6707a40f1 
   libdialogpages/CMakeLists.txt

Re: QUrl from a string (porting KUrl constructor or KCmdLineArgs::url)

2014-10-17 Thread Kevin Kofler
PS:

I wrote:
 So, Lukáš Tinkl answered me on #fedora-kde IRC. For posterity:
 * Qt 5.4 introduces a new overload:
   http://doc-snapshot.qt-project.org/qt5-5.4/qurl.html#fromUserInput-2
   with a third parameter that solves this issue.
 * This is how Okular solves the problem without hard-depending on Qt 5.4:
   
 http://quickgit.kde.org/?p=okular.gita=commith=d98b4d920037422fe052ffa2633349d41fdbe02e

Sorry for yet another self-reply, but I'll point out that Okular currently
uses the two-argument form, which according to the documentation is enough
when only existing files need to be supported, but I definitely recommend
passing the third QUrl::AssumeLocalFile argument. The reason is that I don't
want Kompare (or most other applications that accept either a file or a URL)
to do a hostname lookup if it's passed a nonexisting file. I think it
doesn't make any sense to assume a URL if no http:// is given. It sends
mistyped file names out as DNS lookups, which can even be argued to be a
security issue.

Thus, the Kompare code will probably look like this:
QUrl urlFromArg(const QString arg)
{
#if QT_VERSION = 0x050400
return QUrl::fromUserInput(arg, QDir::currentPath(), QUrl::AssumeLocalFile);
#else
// Logic from QUrl::fromUserInput(QString, QString, 
UserInputResolutionOptions)
return (QUrl(arg, QUrl::TolerantMode).isRelative()  
!QDir::isAbsolutePath(arg))
   ? QUrl::fromLocalFile(QDir::current().absoluteFilePath(arg))
   : QUrl::fromUserInput(arg);
#endif
}

Kevin Kofler



Re: Review Request 120627: Remove kdelibs4support.

2014-10-17 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120627/#review68642
---


 I wonder about the Qt 5.4 check though. This version of kompare using qt5 
 shouldn't be released until after we depend on Qt 5.4 imo,
 but I guess distros could use it with Qt 5.3 if they want.

Well, there are only 2 possibilities: Either you require 5.4 right now in 
CMakeLists.txt, or you use the #ifdefs until 5.4 becomes required. I'm for the 
latter (i.e. keep the #ifdefs for now), because Qt 5.4 isn't even released yet.


komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120627/#comment47847

tempFileName = tempFile.fileName(); missing somewhere between here and line 
299 (and then you can use the variable in line 299). tempFileName is used 
further down in this function.



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120627/#comment47845

So where does this temporary file get deleted? Apparently nowhere.

You have to handle this the same way as the QTemporaryDir, by allocating 
tempFile with new, setting autoRemove to true rather than false, storing 
tempFile in m_info, and deleting it in cleanUpTemporaryFiles.

(This is what the KIO::NetAccess::removeTempFile calls you removed from 
cleanUpTemporaryFiles were for, but those obviously wouldn't work anymore 
anyway if the files are not downloaded through KIO::NetAccess anymore.)



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120627/#comment47846

Just to track the issue you found yourself:
 One issue with kio::copy_file exists still where it can't rename the 
QTemporaryFile in my /tmp folder since it's owned by root. I'll look into that 
in kio itself.


- Kevin Kofler


On Okt. 18, 2014, 3:40 vorm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120627/
 ---
 
 (Updated Okt. 18, 2014, 3:40 vorm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: kompare
 
 
 Description
 ---
 
 Change KUrl to QUrl.
 Use QLayout/QFrame instead of KVBox (seems broken though somehow)
 Use QFileDialog instead of KFileDialog.
 
 
 Diffs
 -
 
   libdialogpages/viewpage.cpp 07bdba5e1edf55a6dcd02e5deef58d30c07660c2 
   libdialogpages/viewsettings.h dbf6afe0d0c70e548e32dfc09391d67ef595cdba 
   libdialogpages/viewsettings.cpp 5a69d0bd9a49f7a3881940c4ea8ad407be56adc1 
   main.cpp 4132c8442f8546ee7d365051dda0e32196249217 
   libdialogpages/pagebase.h 0cef46feaa2cc81deff12c2c5f739e6be6df1b49 
   libdialogpages/pagebase.cpp ba1574aed7124ede49e1c5908a8fe693cf7bc5d3 
   libdialogpages/viewpage.h b5b770d1441650564106e1cc7ef7e587f6ee142d 
   libdialogpages/diffpage.h 37490b1ebb245e9648530429da63a9240010 
   libdialogpages/diffpage.cpp 7800b486e023cffe41e1fa3e9e60781250ea4199 
   libdialogpages/filespage.h 42afafcd0fc8bc0a01e32b79d414742937d791fb 
   libdialogpages/filespage.cpp 6a87fe36abd57bdaa09b516de38969db6c6f2298 
   libdialogpages/filessettings.h dc3306e34fe1b4eb7cb6a9d2b598f91932bedda0 
   libdialogpages/filessettings.cpp 0e19dc00f22a2f6e9588bf2d110dbde682888472 
   komparepart/komparesaveoptionswidget.cpp 
 4c9acba6a7f9c6dda04130946faac37138422875 
   komparepart/komparesplitter.cpp 8d496bf279caa7cb9a305c2d15131f591c48818d 
   kompareurldialog.h dc50c588e70835ad9292da1baf5222f58f512f67 
   kompareurldialog.cpp 7de050bc44770a79f8f7d789cabd95d6707a40f1 
   libdialogpages/CMakeLists.txt 769a1154c56e8eb8aa42f1bc6d84e0f9a4154fd0 
   libdialogpages/dialogpagesexport.h b2de57f6616739d353d4889ef4965ab07f1191aa 
   komparepart/kompare_part.cpp 4d40be0dedcfb91b77ee239de11188b328f8bc13 
   komparepart/komparelistview.cpp 35bbab849d8b7938cba518e97a00ed50cae35612 
   komparepart/kompareprefdlg.cpp 118485663390e9563a77741b490a9cdf8bf6d464 
   kompare_shell.cpp dcc45513f3f9f5f94869046989b6b4f5b1c0995e 
   komparenavtreepart/CMakeLists.txt 53e8e670e70629afac9197fc108d844733ec5c07 
   komparenavtreepart/komparenavtreepart.cpp 
 3faceff78fbbd2f083cd0a7837c74f50fe543474 
   komparepart/CMakeLists.txt 09b61e6ca0cdce391fc759be49a672a050cc16cd 
   komparepart/kompare_part.h 24475f1b0ccf7fbeda56860a9a69955cd0b82808 
   CMakeLists.txt 38167c2099d0ea1600bd5a6893982e809902fa3a 
   doc/index.docbook 578d12a41d9a6afed441ffd38c39bff16c096ab2 
   interfaces/kompareinterface.h 53b19d944b2a4a65c14ea41b8f1c0997581933db 
   kompare_shell.h 8549fcdc4d1536c58734f2bc3a78b9ebc42c6c5f 
 
 Diff: https://git.reviewboard.kde.org/r/120627/diff/
 
 
 Testing
 ---
 
 It builds and runs. The compare dialog ui looks squished though and doesn't 
 resize like it used to, must be something I did wrong when porting away from 
 KVBox
 
 
 Thanks,
 
 Jeremy Whiting
 




Re: Using Gerrit for code review in KDE

2014-10-16 Thread Kevin Kofler
Jan Kundrát wrote:
 A random data point -- I asked a 3rd-party contributor to send a patch to
 Trojita through Gerrit earlier today. He accomplished that goal so fast
 that I asked him for an estimate on how much time it took. The answer was
 15 minutes, including reading the docs and setting up the client-side
 hooks. Quite frankly, I don't think I was faster when I first used
 ReviewBoard.

Strange, because that's not at all the experience I had submitting patches 
to Qt/Gerrit compared to KDE/ReviewBoard.

In ReviewBoard, I export my patch from the git-cola menu and I submit it 
through a nice web interface that lets me input all the details (target 
branch, reviewers, etc.). I don't need to fire up a Konsole at all, nor read 
any documentation. It just works.

In Gerrit, I basically get an ugly command-line interface: I have to push to 
a magic ref encoding all the information (and IIRC, git-cola only lets me 
enter the basic refs/for/branchname, the special characters in stuff like 
%r=f...@example.com confuse it, so I'd have to push from a terminal if I want 
to use those). Setting reviewers requires a special command-line-style 
parameter appended to the ref that is found in the documentation (that %r= 
thing). There is also no autocompletion nor client-side validation of the 
reviewer nicks/addresses, unlike on ReviewBoard's friendly web interface.

And the next time I want to submit something to Gerrit, I'm sure I'll have 
to reread the documentation all over again, whereas ReviewBoard is dead 
simple.

I can see you liking Gerrit if you're used to juggling with obscure git 
command lines, but as a long-term user of Cervisia, kdesvn and now git-cola, 
I find a web submission interface much nicer to work with.

Kevin Kofler



Re: Review Request 120554: Initial frameworks port of kompare

2014-10-16 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120554/#review68582
---


See my point by point review below. Once all the issues are addressed, we 
should be all set, but I'll have another look before giving it a Ship It.


CMakeLists.txt
https://git.reviewboard.kde.org/r/120554/#comment47766

No app icon anymore on the proprietary system(s) using those?

Looks like the discussion is still ongoing on what to do with that: 
http://lists.kde.org/?t=14078460994r=1w=2 so I guess we'll have to wait 
for its outcome. :-( (Thus, no issue raised here.)



komparenavtreepart/komparenavtreepart.cpp
https://git.reviewboard.kde.org/r/120554/#comment47768

Please just delete this obsolete macro entirely.



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120554/#comment47769

This should pass at least QUrl::RemoveUserInfo to toString(), we don't want 
to echo passwords in error messages.



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120554/#comment47770

See above.



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120554/#comment47771

See above.



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120554/#comment47772

See above. (In this case, you'll also need to add the explicit 
.toString(QUrl::RemoveUserInfo) call.)



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120554/#comment47773

See above.



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120554/#comment47774

See above.



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120554/#comment47775

See above.



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120554/#comment47776

See above.



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120554/#comment4

See above.



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120554/#comment47778

See above.



komparepart/kompare_part.cpp
https://git.reviewboard.kde.org/r/120554/#comment47779

See above.



komparepart/kompareprefdlg.cpp
https://git.reviewboard.kde.org/r/120554/#comment47780

If OK is automatically the default button, just delete this line.

If it is not, then please replace it with something that does the job, e.g.:
button( QDialogButtonBox::Ok )-setDefault( true );



komparepart/kompareprefdlg.cpp
https://git.reviewboard.kde.org/r/120554/#comment47781

There goes the button separator.

If button separators are generally unwanted now (as seems the case, judging 
from the porting script that just deletes the showButtonSeparator calls), we 
should just remove the line entirely rather than commenting it out. If they're 
wanted, we should find a way to show them.

Looking at the dialog in my KDE 4 Kompare, I think the separator is not 
really needed to begin with, so you can just delete the commented-out line.



kompareurldialog.cpp
https://git.reviewboard.kde.org/r/120554/#comment47767

There goes the button separator.

If button separators are generally unwanted now (as seems the case, judging 
from the porting script that just deletes the showButtonSeparator calls), we 
should just remove the line entirely rather than commenting it out. If they're 
wanted, we should find a way to show them.

Looking at the dialog in my KDE 4 Kompare, I think the separator is not 
really needed to begin with, so you can just delete the commented-out line.



libdialogpages/diffpage.cpp
https://git.reviewboard.kde.org/r/120554/#comment47783

This one ought to be toLocalFile, I think. Or at least pass 
QUrl::PreferLocalFile to toString(), but I don't think a URL makes sense as a 
diff program, does it?


- Kevin Kofler


On Okt. 16, 2014, 5:24 vorm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120554/
 ---
 
 (Updated Okt. 16, 2014, 5:24 vorm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: kompare
 
 
 Description
 ---
 
 I spent a bit of time porting kompare to kf5. It builds and runs and compares 
 files and folders but I'm pretty sure I missed something. 
 
 I'll reread my changes also but wanted to get this out there to be played 
 with also.
 
 
 Diffs
 -
 
   libdialogpages/viewpage.cpp e26d72843587cf4ed60f0d7dcde51ec4f19aad29 
   libdialogpages/viewsettings.h 305e934815175c0fc60c8a045070e48f7b932935 
   main.cpp ac68e2421c1f02460e732eb4dc7f79036df9db2e 
   pics/CMakeLists.txt 512f6e8cad202bc592c08531654754bd311fcb5e 
   pics/hi128-app-kompare.png  
   pics/hi16-app-kompare.png  
   pics/hi22-app-kompare.png  
   pics/hi32-app-kompare.png  
   pics/hi48

Re: Review Request 120554: Initial frameworks port of kompare

2014-10-16 Thread Kevin Kofler


 On Okt. 16, 2014, 10:47 nachm., Kevin Kofler wrote:
  komparepart/kompare_part.cpp, line 295
  https://git.reviewboard.kde.org/r/120554/diff/4/?file=318659#file318659line295
 
  This should pass at least QUrl::RemoveUserInfo to toString(), we don't 
  want to echo passwords in error messages.

(Somebody found this serious enough an issue to file 
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-2074 against KHTML, 
where the fix back then was to use prettyUrl. I hope there aren't more such 
issues introduced by this sloppy prettyUrl KF5 porting.)


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120554/#review68582
---


On Okt. 16, 2014, 5:24 vorm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120554/
 ---
 
 (Updated Okt. 16, 2014, 5:24 vorm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: kompare
 
 
 Description
 ---
 
 I spent a bit of time porting kompare to kf5. It builds and runs and compares 
 files and folders but I'm pretty sure I missed something. 
 
 I'll reread my changes also but wanted to get this out there to be played 
 with also.
 
 
 Diffs
 -
 
   libdialogpages/viewpage.cpp e26d72843587cf4ed60f0d7dcde51ec4f19aad29 
   libdialogpages/viewsettings.h 305e934815175c0fc60c8a045070e48f7b932935 
   main.cpp ac68e2421c1f02460e732eb4dc7f79036df9db2e 
   pics/CMakeLists.txt 512f6e8cad202bc592c08531654754bd311fcb5e 
   pics/hi128-app-kompare.png  
   pics/hi16-app-kompare.png  
   pics/hi22-app-kompare.png  
   pics/hi32-app-kompare.png  
   pics/hi48-app-kompare.png  
   pics/hisc-app-kompare.svgz  
   libdialogpages/pagebase.cpp 4aa33d7d5b8eb6779bb96e5533d0f11235c30aac 
   libdialogpages/filespage.cpp 417fbd12b0f7622da23d0da0e934476d142df149 
   libdialogpages/CMakeLists.txt 22906650d1f0f8fb0b5d8d3d272f09d44bf7408c 
   libdialogpages/diffpage.cpp 94221ca8badbd1773ff48071fd558bd111750e47 
   komparepart/komparesaveoptionswidget.cpp 
 06530d85159305fc1330f495a1c52b0155e45e37 
   komparepart/komparesplitter.h 11a344f29f46d68ca5418c770bd5e502d527e0fe 
   komparepart/komparesplitter.cpp 2848f881992bae0b0e7141c1f6c47a2239211844 
   komparepart/kompareview.h 93ea0644a590c56e600e466a69bf227dc93328b1 
   kompareurldialog.cpp 561dd4518dda0be64198beff56e986da4294fe2b 
   komparenavtreepart/CMakeLists.txt da52bc7d0d9f032d80f6f2257dbbed1f6fb0e81a 
   komparenavtreepart/komparenavtreepart.h 
 eb08329be477febe93b4ca7a8c787656abbfc68f 
   komparenavtreepart/komparenavtreepart.cpp 
 d3bdc93ddaf28e026b7c1847b8d4f6dbc46125ee 
   komparepart/CMakeLists.txt ee83458a3034c3fb873629d650efe5668955900b 
   komparepart/kompare_part.h 0c4d3dd40ca32e07b2402280539d03f15cfc 
   komparepart/kompare_part.cpp 08df1dc0985391908eb81da9c4cfdd0836cd4b23 
   komparepart/kompareconnectwidget.h 03eb746c24dc3899b64d3907ae21e0de656e369f 
   komparepart/kompareconnectwidget.cpp 
 2a8cb920280f2b42ab09e7962a441529b8cdfc0c 
   komparepart/komparelistview.cpp b2935c917541984532814d301b6a7f5bdd661c72 
   komparepart/kompareprefdlg.cpp 0b18696acf270cf5a0351312aa3ffe13eff9b9e6 
   komparepart/komparesaveoptionswidget.h 
 9c49815b1b95b9448eb5fccda35e4c7c7fb1e2f1 
   kompare_shell.h de099ffbcc92a22a4374ad6cfca0bccc6b0e97bc 
   kompare_shell.cpp 9d22085780fbbffcb9b480cbb16c30e73c0ba71e 
   CMakeLists.txt 86e4504ad3ae06519cbfaaf35781238f5f234857 
   doc/CMakeLists.txt 06d898738aabdfc947e89de848e2fbe903d5e6cc 
   interfaces/CMakeLists.txt 4bb0c6c53e8b995f1c7350cd02268e2e05ddb38a 
   interfaces/kompareinterface.h a28d209b058fb06cc970e6ba3538ace721319be5 
 
 Diff: https://git.reviewboard.kde.org/r/120554/diff/
 
 
 Testing
 ---
 
 It builds, runs and seems to wok ok comparing files and folders. The 
 QFileDialog it uses wasn't showing files I expected to see though, may need 
 to play with the filters etc.
 
 Also ported from KGlobal::ref() and KGlobal::unref() to QEventLoopLocker, 
 though quitting one window closes all windows, not sure if that's expected or 
 not.
 
 
 Thanks,
 
 Jeremy Whiting
 




Re: Review Request 120554: Initial frameworks port of kompare

2014-10-16 Thread Kevin Kofler


 On Okt. 16, 2014, 10:47 nachm., Kevin Kofler wrote:
  komparepart/kompare_part.cpp, line 295
  https://git.reviewboard.kde.org/r/120554/diff/4/?file=318659#file318659line295
 
  This should pass at least QUrl::RemoveUserInfo to toString(), we don't 
  want to echo passwords in error messages.
 
 Kevin Kofler wrote:
 (Somebody found this serious enough an issue to file 
 http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-2074 against KHTML, 
 where the fix back then was to use prettyUrl. I hope there aren't more such 
 issues introduced by this sloppy prettyUrl KF5 porting.)

Actually, rather than QUrl::RemoveUserInfo, QUrl::RemovePassword is enough, you 
don't have to (and probably shouldn't) strip the user name. But you definitely 
don't want to output the password.


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120554/#review68582
---


On Okt. 16, 2014, 5:24 vorm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120554/
 ---
 
 (Updated Okt. 16, 2014, 5:24 vorm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: kompare
 
 
 Description
 ---
 
 I spent a bit of time porting kompare to kf5. It builds and runs and compares 
 files and folders but I'm pretty sure I missed something. 
 
 I'll reread my changes also but wanted to get this out there to be played 
 with also.
 
 
 Diffs
 -
 
   libdialogpages/viewpage.cpp e26d72843587cf4ed60f0d7dcde51ec4f19aad29 
   libdialogpages/viewsettings.h 305e934815175c0fc60c8a045070e48f7b932935 
   main.cpp ac68e2421c1f02460e732eb4dc7f79036df9db2e 
   pics/CMakeLists.txt 512f6e8cad202bc592c08531654754bd311fcb5e 
   pics/hi128-app-kompare.png  
   pics/hi16-app-kompare.png  
   pics/hi22-app-kompare.png  
   pics/hi32-app-kompare.png  
   pics/hi48-app-kompare.png  
   pics/hisc-app-kompare.svgz  
   libdialogpages/pagebase.cpp 4aa33d7d5b8eb6779bb96e5533d0f11235c30aac 
   libdialogpages/filespage.cpp 417fbd12b0f7622da23d0da0e934476d142df149 
   libdialogpages/CMakeLists.txt 22906650d1f0f8fb0b5d8d3d272f09d44bf7408c 
   libdialogpages/diffpage.cpp 94221ca8badbd1773ff48071fd558bd111750e47 
   komparepart/komparesaveoptionswidget.cpp 
 06530d85159305fc1330f495a1c52b0155e45e37 
   komparepart/komparesplitter.h 11a344f29f46d68ca5418c770bd5e502d527e0fe 
   komparepart/komparesplitter.cpp 2848f881992bae0b0e7141c1f6c47a2239211844 
   komparepart/kompareview.h 93ea0644a590c56e600e466a69bf227dc93328b1 
   kompareurldialog.cpp 561dd4518dda0be64198beff56e986da4294fe2b 
   komparenavtreepart/CMakeLists.txt da52bc7d0d9f032d80f6f2257dbbed1f6fb0e81a 
   komparenavtreepart/komparenavtreepart.h 
 eb08329be477febe93b4ca7a8c787656abbfc68f 
   komparenavtreepart/komparenavtreepart.cpp 
 d3bdc93ddaf28e026b7c1847b8d4f6dbc46125ee 
   komparepart/CMakeLists.txt ee83458a3034c3fb873629d650efe5668955900b 
   komparepart/kompare_part.h 0c4d3dd40ca32e07b2402280539d03f15cfc 
   komparepart/kompare_part.cpp 08df1dc0985391908eb81da9c4cfdd0836cd4b23 
   komparepart/kompareconnectwidget.h 03eb746c24dc3899b64d3907ae21e0de656e369f 
   komparepart/kompareconnectwidget.cpp 
 2a8cb920280f2b42ab09e7962a441529b8cdfc0c 
   komparepart/komparelistview.cpp b2935c917541984532814d301b6a7f5bdd661c72 
   komparepart/kompareprefdlg.cpp 0b18696acf270cf5a0351312aa3ffe13eff9b9e6 
   komparepart/komparesaveoptionswidget.h 
 9c49815b1b95b9448eb5fccda35e4c7c7fb1e2f1 
   kompare_shell.h de099ffbcc92a22a4374ad6cfca0bccc6b0e97bc 
   kompare_shell.cpp 9d22085780fbbffcb9b480cbb16c30e73c0ba71e 
   CMakeLists.txt 86e4504ad3ae06519cbfaaf35781238f5f234857 
   doc/CMakeLists.txt 06d898738aabdfc947e89de848e2fbe903d5e6cc 
   interfaces/CMakeLists.txt 4bb0c6c53e8b995f1c7350cd02268e2e05ddb38a 
   interfaces/kompareinterface.h a28d209b058fb06cc970e6ba3538ace721319be5 
 
 Diff: https://git.reviewboard.kde.org/r/120554/diff/
 
 
 Testing
 ---
 
 It builds, runs and seems to wok ok comparing files and folders. The 
 QFileDialog it uses wasn't showing files I expected to see though, may need 
 to play with the filters etc.
 
 Also ported from KGlobal::ref() and KGlobal::unref() to QEventLoopLocker, 
 though quitting one window closes all windows, not sure if that's expected or 
 not.
 
 
 Thanks,
 
 Jeremy Whiting
 




Porting KUrl::prettyUrl: please do not reintroduce CVE-2013-2074!

2014-10-16 Thread Kevin Kofler
Hi,

just a small public service announcement: The correct replacement for:
url.prettyUrl()
in Qt 5 is NOT:
url.toString() // BAD!
but:
url.toString(QUrl::RemovePassword)

The old KUrl::prettyUrl() always removed passwords. You DON'T want to show 
passwords in user output:
http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-2074

(I found this reviewing the initial port of Kompare.)

Thanks for reading,
Kevin Kofler



Re: Review Request 120554: Initial frameworks port of kompare

2014-10-16 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120554/#review68587
---


Oh, I missed this one:
 The QFileDialog it uses wasn't showing files I expected to see though, may 
 need to play with the filters etc.


kompare_shell.cpp
https://git.reviewboard.kde.org/r/120554/#comment47790

You apparently can't pass a MIME type directly to the static QFileDialog 
functions. Use QMimeDatabase().mimeTypeForName(text/x-patch).filterString() 
instead.

(The QFileDialog::setMimeTypeFilters convenience method appears not to be 
exported through the static API.)


- Kevin Kofler


On Okt. 17, 2014, 1:45 vorm., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120554/
 ---
 
 (Updated Okt. 17, 2014, 1:45 vorm.)
 
 
 Review request for kdelibs and Kevin Kofler.
 
 
 Repository: kompare
 
 
 Description
 ---
 
 I spent a bit of time porting kompare to kf5. It builds and runs and compares 
 files and folders but I'm pretty sure I missed something. 
 
 I'll reread my changes also but wanted to get this out there to be played 
 with also.
 
 
 Diffs
 -
 
   CMakeLists.txt 86e4504ad3ae06519cbfaaf35781238f5f234857 
   doc/CMakeLists.txt 06d898738aabdfc947e89de848e2fbe903d5e6cc 
   komparepart/komparesplitter.cpp 2848f881992bae0b0e7141c1f6c47a2239211844 
   komparepart/kompareview.h 93ea0644a590c56e600e466a69bf227dc93328b1 
   kompareurldialog.cpp 561dd4518dda0be64198beff56e986da4294fe2b 
   libdialogpages/CMakeLists.txt 22906650d1f0f8fb0b5d8d3d272f09d44bf7408c 
   libdialogpages/diffpage.cpp 94221ca8badbd1773ff48071fd558bd111750e47 
   libdialogpages/filespage.cpp 417fbd12b0f7622da23d0da0e934476d142df149 
   libdialogpages/pagebase.cpp 4aa33d7d5b8eb6779bb96e5533d0f11235c30aac 
   libdialogpages/viewpage.cpp e26d72843587cf4ed60f0d7dcde51ec4f19aad29 
   libdialogpages/viewsettings.h 305e934815175c0fc60c8a045070e48f7b932935 
   main.cpp ac68e2421c1f02460e732eb4dc7f79036df9db2e 
   pics/CMakeLists.txt 512f6e8cad202bc592c08531654754bd311fcb5e 
   pics/hi128-app-kompare.png  
   pics/hi16-app-kompare.png  
   pics/hi22-app-kompare.png  
   pics/hi32-app-kompare.png  
   pics/hi48-app-kompare.png  
   pics/hisc-app-kompare.svgz  
   interfaces/CMakeLists.txt 4bb0c6c53e8b995f1c7350cd02268e2e05ddb38a 
   interfaces/kompareinterface.h a28d209b058fb06cc970e6ba3538ace721319be5 
   kompare_shell.h de099ffbcc92a22a4374ad6cfca0bccc6b0e97bc 
   kompare_shell.cpp 9d22085780fbbffcb9b480cbb16c30e73c0ba71e 
   komparenavtreepart/CMakeLists.txt da52bc7d0d9f032d80f6f2257dbbed1f6fb0e81a 
   komparenavtreepart/komparenavtreepart.h 
 eb08329be477febe93b4ca7a8c787656abbfc68f 
   komparenavtreepart/komparenavtreepart.cpp 
 d3bdc93ddaf28e026b7c1847b8d4f6dbc46125ee 
   komparepart/CMakeLists.txt ee83458a3034c3fb873629d650efe5668955900b 
   komparepart/kompare_part.h 0c4d3dd40ca32e07b2402280539d03f15cfc 
   komparepart/kompare_part.cpp 08df1dc0985391908eb81da9c4cfdd0836cd4b23 
   komparepart/kompareconnectwidget.h 03eb746c24dc3899b64d3907ae21e0de656e369f 
   komparepart/kompareconnectwidget.cpp 
 2a8cb920280f2b42ab09e7962a441529b8cdfc0c 
   komparepart/komparelistview.cpp b2935c917541984532814d301b6a7f5bdd661c72 
   komparepart/kompareprefdlg.cpp 0b18696acf270cf5a0351312aa3ffe13eff9b9e6 
   komparepart/komparesaveoptionswidget.h 
 9c49815b1b95b9448eb5fccda35e4c7c7fb1e2f1 
   komparepart/komparesaveoptionswidget.cpp 
 06530d85159305fc1330f495a1c52b0155e45e37 
   komparepart/komparesplitter.h 11a344f29f46d68ca5418c770bd5e502d527e0fe 
 
 Diff: https://git.reviewboard.kde.org/r/120554/diff/
 
 
 Testing
 ---
 
 It builds, runs and seems to wok ok comparing files and folders. The 
 QFileDialog it uses wasn't showing files I expected to see though, may need 
 to play with the filters etc.
 
 Also ported from KGlobal::ref() and KGlobal::unref() to QEventLoopLocker, 
 though quitting one window closes all windows, not sure if that's expected or 
 not.
 
 
 Thanks,
 
 Jeremy Whiting
 




Re: kompare, libkomparediff2 and KDE Applications 14.12

2014-10-08 Thread Kevin Kofler
Hi Aleix, hi everyone,

On Wednesday 08 October 2014 at 22:08:58, Aleix Pol wrote:
 I think we're aiming for 2nd quarter of 2015, it depends a lot on when we
 manage to have a sprint I'd say. Things are going forward, but there's
 still work to be done.

So that will likely be after the next Applications release after 14.12 anyway, 
so there's no need to get the libkomparediff2 port into 14.12 now.

 Also we probably want to do some polishing in libkompare? For example I saw
 there's quite some actual code in the .h files, that can be problematic in
 the future and makes it really hard to maintain ABI and API.

And that's another reason not to rush things.

Kevin Kofler


Re: kompare, libkomparediff2 and KDE Applications 14.12

2014-10-07 Thread Kevin Kofler
Hi,

On Tuesday 07 October 2014 at 15:59:17, Jeremy Whiting wrote:
 Libkomparediff2 has been ported to kf5 for quite some time, kdevelop
 or kdevplatform frameworks based code (not sure if it's master or
 frameworks branches) uses it and it seems to work ok there. I wonder
 when we should merge libkomparediff2's frameworks branch into master
 though.  For one thing it would probably be a bad idea to release
 kdesdk 14.12 with kompare based on qt4 and libkomparediff2 based on
 qt5, also, kdevelop using kf5 will most likely want/need a release of
 libkomparediff2 that uses kf5/qt5 before they can release.

Hmmm, indeed, I see that it's a mess. What about releasing both, a kdelibs 4 
version of libkomparediff2 from master with kdesdk 14.12 and a KF5 version of 
libkomparediff2 from frameworks with KDevelop 5? We would then merge frameworks 
into master at some point after 14.12 branches, when the Kompare port will be 
ready.

 So Kevin, are you planning to port kompare itself to qt5/kf5 for the
 14.12 release (looks like feature freeze is at the end of october
 though, so maybe not enough time, depending) ?

My plan was actually to target the port for the next release. It has always 
been the KDE-wide plan that not all applications would be ported in 14.12. I 
almost certainly don't have the time to do that port within the 3 weeks that 
are left until feature freeze (October 29 according to TechBase).

 I can help with/do the kompare port if needed/wanted, just thought I'd
 get some information from the libkomparediff2 users before I took any
 action at this point I guess.

If you are going to do the port for 14.12, I'm not going to stop you (as long 
as you stick to the freeze schedule and the port is feature-complete by 
feature freeze), but the amount of time I can spend on this is limited at the 
moment.

Kevin Kofler


Re: KF5: Parsing times with timezone abbreviations

2014-03-31 Thread Kevin Kofler
Thiago Macieira wrote:
 Time zone abbreviations are useless, since they are not unique. Simply
 strip them out of your string before passing to QDateTime.

Nice theory, but there is no other way to know what time this actually is. 
Unless you can offer a mapping from latitude and longitude to timezone, or a 
way to automatically figure it out from place name, country (which is 
especially fun for those countries that span multiple time zones, because 
the place name can be a small town somewhere).

I do see the problem, e.g. I get EST as the timezone for Sydney, 
Australia, which is obviously not the same as the US EST. I suppose 
KDateTime will do the wrong thing for that. :-(

Maybe we need a (timezone abbreviation, country) → timezone map (and an API 
where I can just feed in the time including the abbreviation and the country 
name and get a correct QDateTime; heck, for most countries, the abbreviation 
could be ignored entirely, it only matters for huge countries such as the 
USA or Russia)?

Kevin Kofler



Re: KF5: Parsing times with timezone abbreviations

2014-03-31 Thread Kevin Kofler
David Jarvie wrote:
 They can't just be ignored for small countries, since they may contain a
 daylight savings time indication.

Except for one evil hour every year, a given local time with the date 
included is either necessarily DST or necessarily non-DST.

That said, sure, if we are about to look at the abbreviation anyway, we may 
just as well always do it.

Now for that BBC weather RSS issue, I figured out that it apparently always 
reports the pubDate (which is unfortunately not the real datetime the 
conditions are from, but the datetime the RSS was requested at) in the 
local time of the place, given in a format like +1100. (That's the offset 
for Sydney, at which I looked because of that Australian vs. US timezone 
ambiguity.) So we could probably work with that, and hope that the reported 
observation time is really always given in the same timezone as that pubDate 
(and that this does not change in the future, e.g. to a pubDate in UTC).

But what do we do with other sources using this (admittedly broken) type of 
timezone specification? There's even RFC-822 that specifies those 3-letter 
specifications as one of the allowed formats, but only for UT=GMT=UTC 
and for the continental US time zones ([PMCE][SD]T). As a data point of what 
other implementations do with that, the BSD strptime actually tries to 
support RFC-822 completely if you pass %z (but does not support non-US 
three-letter abbreviations), the glibc one didn't bother at all last I 
checked (its %z supports only the +1100 type (ISO 8601) format).

Kevin Kofler



Re: KF5: Parsing times with timezone abbreviations

2014-03-30 Thread Kevin Kofler
Hi,

about a week ago, I wrote:
 what is the recommended way to parse times with short timezone
 abbreviations, such as 16:00 CET, in Qt 5 / KF5 land? (I am asking
 because this is the format the BBC is using for weather observation times
 in their RSS files.)

One week later, I still didn't get a useful answer to this question. (I also 
tried pinging #kde-devel on IRC and John Layt through the contact form on 
his blog, neither has helped getting an answer either.)

The real-world code where this is needed:
https://bugs.kde.org/show_bug.cgi?id=330773#c20
(This patch is against kde-workspace 4.11.7 and so just uses KDateTime, but 
as I said, that is deprecated in KF5, so we will need something different 
for the KF5 port.)

Do I have to conclude that there is no supported way to perform this very 
basic task in KF5? Do we have to start requiring some third-party time-
handling library in the weather plasmoid?

Kevin Kofler



Re: KF5: Parsing times with timezone abbreviations

2014-03-25 Thread Kevin Kofler
Luca Beltrame wrote:
 Can you try looking at KFormat? The KF5 porting notes explicitly mention
 that.

KFormat does not do any parsing.

Kevin Kofler



KF5: Parsing times with timezone abbreviations

2014-03-23 Thread Kevin Kofler
Hi,

what is the recommended way to parse times with short timezone
abbreviations, such as 16:00 CET, in Qt 5 / KF5 land? (I am asking because
this is the format the BBC is using for weather observation times in their
RSS files.)

In kdelibs 4, this should do what we want:
dateTime = KDateTime::fromString(timeString, %H:%M 
%Z).toLocalZone().dateTime();
but KDateTime is deprecated in KF5.

I looked at:
* QDateTime, but neither of the 2 QDateTime::fromString overloads in Qt 5.2
  support the needed format, at least according to the documentation. The
  one that takes a string does not support time zones (or so the
  documentation says), the one that takes an enum entry does not support the
  required format either.
* QTimeZone, but I cannot find a straightforward (documented) way to get a
  QTimeZone from only the short abbreviation. I guess one could go through
  all the availableTimeZoneIds and checking if the abbreviation matches, but
  that won't necessarily give the expected (most probable) result
  (especially if the abbreviation is not unique), will it?

Kevin Kofler



Re: KDM + ConsoleKit + Logind

2014-03-23 Thread Kevin Kofler
Sorry for digging up this 1-month-old thread, but:

Martin Briza wrote:
 We're building KDM with the support (no real reason to put it away except
 getting rid of dependency on the library) but anyway, everything should
 run just fine if you don't really use any of the CK functionality...

Not true. We have been building KDM without libck-connector support in 
Fedora since 4.8.0-11.fc17 on February 22, 2012.

Kevin Kofler



Re: Review Request 110687: DrKonqi should check for disabled version as the very first step in the reporting assistant.

2013-06-01 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110687/#review33556
---


Not commenting on the merits of the patch here, only on the spelling/grammar:


drkonqi/reportassistantpages_bugzilla_versioncheck.cpp
http://git.reviewboard.kde.org/r/110687/#comment24799

s/does not accepts/does not accept/



drkonqi/reportassistantpages_bugzilla_versioncheck.cpp
http://git.reviewboard.kde.org/r/110687/#comment24800

s/does not accepts/does not accept/



drkonqi/ui/assistantpage_versioncheck.ui
http://git.reviewboard.kde.org/r/110687/#comment24798

s/trakcing/tracking/


- Kevin Kofler


On May 30, 2013, 7:08 a.m., Jekyll Wu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/110687/
 ---
 
 (Updated May 30, 2013, 7:08 a.m.)
 
 
 Review request for KDE Runtime and George Kiagiadakis.
 
 
 Description
 ---
 
 As I have said in https://bugs.kde.org/show_bug.cgi?id=315073#c3 ,  
 Bugzilla's new and nice behavior (since 4.2.5) of rejecting reports against 
 disabled versions brings a new usability problem to DrKonqi: users spend 
 value time in downloading debug symbols, generating the backtrace, writing 
 all information he/she can recall, but in the end only to find an error 
 dialog telling them (in a not so clear and friendly way) that bugs.kde.org 
 won't accept his/her report. 
 
 I would propose making version checking the very first step in the reporting 
 assistant: a perfect bug report against an outdated version is still useless. 
 
 The patch has been created for sometime and works, but unfortunately I don't 
 have much time for coding since then, so it is not as good as what I have 
 planned to make it to be. Nevertheless, I think it is still a good 
 improvement of the current situation.
  
 
 
 This addresses bug 315073.
 http://bugs.kde.org/show_bug.cgi?id=315073
 
 
 Diffs
 -
 
   drkonqi/CMakeLists.txt 39833d7 
   drkonqi/drkonqi_globals.h d5f098a 
   drkonqi/productmapping.h f926c9d 
   drkonqi/productmapping.cpp f4e59e5 
   drkonqi/reportassistantdialog.cpp c296059 
   drkonqi/reportassistantpages_bugzilla_versioncheck.h PRE-CREATION 
   drkonqi/reportassistantpages_bugzilla_versioncheck.cpp PRE-CREATION 
   drkonqi/reportinterface.h c7e374e 
   drkonqi/reportinterface.cpp 4190c40 
   drkonqi/ui/assistantpage_versioncheck.ui PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/110687/diff/
 
 
 Testing
 ---
 
 
 File Attachments
 
 
 rejecting disabled version
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/05/28/drkonqi-version-checking.png
 reject disabled versions (revised edition)
   
 http://git.reviewboard.kde.org/media/uploaded/files/2013/05/30/bugzilla-version-cheking-improved.png
 
 
 Thanks,
 
 Jekyll Wu
 




Re: Plasma Workspaces 4.11: the last feature release in the 4.xseries for kde-workspace

2013-05-04 Thread Kevin Kofler
On Friday 03 May 2013 at 14:39:31, Aaron J. Seigo wrote:
 a completely workable approach was suggested. the work on ksecrets started
 and stopped a couple times.

A complicated workaround for the freeze was suggested that would have been a 
lot of work when the existing much simpler approach already worked. The 
approach would have had no technical advantage other than working around the 
pointless freeze. (Quite the opposite, the plugin approach that was suggested 
would have introduced a circular dependency in distribution packages.) It's no 
wonder the KSecrets developer didn't have the time and/or motivation to 
rewrite all his code for that approach.

 at one point it was tested for inclusion and apparently it did not work in
 some configurations (i forget the exact issue; it was quite a while ago and
 the discussion iirc was on kde-packager?). but usefully, it progressed quite
 far.

The version that got released didn't work at all:
* replacing KWallet didn't work because the kdelibs patch was rejected and the 
suggested plugin-based solution was never implemented,
* replacing gnome-keyring didn't actually work either, and the bug(s) which 
prevented that from working was/were never fixed because the project got 
abandoned due to the kdelibs freeze.

We tried packaging it in Fedora. The above was what we found out. And we were 
not alone: In fact, KSecrets got removed from later coordinated KDE releases 
due to it not working at all.

 hopefully you can put it in a repository that can be used by kdelibs which
 would both get around the 4.x kdelibs freeze *and* prepare it for
 frameworks.

I'm not the KSecrets developer.

Kevin Kofler


Re: Plasma Workspaces 4.11: the last feature release in the 4.xseries for kde-workspace

2013-05-04 Thread Kevin Kofler
On Friday 03 May 2013 at 17:04:44, Matthias Klumpp wrote:
 @Kevin: I am only remotely following this issue, but as PackageKit
 developer, I would of course like to see our project in Plasma
 Workspaces as soon as possible. But I don't know the exact issues
 here. Also, having KSecrets merged would be a nice goal too. (The
 SecretService API is very stable, and has the potential to become a
 new de-facto XDG standard soon)

The issues are the same in both cases: They're kdelibs features and kdelibs 
does not accept any features. By the time the first release of the Plasma 
Workspaces based on KF5 is planned, kdelibs will have been feature-frozen for 
THREE YEARS!!! (And I'm not even speaking of applications there!) This is a 
completely unacceptably long freeze period. It's time to reopen kdelibs for 
new features (though a lot of the damage has already been done! But still, 
it'd prevent any further damage) instead of freezing kde-workspace the same 
way.

 So, why not work on merging all this stuff into Workspaces 5 as early
 as possible, so it is present there?

Because Workspaces 5 / 2 / whatever (versioning and naming have become a total 
mess ever since the rebranding fiasco) is not anywhere near testable. Not even 
Frameworks 5 is (but my feature is not really testable without a working 
plasma-desktop anyway). At best I could dump a completely untested and 
untestable code drop.

In addition, even just compiling the change to verify that it even builds 
takes hours because there are no packages to work against. (We have Qt 5 
packages now going through review in Fedora, which is a lengthy process; KF5 
is not even started, and to be honest I'm not sure whether it even makes sense 
to start packaging it in its current stage.)

Testing my kdelibs 4.x patch was easy: I backported it to the version of 
kdelibs that was stable in Fedora, rebuilt my kdelibs package with it, updated 
kdelibs on my notebook, restarted plasma-desktop and tested the feature that 
way.

 (With having WS 4.11 frozen, it IMHO does not make any sense at all to
 develop new features for it, unless the new feature works on both
 Workspaces versions with less modifications)

The problem there is exactly that 4.x is frozen. It's not practical to develop 
features against 5 yet!

Kevin Kofler


Re: Re: Plasma Workspaces 4.11: the last feature release in the 4.xseries for kde-workspace

2013-05-03 Thread Kevin Kofler
 Martin Gräßlin mgraess...@kde.org schrieb:
 @Kevin: could you please stop this nonesense about your GSoC project. Yes we 
 all got that you are frustrated two years ago. Enough is enough! I don't want 
 to ever read about it any more!

OK, so let's put aside *my* project and look at a more important one which got
sabotaged even more by the kdelibs freeze: KSecrets! At least my project works
(as a distro patch), KSecrets never got finished after the kdelibs patch was
rejected. Really sad for a cross-desktop interoperability project all distros 
had
been waiting for for ages! :-(

Kevin Kofler


Re: Plasma Workspaces 4.11: the last feature release in the 4.x series for kde-workspace

2013-05-02 Thread Kevin Kofler
On Wednesday 01 May 2013 at 09:41:15, Andriy Rysin wrote:
 If the feature freeze for 4.11 is May 22 and there is no more feature
 releases for branch 4.x, where all those features from GSoC go?

Probably the same as my libplasma PackageKit integration from GSoC 2011: in 
the digital nirvana and/or in distro patches. :-(

Just as for kdelibs, I really don't understand the purpose of sabotaging the 
work of those people who want to implement features in 4.x. Doing this in 
parallel to Qt-5-based long-term development is exactly what git (or any half-
decent SCM, really) has branches for. It would not be the same people working 
on the 2 branches.

The kdelibs freeze has sabotaged several important features distributions have 
been waiting for for years, e.g. KSecrets (which was abandoned in non-working 
stage after the kdelibs patches were rejected). We should not repeat the same 
mistake for kde-workspace!

Kevin Kofler


Re: Review Request 108727: ktimezoned: Watch /etc/localtime if it doesn't exist yet.

2013-02-27 Thread Kevin Kofler


 On Feb. 27, 2013, 1:07 p.m., Commit Hook wrote:
  This review has been submitted with commit 
  7a42d977c90a5f0f387d170745e8a7b01f7d9675 by Kevin Kofler to branch KDE/4.10.

Merged to master with merge commit c36f1809671d434db1be700ef9c433f40a9157b5.


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108727/#review28202
---


On Feb. 3, 2013, 4:30 a.m., Kevin Kofler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/108727/
 ---
 
 (Updated Feb. 3, 2013, 4:30 a.m.)
 
 
 Review request for KDE Runtime and David Jarvie.
 
 
 Description
 ---
 
 /etc/localtime legitimately might not exist. The default is then UTC. But the 
 file can then be created later, so watch for its creation.
 
 If we don't do this, when setting the time zone for the first time using 
 kcm_clock, the initially set time zone will fail to get reloaded and the 
 dialog will unexpectedly jump back to UTC.
 
 This problem shows up on Fedora 18, see:
 https://bugzilla.redhat.com/show_bug.cgi?id=906972
 
 Please note that to test the fix with kcm_clock, you also need the kcm_clock 
 (kde-workspace) fix from:
 https://git.reviewboard.kde.org/r/108711/
 (which is already approved and which I'll push to KDE/4.10 and merge to 
 master as soon as the 4.10.0 tagging freeze is lifted).
 
 
 Diffs
 -
 
   ktimezoned/ktimezoned.cpp 4eafa4e 
 
 Diff: http://git.reviewboard.kde.org/r/108727/diff/
 
 
 Testing
 ---
 
 Builds against at least 4.10.0 and 4.9.5.
 
 Works at runtime (and appears to fix the bug): 
 https://bugzilla.redhat.com/show_bug.cgi?id=906972#c5
 
 
 Thanks,
 
 Kevin Kofler
 




Re: Review Request 108727: ktimezoned: Watch /etc/localtime if it doesn't exist yet.

2013-02-06 Thread Kevin Kofler


 On Feb. 5, 2013, 8:27 p.m., David Faure wrote:
  Looks ok to me, although I don't really know this code.

So, should I wait for David Jarvie or some other person who feels familiar with 
this code to give it a go, or do you think I can commit this right now?


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108727/#review26711
---


On Feb. 3, 2013, 4:30 a.m., Kevin Kofler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/108727/
 ---
 
 (Updated Feb. 3, 2013, 4:30 a.m.)
 
 
 Review request for KDE Runtime and David Jarvie.
 
 
 Description
 ---
 
 /etc/localtime legitimately might not exist. The default is then UTC. But the 
 file can then be created later, so watch for its creation.
 
 If we don't do this, when setting the time zone for the first time using 
 kcm_clock, the initially set time zone will fail to get reloaded and the 
 dialog will unexpectedly jump back to UTC.
 
 This problem shows up on Fedora 18, see:
 https://bugzilla.redhat.com/show_bug.cgi?id=906972
 
 Please note that to test the fix with kcm_clock, you also need the kcm_clock 
 (kde-workspace) fix from:
 https://git.reviewboard.kde.org/r/108711/
 (which is already approved and which I'll push to KDE/4.10 and merge to 
 master as soon as the 4.10.0 tagging freeze is lifted).
 
 
 Diffs
 -
 
   ktimezoned/ktimezoned.cpp 4eafa4e 
 
 Diff: http://git.reviewboard.kde.org/r/108727/diff/
 
 
 Testing
 ---
 
 Builds against at least 4.10.0 and 4.9.5.
 
 Works at runtime (and appears to fix the bug): 
 https://bugzilla.redhat.com/show_bug.cgi?id=906972#c5
 
 
 Thanks,
 
 Kevin Kofler
 




Re: Review Request 108711: kcmdatetimehelper: Hardcode PATH because $PATH is empty here.

2013-02-06 Thread Kevin Kofler


 On Feb. 6, 2013, 10:26 p.m., Commit Hook wrote:
  This review has been submitted with commit 
  c517bbc175a3fbb09f13a8468a6987fdbc547d23 by Kevin Kofler to branch KDE/4.10.

… and merged to master with merge commit 
e99a0e5b1223b7a673b0bc29c280d962c6e8dfb6.


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108711/#review26785
---


On Feb. 2, 2013, 8:27 a.m., Kevin Kofler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/108711/
 ---
 
 (Updated Feb. 2, 2013, 8:27 a.m.)
 
 
 Review request for kde-workspace, Christoph Feck and Oswald Buddenhagen.
 
 
 Description
 ---
 
 Unfortunately, we cannot rely on the $PATH environment variable in KAuth 
 helpers, because D-Bus activation clears it. So we have to use a reasonable 
 default for the KStandardDirs::findExe search path, and actually use the 
 return value of KStandardDirs::findExe in the calls to KProcess::execute.
 
 This fixes things so hwclock and zic actually get found. See: 
 https://bugzilla.redhat.com/show_bug.cgi?id=906854 . This got noticed in 
 Fedora 18 because it does not always create /etc/localtime, so the fallback 
 code operating on /etc/localtime triggered an error.
 
 
 Diffs
 -
 
   kcontrol/dateandtime/helper.cpp 5a946d8 
 
 Diff: http://git.reviewboard.kde.org/r/108711/diff/
 
 
 Testing
 ---
 
 Builds against at least 4.10.0 and 4.9.5.
 
 Works at runtime on Fedora 18, see: 
 https://bugzilla.redhat.com/show_bug.cgi?id=906854#c12 (The reporter 
 encountered another issue, apparently because ktimezoned also misbehaves when 
 /etc/localtime is absent, but at least this particular issue is confirmed 
 fixed.)
 
 
 Thanks,
 
 Kevin Kofler
 




Re: Review Request 108711: kcmdatetimehelper: Hardcode PATH because $PATH is empty here.

2013-02-04 Thread Kevin Kofler


 On Feb. 4, 2013, 3:53 p.m., Konstantinos Smanis wrote:
  We can do better than hardcoding a reasonable default. We can launch a 
  login shell (1) and 'echo $PATH' the user's PATH. This has many advantages:
  
  1. We don't miss paths (e.g. /usr/local/bin, /usr/local/sbin etc.).
  2. We honor the precedence of paths as set in $PATH by the user.
  3. We only use the user's PATH (DBus activation works for non-root users 
  too).
  
  I am currently working on this approach for kcm-grub2 which also misbehaves 
  when $PATH is not set. If you are interested, you may restrain from 
  committing until I post a link to the commit.
  
  (1) A login shell is needed to properly source /etc/profile, ~/.profile 
  and/or other shell-specific login scripts (such as ~/.bash_profile for 
  Bash).
 
 Konstantinos Smanis wrote:
 Here you are: 
 http://commits.kde.org/kcm-grub2/7c5beb979fdf9dd14abfffb0e24d4f69b11ca985
 
 Thomas Lübking wrote:
 Question (for the particular case and in general):
 This is about a suid root:dbus call (thus env clearing for dbus system 
 activation) correct?
 Ie. the called application is executed with root privs?
 
 In this case there should afaics rather not be _any_ PATH resolution at 
 all and checking the usual suspects is about the last acceptable process.
 
 Otherwise one could place a random binary zic or hwclock into the 
 $PATH and gain a root shell (or are there further protections against this?)

* This code (and any KAuth helper, actually) always runs as root.
* We're looking for 2 core system executables, the chances they are in 
/usr/local are near zero, and I'd also share Thomas Lübking's security 
concerns. (D-Bus clears the path for a reason.)
* Spawning a login shell looks like a really overengineered and ugly solution, 
considering the above.
* kcm-grub2 is the last piece of software I'd model a KAuth helper on, given 
that your KAuth actions are implemented in a totally insecure way. (Last I 
checked, the process running as the user passes an arbitrary file to the helper 
running as root, defeating the whole purpose of finegrained PolicyKit 
permissions. Give a user that PolicyKit permission and you essentially gave him 
root. Of course, it's not a problem if you stick to the default auth_admin 
policy, but if some local admin tries to change it, ouch!)


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108711/#review26617
---


On Feb. 2, 2013, 8:27 a.m., Kevin Kofler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/108711/
 ---
 
 (Updated Feb. 2, 2013, 8:27 a.m.)
 
 
 Review request for kde-workspace, Christoph Feck and Oswald Buddenhagen.
 
 
 Description
 ---
 
 Unfortunately, we cannot rely on the $PATH environment variable in KAuth 
 helpers, because D-Bus activation clears it. So we have to use a reasonable 
 default for the KStandardDirs::findExe search path, and actually use the 
 return value of KStandardDirs::findExe in the calls to KProcess::execute.
 
 This fixes things so hwclock and zic actually get found. See: 
 https://bugzilla.redhat.com/show_bug.cgi?id=906854 . This got noticed in 
 Fedora 18 because it does not always create /etc/localtime, so the fallback 
 code operating on /etc/localtime triggered an error.
 
 
 Diffs
 -
 
   kcontrol/dateandtime/helper.cpp 5a946d8 
 
 Diff: http://git.reviewboard.kde.org/r/108711/diff/
 
 
 Testing
 ---
 
 Builds against at least 4.10.0 and 4.9.5.
 
 Works at runtime on Fedora 18, see: 
 https://bugzilla.redhat.com/show_bug.cgi?id=906854#c12 (The reporter 
 encountered another issue, apparently because ktimezoned also misbehaves when 
 /etc/localtime is absent, but at least this particular issue is confirmed 
 fixed.)
 
 
 Thanks,
 
 Kevin Kofler
 




Re: Review Request 108711: kcmdatetimehelper: Hardcode PATH because $PATH is empty here.

2013-02-04 Thread Kevin Kofler


 On Feb. 4, 2013, 3:53 p.m., Konstantinos Smanis wrote:
  We can do better than hardcoding a reasonable default. We can launch a 
  login shell (1) and 'echo $PATH' the user's PATH. This has many advantages:
  
  1. We don't miss paths (e.g. /usr/local/bin, /usr/local/sbin etc.).
  2. We honor the precedence of paths as set in $PATH by the user.
  3. We only use the user's PATH (DBus activation works for non-root users 
  too).
  
  I am currently working on this approach for kcm-grub2 which also misbehaves 
  when $PATH is not set. If you are interested, you may restrain from 
  committing until I post a link to the commit.
  
  (1) A login shell is needed to properly source /etc/profile, ~/.profile 
  and/or other shell-specific login scripts (such as ~/.bash_profile for 
  Bash).
 
 Konstantinos Smanis wrote:
 Here you are: 
 http://commits.kde.org/kcm-grub2/7c5beb979fdf9dd14abfffb0e24d4f69b11ca985
 
 Thomas Lübking wrote:
 Question (for the particular case and in general):
 This is about a suid root:dbus call (thus env clearing for dbus system 
 activation) correct?
 Ie. the called application is executed with root privs?
 
 In this case there should afaics rather not be _any_ PATH resolution at 
 all and checking the usual suspects is about the last acceptable process.
 
 Otherwise one could place a random binary zic or hwclock into the 
 $PATH and gain a root shell (or are there further protections against this?)
 
 Kevin Kofler wrote:
 * This code (and any KAuth helper, actually) always runs as root.
 * We're looking for 2 core system executables, the chances they are in 
 /usr/local are near zero, and I'd also share Thomas Lübking's security 
 concerns. (D-Bus clears the path for a reason.)
 * Spawning a login shell looks like a really overengineered and ugly 
 solution, considering the above.
 * kcm-grub2 is the last piece of software I'd model a KAuth helper on, 
 given that your KAuth actions are implemented in a totally insecure way. 
 (Last I checked, the process running as the user passes an arbitrary file to 
 the helper running as root, defeating the whole purpose of finegrained 
 PolicyKit permissions. Give a user that PolicyKit permission and you 
 essentially gave him root. Of course, it's not a problem if you stick to the 
 default auth_admin policy, but if some local admin tries to change it, ouch!)

Oh, and looking at your commit, what you did before was even worse, of course. 
Passing PATH from the main application to the helper is totally unacceptable 
for the same reason as passing an arbitrary file is, see my previous comment 
and Thomas Lübking's comment.


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108711/#review26617
---


On Feb. 2, 2013, 8:27 a.m., Kevin Kofler wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/108711/
 ---
 
 (Updated Feb. 2, 2013, 8:27 a.m.)
 
 
 Review request for kde-workspace, Christoph Feck and Oswald Buddenhagen.
 
 
 Description
 ---
 
 Unfortunately, we cannot rely on the $PATH environment variable in KAuth 
 helpers, because D-Bus activation clears it. So we have to use a reasonable 
 default for the KStandardDirs::findExe search path, and actually use the 
 return value of KStandardDirs::findExe in the calls to KProcess::execute.
 
 This fixes things so hwclock and zic actually get found. See: 
 https://bugzilla.redhat.com/show_bug.cgi?id=906854 . This got noticed in 
 Fedora 18 because it does not always create /etc/localtime, so the fallback 
 code operating on /etc/localtime triggered an error.
 
 
 Diffs
 -
 
   kcontrol/dateandtime/helper.cpp 5a946d8 
 
 Diff: http://git.reviewboard.kde.org/r/108711/diff/
 
 
 Testing
 ---
 
 Builds against at least 4.10.0 and 4.9.5.
 
 Works at runtime on Fedora 18, see: 
 https://bugzilla.redhat.com/show_bug.cgi?id=906854#c12 (The reporter 
 encountered another issue, apparently because ktimezoned also misbehaves when 
 /etc/localtime is absent, but at least this particular issue is confirmed 
 fixed.)
 
 
 Thanks,
 
 Kevin Kofler
 




Review Request 108711: kcmdatetimehelper: Hardcode PATH because $PATH is empty here.

2013-02-02 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108711/
---

Review request for kde-workspace, Christoph Feck and Oswald Buddenhagen.


Description
---


Diffs
-

  kcontrol/dateandtime/helper.cpp 5a946d8 

Diff: http://git.reviewboard.kde.org/r/108711/diff/


Testing
---


Thanks,

Kevin Kofler



Re: Review Request 108711: kcmdatetimehelper: Hardcode PATH because $PATH is empty here.

2013-02-02 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108711/
---

(Updated Feb. 2, 2013, 8:27 a.m.)


Review request for kde-workspace, Christoph Feck and Oswald Buddenhagen.


Description (updated)
---

Unfortunately, we cannot rely on the $PATH environment variable in KAuth 
helpers, because D-Bus activation clears it. So we have to use a reasonable 
default for the KStandardDirs::findExe search path, and actually use the return 
value of KStandardDirs::findExe in the calls to KProcess::execute.

This fixes things so hwclock and zic actually get found. See: 
https://bugzilla.redhat.com/show_bug.cgi?id=906854 . This got noticed in Fedora 
18 because it does not always create /etc/localtime, so the fallback code 
operating on /etc/localtime triggered an error.


Diffs
-

  kcontrol/dateandtime/helper.cpp 5a946d8 

Diff: http://git.reviewboard.kde.org/r/108711/diff/


Testing (updated)
---

Builds against at least 4.10.0 and 4.9.5.

Works at runtime on Fedora 18, see: 
https://bugzilla.redhat.com/show_bug.cgi?id=906854#c12 (The reporter 
encountered another issue, apparently because ktimezoned also misbehaves when 
/etc/localtime is absent, but at least this particular issue is confirmed 
fixed.)


Thanks,

Kevin Kofler



Review Request 108727: ktimezoned: Watch /etc/localtime if it doesn't exist yet.

2013-02-02 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108727/
---

Review request for KDE Runtime and David Jarvie.


Description
---

/etc/localtime legitimately might not exist. The default is then UTC. But the 
file can then be created later, so watch for its creation.

If we don't do this, when setting the time zone for the first time using 
kcm_clock, the initially set time zone will fail to get reloaded and the dialog 
will unexpectedly jump back to UTC.

This problem shows up on Fedora 18, see:
https://bugzilla.redhat.com/show_bug.cgi?id=906972

Please note that to test the fix with kcm_clock, you also need the kcm_clock 
(kde-workspace) fix from:
https://git.reviewboard.kde.org/r/108711/
(which is already approved and which I'll push to KDE/4.10 and merge to master 
as soon as the 4.10.0 tagging freeze is lifted).


Diffs
-

  ktimezoned/ktimezoned.cpp 4eafa4e 

Diff: http://git.reviewboard.kde.org/r/108727/diff/


Testing
---

Builds against at least 4.10.0 and 4.9.5.

Works at runtime (and appears to fix the bug): 
https://bugzilla.redhat.com/show_bug.cgi?id=906972#c5


Thanks,

Kevin Kofler



Re: [Nepomuk] Nepomuk - Moving away from Strigi

2012-10-08 Thread Kevin Kofler
On Monday 08 October 2012 at 16:03:26, todd rme wrote:
 What do you mean by separate repository?  Do you mean people would
 be able to build and install the plugins independently of
 nepomuk-core?  Certainly for the legally questionable ones this would
 be nice.

For us in Fedora, this would be not just nice, but REQUIRED. We cannot ship 
anything depending on FFmpeg in Fedora. So it's either shipping the offending 
indexers in RPM Fusion in a separate package (which will obviously NOT be 
installed by default, we cannot install things by default which are not in 
Fedora proper) or not shipping them at all.

Now ideally you wouldn't depend on FFmpeg at all! I especially don't 
understand why you need an AV decoding library to do indexing.

Kevin Kofler

 Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe 


Re: [Nepomuk] Nepomuk - Moving away from Strigi

2012-10-08 Thread Kevin Kofler
On Monday 08 October 2012 at 15:59:12, Aleix Pol wrote:
 In any case, I'm unsure that KDE can shine without ffmpeg.

It can, and it must. FFmpeg (or Libav which is basically the same thing) is 
not acceptable for Fedora and will not be in Fedora any time soon. (Maybe in 
something like 20 years when all the current multimedia patents expire, but by 
then there will probably be new codecs with new patents.)

Decoding needs to be done using GStreamer.

Kevin Kofler

 Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe 


Re: Re: [Nepomuk] Nepomuk - Moving away from Strigi

2012-10-08 Thread Kevin Kofler
On Monday 08 October 2012 at 22:26:30, Martin Gräßlin wrote:
 Neither Kevin (Fedora), Scott (Kubuntu) nor Adrian (SUSE) came up with the
 rules, they are just pointing out the policies. While I do not agree with
 their policies I can kind of understand it.
 
 What everybody can do is to bring the question back to the distributions
 that they might reconsider the situation and bringing back the feedback
 that Upstream really wants to have working multimedia in the downstream.
 
 Though especially for the distributions situated in the USA I have little
 hope that they might change anything. The patent system there is extremely
 broken and there is quite a difference between knowing that the software
 breaks patents or some bogous IP claims like SCO.
 
 We all know that situation and we won't change it here on kde-devel. I'd
 love to do so, but it won't happen :-(
 
 So let's all calm down and work on good solutions :-)

The best solution I can offer is: if at all possible, please (pretty please) DO 
NOT use FFmpeg (directly), use GStreamer.

There are 4 APIs you can use to use GStreamer in your application:
* Phonon (with Phonon-GStreamer, which is the default in most distributions). 
The highest-level API, suitable for simple playback, not so much for other 
tasks.
* QtMobility Multimedia. Listed for completeness, probably not ideal for KDE 
applications. Also high-level.
* QtGStreamer. Lower-level Qt bindings for GStreamer. More flexible.
* GStreamer directly. C/GObject API. Lowest-level, but not as low-level as 
FFmpeg. ;-) If even QtGStreamer does not do what you want, try this one.

Now I don't know whether GStreamer is suitable for the Nepomuk indexing task 
which started this thread, but e.g. for the Amarok transcoding and 
fingerprinting, it could definitely be used instead of the FFmpeg-based code we 
have now (see sound-converter (the GNOME one, not soundKonverter) for an 
example of how to use GStreamer for transcoding, transcoding is actually one 
of the things it can do well). Thumbnailing is also something which could and 
should be done using GStreamer rather than FFmpeg as ffmpegthumbnailer does.

GStreamer helps us in several ways:
* GStreamer is modular, so distros can ship only the codecs they feel safe and 
others can be added as plugins. (FFmpeg, on the other hand, links all the 
codecs into a single monolithic shared library.)
* GStreamer supports system decoding libraries for many formats. In 
particular, most unencumbered codecs can be decoded without depending on the 
monolithic FFmpeg. This approach also encourages code sharing rather than 
reinventing the wheel as FFmpeg loves to do.
* GStreamer supports FFmpeg (or actually Libav these days) codecs as one of 
the many plugins.
* GStreamer upstream actually cares about the legal issues and already 
classifies the stable codecs into good and ugly. (The less-proven bad 
ones, on the other hand, are not classified by upstream and are split into 3 
pieces in Fedora and RPM Fusion: free and patent-free, free but patent-
encumbered and outright non-free. But that's just a matter of auditing the 
plugins and splitting them, interested third parties can look at our split.)
* If everything uses GStreamer, the user only has to add the evil codecs 
once for all apps. (And there's even a codec auto-installation feature, though 
admittedly that will only work if the repository containing the codec is 
already configured, so it's not a magic bullet.) If, on the other hand, we go 
with the alternative solution of having FFmpeg-based plugins for everything, 
sure, that also solves our problem (as long as they're really plugins, not 
compile-time options like Amarok's fingerprinting!), but it means the user has 
to install a lot of *-ffmpeg packages to get everything working.

So in short, if you want multimedia to just work in your software, use 
GStreamer!

Kevin Kofler

 Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe 


Re: ALERT: KDElibs (at least) 4.8.4 is actually 4.8.80+

2012-06-09 Thread Kevin Kofler
On Saturday 09 June 2012, Modestas Vainius wrote:
 I don't know yet if any other modules from 4.8.4 has been
 mis-packaged in the same way

Due to the permanent feature freeze of kdelibs 4, there is actually no 
difference between kdelibs 4.8.4 and 4.8.80/4.8.90 other than the version 
number, they both come from the 4.8 branch (which itself was originally the 
4.7 branch).

Yes, it's stupid, but it's how the kdelibs maintainers want things to be. :-(

Kevin Kofler


Re: ALERT: KDElibs (at least) 4.8.4 is actually 4.8.80+

2012-06-09 Thread Kevin Kofler
On Saturday 09 June 2012, Albert Astals Cid wrote:
 There is not stupid not weird thing in kdelibs. We simply declared kdelibs
 to be feature perfect and only bugfixes should be happening there.
 
 What's the problem with that?

That no software is ever perfect.

For example, we will NEED the Solid udisks2 backend in future releases of
Fedora because udisks 1 is going away! As far as I know (I'm not a Red Hat
employee), it will be used in the next release of RHEL as well. So we have to
backport it as a distro patch. (The author of the backend did the backport.)
For the other distros:
http://pkgs.fedoraproject.org/gitweb/?p=kdelibs.git;a=blob;f=kdelibs-udisks2_prep.patch;h=2962837cdbc3cc1200ce323902f9173b79c2b1b5;hb=HEAD
http://pkgs.fedoraproject.org/gitweb/?p=kdelibs.git;a=blob;f=kdelibs-udisks2-backend.patch;h=74ec3c2872d118d0335cc95c608a95ef9cd40f92;hb=HEAD
http://pkgs.fedoraproject.org/gitweb/?p=kdelibs.git;a=blob;f=kdelibs-udisks2_post.patch;h=f22742dc8db8b2dd2b48de6f916c4a50a360db6b;hb=HEAD

kdelibs has now been feature-frozen for a year, and it looks like it will live
at least 2 years in feature-frozen state, plus several more as a compatibility
library. (We still ship a kdelibs3 compatibility library in Fedora, it's still
needed, e.g. for Quanta.) This is just unacceptable.

The libraries should be the LAST thing to be frozen, even after the core
applications have already moved on, because of the many third-party
applications relying on them. Freezing the libraries first, when even the core
applications still rely on the old version, just makes no sense whatsoever.

Kevin Kofler


Re: ALERT: KDElibs (at least) 4.8.4 is actually 4.8.80+

2012-06-09 Thread Kevin Kofler
On Saturday 09 June 2012, Aaron J. Seigo wrote:
 things that change APIs, that do not leave kdelibs non-useful on large %s
 of user systems, etc. would therefore not meet the likely requirements.
 and i say that as someone who is sitting on patches to kparts (written by
 Ivan, not me :) that i would LOVE to see in kdelibs now rather than later
 (we use those patches in Active images actually :) ... so this is a standard
 i believe in strongly enough to hold my own interests to as well :)

The fact that such patches are shipped as distro patches in Active images 
rather than upstreamed into kdelibs as they should is actually one of the 
major obstacles to getting Plasma Active packaged in general-purpose 
distributions. You are not actually hurting yourself with this decision (you 
just carry those patches as distro patches, while at the same time complaining 
that we do the same thing to our Plasma improvements in Fedora), you are 
hurting everyone else trying to package Plasma Active.

The kdelibs freeze was a major mistake and such a wrong decision should be 
fixed to the extent still possible (at least reopen kdelibs 4.10 for new 
features, even though the fallout of kdelibs 4.8 and 4.9 having been bugfix-
only is already bad enough) and never be made again.

Kevin Kofler


Re: ALERT: KDElibs (at least) 4.8.4 is actually 4.8.80+

2012-06-09 Thread Kevin Kofler
On Sunday 10 June 2012, Nicolás Alvarez wrote:
 Why not start now and make the next kdelibs 4.8.5? Releasing a kdelibs
 4.9 will just add to the confusion of how kdelibs development is
 working.

Because having a kdelibs version different from (and especially lower than) the 
KDE SC version messes up our packaging pretty badly.

 Even if we call it 4.9.0, it doesn't need a beta/RC. That causes
 problems because we are releasing multiple versions which *aren't in
 increasing order* and have overlapping release schedules (4.8.80 and
 4.8.4 were very close to each other) from the same branch...

See above.

In addition, it would also likely show up in things like
kde4-config --kde-version and thus confuse users.

Your solution was already proposed (in fact it was the original plan of the 
kdelibs developers) and rejected for very good reasons.

Kevin Kofler


Re: Review Request: Fix Drkonqi to work with bugzilla 4 (partial port to xml-rpc)

2012-03-09 Thread Kevin Kofler

---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104200/#review11252
---


I think adding a dependency is a much cleaner solution than bundling. But I 
wonder whether the best long-term fix wouldn't be to just move kxmlrpcclient to 
kdelibs, it clearly seems to be useful for more than just PIM. But in distros, 
I think most of us prefer the dependency on kdepimlibs (we can also make a 
subpackage for that library in our packaging) to bundling (which is against 
Fedora and Debian policies, at least).

- Kevin Kofler


On March 9, 2012, 12:45 a.m., George Kiagiadakis wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/104200/
 ---
 
 (Updated March 9, 2012, 12:45 a.m.)
 
 
 Review request for KDE Runtime.
 
 
 Description
 ---
 
 This patch partially ports DrKonqi to use xml-rpc, which is done at this 
 point to support the new bugzilla 4.2 instance on bugs.kde.org. I have tried 
 to keep the changes as minimal as possible, so that this can be applied to 
 the 4.8 branch.
 
 Note that this patch involves copying the kxmlrpcclient library from 
 kdepimlibs in drkonqi's source code tree. I have done this to avoid modifying 
 dependencies in 4.8.2. However, if an extra dependency of kde-runtime on 
 kdepimlibs sounds ok, I can remove the internal copy in master.
 
 Full history at:
 http://quickgit.kde.org/index.php?p=clones%2Fkde-runtime%2Fgkiagia%2Fdrkonqi.gita=shortlogh=refs/heads/drkonqi-xmlrpc
 
 
 This addresses bug 295276.
 http://bugs.kde.org/show_bug.cgi?id=295276
 
 
 Diffs
 -
 
   drkonqi/CMakeLists.txt 590239dfbd9236463ef11eede699eb84f5806b7a 
   drkonqi/bugzillalib.h c89c85c8df3724dc6e51ed03aab7ef7362a22472 
   drkonqi/bugzillalib.cpp 2df29bd975196b13df02934bfa8899e30d9627f6 
   drkonqi/drkonqi_globals.h 9281b0574737632782c96f90dadad1c86abebd0c 
   drkonqi/kxmlrpc/CMakeLists.txt PRE-CREATION 
   drkonqi/kxmlrpc/README PRE-CREATION 
   drkonqi/kxmlrpc/client.h PRE-CREATION 
   drkonqi/kxmlrpc/client.cpp PRE-CREATION 
   drkonqi/kxmlrpc/query.h PRE-CREATION 
   drkonqi/kxmlrpc/query.cpp PRE-CREATION 
   drkonqi/reportassistantpages_bugzilla.cpp 
 fba91b6d179c867d9984d2296b7302a469c8d0e5 
 
 Diff: http://git.reviewboard.kde.org/r/104200/diff/
 
 
 Testing
 ---
 
 Sucessfully reported and attached extra backtrace on:
 https://bugs4.kde.org/show_bug.cgi?id=294820
 https://bugs4.kde.org/show_bug.cgi?id=294821
 
 The duplicate search also seems to be working fine.
 
 
 Thanks,
 
 George Kiagiadakis
 




Re: Suggestions for some KDE default options

2011-12-12 Thread Kevin Kofler
Ingo Klöcker wrote:
 Markus made exactly the right reply, constructive (except for the
 whining bit) and to the point. You didn't. Your reply was not really
 helpful. In fact, I, as one of your esteemed list mods, find it
 outright insulting.

I'm with Eike Hein. Don't feed the troll!

Sending the trolls to the distro mailing lists like Markus did between the 
lines is NOT helpful, we can't use them any more over there than here! Those 
suggested defaults don't make any more sense in distros than upstream.

Kevin Kofler



Re: kactivities update

2011-12-08 Thread Kevin Kofler
Aaron J. Seigo wrote:
 yes, i skipped the details. done properly it should be linking
 libkactivities. this is an acceptable short term work-around given it's in
 a branch, while doing it right requires the modularization to be
 complete for at least libkactivities dependencies.

Or it'd require just building everything as one package as we've been doing 
before, which is the easiest way to avoid circular dependencies…

Modularization keeps causing problems like this.

FWIW, since we're planning to ship Plasma Active alongside Plasma Desktop in 
Fedora 17 (parts are already in Rawhide), we'll likely carry these patches 
anyway (if they get released as part of the Plasma Active patchset), though 
we are no fans of copied classes just to avoid circular dependencies (There 
ought to be a better solution!).

I find it also quite funny that the very people complaining about 
distributions patching in features into their kdelibs are doing that very 
same thing as part of their Plasma Active distribution…

Kevin Kofler



Re: kactivities update

2011-12-08 Thread Kevin Kofler
Aaron J. Seigo wrote:
 yes, i skipped the details. done properly it should be linking
 libkactivities. this is an acceptable short term work-around given it's in
 a branch, while doing it right requires the modularization to be
 complete for at least libkactivities dependencies.

Or it'd require just building everything as one package as we've been doing 
before, which is the easiest way to avoid circular dependencies…

Modularization keeps causing problems like this.

FWIW, since we're planning to ship Plasma Active alongside Plasma Desktop in 
Fedora 17 (parts are already in Rawhide), we'll likely carry these patches 
anyway (if they get released as part of the Plasma Active patchset), though 
we are no fans of copied classes just to avoid circular dependencies (There 
ought to be a better solution!).

I find it also quite funny that the very people complaining about 
distributions patching in features into their kdelibs are doing that very 
same thing as part of their Plasma Active distribution…

Kevin Kofler



Re: Hunspell offering Hebrew by default

2011-12-02 Thread Kevin Kofler
Steven Sroka wrote:
 Why does Hunspell install the Hebrew dictionary by default? I'm
 running Chakra and I noticed that I had the option in System
 Settings-Locale-Spell Checker to use Hebrew even though I couldn't
 find a Hebrew package installed. Later I found out that the main
 Hunspell package offered the Hebrew dictionary.

I suspect that the Hebrew dictionary you're seeing is probably not from 
Hunspell, but from Hspell, which is Hebrew-specific. Hunspell does have a 
Hebrew dictionary available, but it is not installed by default.

Kevin Kofler



Caveat about QFileDialog

2011-11-17 Thread Kevin Kofler
Hi,

In this recent discussion:
http://mail.kde.org/pipermail/kde-frameworks-devel/2011-November/26.html
it was decided to use QFileDialog instead of KFileDialog, because:
 QFileDialog now uses the platform dialog, including the KDE one

This was executed with commits such as:
http://commits.kde.org/kdelibs/017799ceb0c8a05d2bd3ce7d3c259529d68c6217

But the problem is: At least in Qt 4.8 (I haven't checked Qt 5 yet because 
browsing its submodules in the Gitorious interface doesn't work properly), 
only the STATIC API of QFileDialog supports platform dialogs. If you construct 
a QFileDialog OBJECT as the above commit does, you always get the ugly Qt-only 
dialog! So it's a very bad idea to do that.

Kevin Kofler
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: This starts to become a dangerous path (Was: New Feature for kdelibs)

2011-11-17 Thread Kevin Kofler
I think this thread is indeed quite a dead end, but since you mentioned my 
feature:

Thomas Zander wrote:
 And I'm still not seeing anyone put in the in comparison tiny fraction of
 time of porting the auto-download plasma thing to frameworks.

Let me clear up a few things:

* The problem isn't that I need to port the feature to Frameworks, but that 
I need to ship it in Fedora 17 in April-May 2012, not somewhere in 2013 when 
the Frameworks/libplasma2-based Plasma Desktop 5 will be ready.

* FYI, porting the feature to Frameworks 5 requires changes in PackageKit 
and Apper first: currently, the requested services are assumed to be in the 
type-name, e.g. dataengine-weather, format, and PackageKit then asks RPM for 
plasma4(dataengine-weather). Now, we need to have a separate namespace for 
Plasma 5 / libplasma2 services, so we need to change PackageKit to do what 
it already does for Python (python2 vs. python3 namespaces) and also accept 
requests of the plasma5(dataengine-weather) form (and just like for Python 
where it defaults to python2 if no namespace is given, it'll default to 
plasma4 if no namespace is given here, for backwards compatibility – note 
that this is already in released versions of PackageKit, so we can't just 
change the semantics to default to the new namespace). And the pretty 
display of the services in Apper also needs to cope with the extended 
format. So my plan is to submit patches to PackageKit and Apper first of 
all. (I'm sorry that I didn't get this right in the first place, but when I 
wrote the PackageKit and Apper patches, I had no idea libplasma2 would be 
such a pressing matter.)

Kevin Kofler



Re: New Feature for kdelibs (Was: The case for a kdelibs 4.8)

2011-11-15 Thread Kevin Kofler
Andrea Diamantini wrote:
 Ok, let's wait 18 months to see private browsing fixed. Going to update
 bug reports...

Try nagging distros to backport your (or your contributors') patches. 
Unfortunately, it looks like trying to convince the kdelibs maintainers at 
KDE is a lost cause, as you can see from this and other threads. :-( They 
just don't care about their users anymore, instead focusing only on some 
ideal future.

I talked to my fellow Fedora KDE packagers on IRC today and it looks like 
we'll be backporting (or rather, applying, since 4.x is what they got 
developed against) the kde#54300 patches.

Of course, whenever you ask distros to backport features, it helps if you 
can already offer a patch(set) against 4.x (as is the case for the kde#54300 
patches), since backporting from Frameworks is more work. (Whether the patch 
is against 4.8, 4.7 or even 4.6 usually won't matter, as long as it's 4.x.)

Kevin Kofler



Re: New Feature for kdelibs (Was: The case for a kdelibs 4.8)

2011-11-15 Thread Kevin Kofler
Scott Kitterman wrote:
 It is probably worth a discussion on packagers to share cross-distro ideas
 about what kdelibs feature patches to ship with 4.8. Having some
 commonality at least among the distros seemslike a good idea to me.

Please move this to kde-packager. It's off topic for kde-core-devel, and 
kde-packager is where the distro people sit. Thanks in advance.

Kevin Kofler



Re: This starts to become a dangerous path (Was: New Feature for kdelibs)

2011-11-15 Thread Kevin Kofler
Thomas Lübking wrote:
 And that is gonna happen in what way if it's not in any lib?
 Static linking?! External lib? Problem solved?

Applications which are not part of KDE SC might end up just requiring a 
patched kdelibs to even build.

Kevin Kofler



Re: This starts to become a dangerous path (Was: New Feature for kdelibs)

2011-11-15 Thread Kevin Kofler
Thomas Lübking wrote:
 Therefore my suggestion (if opening 4.x even as wrapper linking
 frameworks is no option) would be to compile the features into the
 application requiring it right now rather than forking a library
 because you cannot alter it. (Don't forget: this is about covering the
 time to frameworks, thus temp. bloat can be accepted to a certain
 degree for a better good)

That's not always possible, at least not without forking huge portions of 
kdelibs into the application. (Remember that the main thing the Frameworks 
effort wants to change is exactly that everything depends on everything in 
kdelibs.) E.g. if you need a new way to handle cookies, you can't just add 
that to your application without copying the whole cookie-handling code and 
everything which references it (i.e. probably KHTML etc.), and renaming it 
all to prevent symbol clashes, which is not a scalable approach. Also note 
that distributors absolutely HATE bundled library code.

Kevin Kofler



Re: [proposal] Move all of the ksecretsservice components into kdeutils/ksecrets

2011-11-13 Thread Kevin Kofler
Sebastian Kügler wrote:

 On Saturday, November 12, 2011 08:12:27 Kevin Kofler wrote:
 All this stuff is going to make things much more work for us packagers
 (just  like the separate kactivities), for no good reason other than some
 arbitrary we froze kdelibs decision!
 
 Calling it an arbitrary decision shows either of two things: you ignore
 the rationale behind it, or you don't understand it.

It's true that I don't understand the rationale behind the freeze, and never 
will. It just doesn't make sense.

Developing for the future needs to be done in parallel to developing for the 
present, not at its expense.

Note that I do understand the rationale about Frameworks 5 to some extent, 
though I'll admit that I'm not enthousiastic about yet another major binary-
incompatible version and that I'm worried about the undesired side effects 
of reducing dependencies (from a quick look at the current frameworks 
branch: lost features (e.g. tilde expansion in KUrl), lost consistency (e.g. 
use of QFileDialog rather than KFileDialog, and last I checked QFileDialog 
only supported native dialogs with the static methods, so when you construct 
a QFileDialog object, you'll get the ugly Qt-only one), possible code 
duplication to avoid a dependency (not seen in frameworks yet, but I've seen 
this elsewhere and am worried that it might happen in frameworks too, 
especially when you'll try to actually fix that code you just #if 0ed 
without readding the dependency)). It's the freeze of kdelibs 4.x I have a 
problem with, not Frameworks 5 per se.

 Frameworks 5 was a very conscious decision taken by the people, who by all
 reasonable means are in the position to do so, and so was freezing
 kdelibs. I understand you're not happy with this decision, and I'm sure by
 now everybody else knows that, too -- no need to repeat it over and over
 again without any added value. People are actually getting tired of it.

And I'm getting tired of seeing issue after issue pop up here, all caused by 
that freeze, and the obvious solution to the issues getting summarily 
rejected. Am I really alone in that? :-(

 Stating the bleeding obvious, if you want to get kdelibs (or its
 successor) open to feature additions, help getting F5 ready, don't make it
 take longer.

If I didn't care about the long term, why would I have removed all the 
remaining Qt3Support and kde3support usage from Kompare recently?

See for yourself:
http://websvn.kde.org/trunk/KDE/kdesdk/kompare/?view=log

What I don't see is why the long-term development needs to happen at the 
expense of short-term development.

Kevin Kofler



Re: Request: remove libkactivites from kdelibs (in experimental/)

2011-11-12 Thread Kevin Kofler
Aaron J. Seigo wrote:
 first, let's back off from the unecessary rhetoric. for instance, i don't
 think calling people hypocrites is going to get anyone anywhere other than
 annoyed, upset and divided. i hope we can agree on that.

Unfortunately, it's the decision by some kdelibs developers including you to 
stop everyone from working on kdelibs 4.x which is annoying, upsetting and 
dividing people.

I don't want to offend you, but to make you realize this fact.

I am sorry if I come off as rude, it's really not my intention, but you have 
to realize that I'm really frustrated by this situation.

 On Friday, November 11, 2011 04:58:09 Kevin Kofler wrote:
 You cannot really FORCE volunteers to work on what you want them to work
 on.
 
 no, but we can decide to work together and coordinate instead of working
 against each other, particularly when we share the same final interests.

Working together goes both ways, you also have to listen to our needs. 
Nobody who doesn't read kde-core-devel daily and who didn't happen to be at 
the in-person meeting you discussed this in was even aware of your post-4.7 
plans, let alone asked for their opinion. In particular, nobody asked us 
distro people for it: I don't recall any sort of notice about this issue to 
kde-packager. Yet scheduling is definitely something which needs to be 
discussed with distro packagers.

The need in my case is that we are going to ship my feature in Fedora 17 
(This is not negotiable. It's already a release later than I had initially 
hoped for, but at least in Fedora, missing a freeze window only delays you 
for 6 months. I really miss the time where the same thing was true for all 
the KDE SC modules. Sadly, judging from the kdepim fiasco first and now 
this, it looks like Shuttleworth's lecture on how important predictable 
periodic releases are for us distributions already got forgotten!) and that 
IMHO it would have been better for everyone if it had been upstream by then. 
There is obviously no way we can ship a Plasma based on libplasma from 
Frameworks in Fedora 17, so 4.8 was and is the target. And when I submitted 
my GSoC proposal, I didn't even THINK that there'd possibly be no kdelibs 
4.8, or one with no new features allowed. In fact I only heard of this after 
my patches to PackageKit and Apper were all already finished and applied 
upstream (which proved to be very straightforward, unlike libplasma where I 
hit a brick wall).

 note that your statement is the same as saying that as a volunteer i can
 then commit anything i want to your application / library, which is,
 obviously, not true. there are means of process control, particularly
 maintainership.

Sorry, but for me, keeping other people from doing their work is a sign of 
bad maintainership. I don't dispute that a maintainer is ALLOWED to do so, 
but that doesn't make it a good idea. The fact that this topic comes up 
again and again (see e.g. today's threads about ksecretsservice) proves that 
several people have a concrete need for doing work on kdelibs 4.x. Instead, 
they're forced to work around you (plural you!) when you could easily 
allow them to work WITH you. (Again, working together goes both ways, 
closing down the branch people want to commit to is NOT working WITH them.)

 note that the decisions which i am simply repeating and asking people to
 respect were made this past summer in a meeting of some 30 libs
 contributors

In-person meetings (a.k.a. the infamous watercooler discussions) are about 
the worst possible place to take decisions in a global community project 
with many volunteer (and thus necessarily part-time) contributors.

 and then brought here to k-c-d for further discussion.

That's a more appropriate forum, but why hasn't anybody notified e.g.
kde-packager? And even then, there will always be people who missed the 
discussion, which is another reason why reconsidering decisions based on new 
evidence is sometimes needed.

I think the sample of developers included in your discussions suffered from 
heavy selection bias, as (for various reasons) those people interested in 
4.x work aren't the ones who go to your long-term planning meetings nor the 
ones who watch kde-core-devel daily. (I wasn't following it at all when this 
was initially discussed.)

 to ignore would be disrespectful of the time honoured principles in KDE.

No, it would simply be realizing you made a mistake!

 Preventing them from working on what THEY want to work on will just lead
 to them moving their work elsewhere, e.g.:
 
 well, that's exactly what i hope will happen. i hope they will movetheir
 work into frameworks (either the branch if working on existing code or a
 git repository that will become part of frameworks)

Putting the work into frameworks (only) is obviously not a solution for code 
we want to ship now.
 
 * separate git repositories (which in fact is exactly what you are doing
 with libkactivities!
 
 yes, because that is the shape the frameworks

Re: [proposal] Move all of the ksecretsservice components into kdeutils/ksecrets

2011-11-12 Thread Kevin Kofler
Valentin Rusu wrote:
 However, I should remove the ksecretsserviced from kde-runtime and let
 it go the the ksecrets repository, under kdeutils. And I'll do it later
 today.

Uhm, kde-runtime isn't frozen like kdelibs…

Kevin Kofler



Re: [proposal] Move all of the ksecretsservice components into kdeutils/ksecrets

2011-11-11 Thread Kevin Kofler
Valentin Rusu wrote:
 As you may already know, the ksecretsservice API merging to the
 kdelibs/4.7 branch was no longer an acceptable solution because of
 recent frameworks related decisions. It was suggested to put it into
 it's separate repository, alongside with the
 kde-runtime/ksecretsserviced I also merged yesterday.
 
 After thinking twice, I'd like to bring these components into the
 kdeutils/ksecrets repository.
 Are you ok with this?
 
 A possible issue if that's ok for you: the kdeutils/kwallet will depend
 on this repository and it contains guard code to exclude
 ksecretsservice-related code if qca not found - that will be extended to
 the case ksecrets repository will not be found. This raises the problem
 of module dependencies: kdelibs needs to be built first, but if ksecrets
 was not compiled first, the required headers will not be there and as
 such kwallet will not have the ksecretsservice-related code.
 That looks like a circular reference :-) I'll discuss this on the next
 frameworks irc meeting or maybe a solution for this kind of situation
 already exists?

All this stuff is going to make things much more work for us packagers (just 
like the separate kactivities), for no good reason other than some arbitrary 
we froze kdelibs decision!

Circular dependencies are an absolute PITA for packaging.

We definitely do want your ksecretsservice work ASAP and I don't see why it 
can't be in kdelibs where it belongs.

Kevin Kofler



Re: Request: remove libkactivites from kdelibs (in experimental/)

2011-11-10 Thread Kevin Kofler
Aaron J. Seigo wrote:

 On Monday, November 7, 2011 16:35:57 Albert Astals Cid wrote:
 Maybe we should really resurrect the existence of a master 4.8?
 
 unecessary imho, and runs the extreme danger of repeating the 3-4 debacle
 with kdelibs where people just keep working on the stable release and
 don't care enough for the next important release of libs.

You cannot really FORCE volunteers to work on what you want them to work on. 
Preventing them from working on what THEY want to work on will just lead to 
them moving their work elsewhere, e.g.:

* separate git repositories (which in fact is exactly what you are doing 
with libkactivities! Talk about hypocrisy…),

* distro patches (which you keep complaining about, yet it is exactly what 
the frozen master debacle is leading to),

* maybe even an outright fork like Trinity (of KDE 3) or MATE (of GNOME 2). 
(But don't get me wrong, I'm NOT interested in keeping 4.x alive for ages, 
I'm just interested in 4.x NOW because 5.0 isn't anywhere near ready, even 
when we're just talking about the libraries/frameworks, let alone when we 
actually consider applications and/or workspaces building on the new 
libraries/frameworks. And I believe that most, if not all, people interested 
in 4.x work right now are in that boat, I doubt 4.x is going to suscite 
anywhere near the amount of long-term nostalgia 3.x did. We WILL eventually 
work on 5.x. Just not NOW.)

In addition, this situation might actually push some contributors NOT to 
work with you on 5.0 material. I can tell you that your refusal to get my 
libplasma PackageKit work into 4.8 definitely did demotivate me from doing 
any work on the 5.0 version. So you achieve exactly the opposite of what 
you're trying to achieve.

That said, considering the 4.8 release schedule, it is now really too late 
to reopen kdelibs 4.8 for feature development anyway. :-( It should have 
been done when I originally asked for it a month ago (or even better, it 
should never have been closed down in the first place!).

Kevin Kofler



Re: [Kde-cvs-announce] KDE SC 4.8 Soft Feature Freeze

2011-10-31 Thread Kevin Kofler
Valentin Rusu wrote:

 On 10/31/2011 01:01 PM, Allen Winter wrote:
 [Release Team added to the discussion]
 [also adding kde-core]

 On Sunday 30 October 2011 5:09:47 PM Valentin Rusu wrote:
 2. code under a kdelibs branch may be merged to the to be 4.8 branch?

 I don't know.
 We've never had this situation with kdelibs before.

 In theory, KSecretService is a new feature for kdelibs and as long as it
 passes review before 10 November it should be allowed in KDE SC 4.8.

 OTOH: we've been unofficially making noise that kdelibs is frozen
 except for bug fixes.

 I'm of the opinion that KSecretService can be released with kdelibs 4.8
 as long as it passes review.
 I'd be glad to get it into 4.8, as others would start testing it and
 eventually (or better surely) filing bug reports. That would be very
 helpful.

What about my Plasma PackageKit integration work? This has been in 
ReviewBoard for a while, but not pushed because there was no branch to push 
it to, and the reviews also got stuck because of this issue (which means 
they would need to be approved, of course).

Kevin Kofler



Re: Review Request: Start D-Bus after setting KDE_FULL_SESSION

2011-10-30 Thread Kevin Kofler


 On Aug. 3, 2011, 6:19 p.m., Oswald Buddenhagen wrote:
  this sounds wrong. what if dbus is started earlier, e.g. by a PAM module? 
  this can easily happen once we use the new SecretService.
  there should be some interface to push environment variables into the bus 
  after it is running. maybe it would be ok to read  a file with VAR=value 
  assignments (one per line, no quoting whatsoever - printenv output) from a 
  defined per-session location before activating services.
 
 Kevin Krammer wrote:
 I guess a D-Bus enabled app could actually check the workspace it is 
 running on instead of falling back to the detect by environment variable 
 hack.
 E.g. by checking if Plasma Desktop's name is registered or ksmserver is 
 the session server, etc.
 
 Arno Rehn wrote:
 I'm with Ossi here. And AFAIK the detect by env var way is the official 
 way of detecting a full KDE session. Plasma Desktop alone doesn't make a full 
 KDE session (or does it?).
 This patch might not be a full fix, but it certainly improves the 
 situation for many users. I think it should go in, but with a big FIXME 
 marker.

+1 to Ossi. This fix will NOT work in Fedora, because Fedora now starts D-Bus 
from xinitrc.


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102194/#review5362
---


On Aug. 3, 2011, 12:15 p.m., Christoph Feck wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/102194/
 ---
 
 (Updated Aug. 3, 2011, 12:15 p.m.)
 
 
 Review request for kdelibs, Kevin Kofler, George Kiagiadakis, and David Faure.
 
 
 Description
 ---
 
 As described in bug 267770. Luckily, there is no KDE-specific initialization 
 between D-Bus launch and setting KDE_FULL_SESSION, so interchanging them 
 should not affect KDE itself.
 
 
 This addresses bug 267770.
 http://bugs.kde.org/show_bug.cgi?id=267770
 
 
 Diffs
 -
 
   startkde.cmake 92c8753 
 
 Diff: http://git.reviewboard.kde.org/r/102194/diff/diff
 
 
 Testing
 ---
 
 I don't know D-Bus activated apps, I assume I don't have any, so I did not 
 see any difference in startup.
 
 
 Thanks,
 
 Christoph Feck
 




Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)

2011-10-30 Thread Kevin Kofler
Thiago Macieira wrote:
 2. heuristically (replacing the current KUrl::KUrl and
 KUrl::fromPathToUrl): * absolute paths should be file paths
* relative paths should be file paths (!) (currently, they're URLs)
* everything with a URL scheme (protocol) should be a URL
 
 I disagree and will not implement this in QUrl.

You keep claiming this is not needed or useful, but how else would you 
suggest handling the use case of a network-transparent file dialog (or file 
requester, i.e. a line edit + an open button which brings up a file 
dialog, to be used in dialogs, see KUrlRequester) where the USER expects to 
be able to type in any of:
* an absolute path,
* a path relative to some local current directory or
* a URL, which will always be absolute?
Should all the code needing to handle something like this come with their 
own heuristics to handle this same user input?

Anyway, it can be implemented in KUrl (replacing or adding to the current 
heuristics, which are also not in QUrl).

Kevin Kofler



Re: Re: Qt 4.8 QUrl.toLocalFile behavior change, impacts to KUrl (and friends)

2011-10-28 Thread Kevin Kofler
Albert Astals Cid wrote:
 Personally i find it another joke in the history of Qt, saying you
 maintain API and ABI (that you do) but then making functions behave
 totally different from one version to another is just plain useless.

+1

You just CANNOT change the behavior of an existing function in such a way.

Kevin Kofler



Re: kdelibs 4.8? What to do about GENERIC_LIB_VERSION?

2011-10-03 Thread Kevin Kofler
Martin Gräßlin wrote:
 If it's only for bug fixes, wouldn't it be easier to branch 4.8 from 4.7?
 That way only two instead of three branches have to be maintained. If we
 open master for 4.8 development again (even only for bugfixes), I fear we
 will have much discussions again about adding features to it. I don't want
 to have lengthy threads on kcd each other week were developers state how
 evil KDE is by not letting them include their awesome feature NOW.

Surely making fun of volunteers trying to contribute features to KDE 
software and failing because of a stupid bureaucratic policy is NOT the way 
to encourage more contributions to the KDE projects…

Kevin Kofler



Re: Review Request: W7 Tab thumbnails in dolphin.

2011-10-03 Thread Kevin Kofler


 On Oct. 3, 2011, 1:15 p.m., David Faure wrote:
  Couldn't this be done higher in the stack, e.g. in KTabWidget or in Qt?

Indeed, I think there should be some interface for this stuff in kdelibs, so 
that 1. applications don't have to add such platform-specific code and 2. the 
feature can also be implemented for our own Plasma Desktop workspace, and 
applications will automatically benefit (whereas with the current solution, 
they'd need more platform-specific code doing the same thing). Seeing KDE 
applications doing more things in a foreign desktop workspace than in our own 
doesn't look right to me. :-(

(Yet another kdelibs feature that would justify doing a 4.8 release…)


- Kevin


---
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102758/#review7032
---


On Oct. 3, 2011, 1:25 a.m., Andrius da Costa Ribas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/102758/
 ---
 
 (Updated Oct. 3, 2011, 1:25 a.m.)
 
 
 Review request for KDE Base Apps, KDE Accessibility, kdewin, Patrick 
 Spendrin, and Peter Penz.
 
 
 Description
 ---
 
 Add Windows 7 tab thumbnails feature to dolphin.
 Mostly based on the example from 
 http://nicug.blogspot.com/2011/03/windows-7-taskbar-extensions-in-qt-tab.html.
 
 An icon representation is used instead of actual thumbnails ( please agree 
 that those microscopic previews are not useful at all ;] ). Changing an icon 
 when url changes is also easier than checking all the time whether something 
 inside the window has been changed. Using icons is a lot more KDE-ish and 
 therefore more beautiful and user-friendly than the default Windows behavior 
 ;).
 
 win7utils.h and win7utils.cpp are from 
 https://github.com/xfreebird/blogstuff/tree/master/qt/thumbnailtabs_example1
  with few adaptations.
 
 
 Diffs
 -
 
   dolphin/src/CMakeLists.txt 93225c5 
   dolphin/src/dolphinapplication.h 69d07c3 
   dolphin/src/dolphinapplication.cpp 0dc9c96 
   dolphin/src/dolphinmainwindow.h 9fb83bf 
   dolphin/src/dolphinmainwindow.cpp 6ca6e59 
   dolphin/src/platform/win7utils.h PRE-CREATION 
   dolphin/src/platform/win7utils.cpp PRE-CREATION 
 
 Diff: http://git.reviewboard.kde.org/r/102758/diff/diff
 
 
 Testing
 ---
 
 Tested using MSVC 2010 32 bit, in a Windows 7 64 bit machine.
 
 May need testing:
 - Compiling under mingw-w32 and mingw-w64
 - Using Windows Vista or below to ensure there are no side-efects on a box 
 without this feature.
 - Using Windows 8 (I don't know much about its bugs^H^H^H^Hfeatures ;] )
 
 Known problems:
 - There is no way to know if KTabBar got a tab reordered, so the thumbnails 
 won't be reordered, but their reference is still correct
 - Undefined behavior when dolphin gets unresponsive [e.g.: because of a 
 defective kioslave], most of the code assume dolphin is okay [e.g.: 
 QPixmap::grabWidget won't work in a frozen window]. 
 
 
 Screenshots
 ---
 
 Tabs!
   http://git.reviewboard.kde.org/r/102758/s/281/
 More Tabs!
   http://git.reviewboard.kde.org/r/102758/s/282/
 Too many tabs!
   http://git.reviewboard.kde.org/r/102758/s/283/
 
 
 Thanks,
 
 Andrius da Costa Ribas
 




Re: kdelibs 4.8? What to do about GENERIC_LIB_VERSION?

2011-10-03 Thread Kevin Kofler
Thomas Lübking wrote:
 So what's the big difference?

The big difference is that kdelibs 4 and KDE Frameworks 5 are different 
(sets of) libraries with incompatible APIs which will be shipped alongside 
each other in distributions for years. So this is not a normal freeze, it's 
a permanent freeze of kdelibs 4.

Kevin Kofler



Re: kdelibs 4.8? What to do about GENERIC_LIB_VERSION?

2011-10-03 Thread Kevin Kofler
Thomas Lübking wrote:
 In this case porting would be recompiling and relinking, so if one
 desperately wants a new feature in an application (since libraries
 don't have features for their own sake) one would just port to the
 KDE5 frameworks, yesno?

Not if the application is using deprecated APIs, including but not limited 
to Qt3Support and kde3support.

Kevin Kofler



Re: The case for a kdelibs 4.8

2011-10-01 Thread Kevin Kofler
Aaron J. Seigo wrote:
 the features that got into the 4.7 branch to date have been things that
 were already worked on before the Frameworks decision was made. it's was
 an odd cas were features had been worked on while 4.7 was frozen with the
 expectation of a 4.8 ... and that left us with the choice of having a 4.8
 release which we didn't want for those few features, leaving those
 features out and screwing up the planning of the applications depending on
 those changes or fudging a little bit.

Not all those features are merged in yet, see e.g. my Plasma PackageKit GSoC 
work.

 note that some of the more actively changing and developed code in kdelibs
 have moved to separate git repositories already to make this issue moot
 for them (kactivities, nepomuk)

That is also a nightmare to packagers. Especially kactivities is a big 
problem because kde-workspace 4.7 (not 4.8!) is now reported to rely on a 
new kactivities which is not in kdelibs 4.7. That makes no sense whatsoever.

Kevin Kofler



Re: Re: The case for a kdelibs 4.8

2011-10-01 Thread Kevin Kofler
Martin Gräßlin wrote:
 One of the main reasons for the rebranding was to realize that KDE is
 not one product, but a community that produces multiple products among
 them a desktop environment (Plasma). What you just try to tell us is that
 the complete rebranding is nonsense and we should go back to talking just
 about KDE for everything bringing the users back to assuming they need
 Plasma in order to use KMail.

That's what some of us have been trying to tell you for months…

Kevin Kofler



Re: The case for a kdelibs 4.8

2011-10-01 Thread Kevin Kofler
 to be in kdelibs are not so critical
 or fundamental that Bad Things will hapen if they aren't available right
 away. if we are more conservative about the value assessments we make on
 these features, it becomes apparent that those features can indeed wait,
 just as they already have for some 3 years now.

Those features are already in Rawhide for Fedora 17 and there's no way I 
will drop them from there. I'm more likely to also backport them to Fedora 
16 (coming soon) and maybe even 15 (the current stable release).

 5. We have a master branch which is unused.
 
 this isn't a reason for or orgainst a 4.8 release, and there is good
 development reasons for that decision. yes, it has been a source of
 confusion for those who build from git sources who didn't follow the
 frameworks decision and so got caught out by not changing their kdelibs
 repo to the 4.7 branch. that seems to be fixing itself and people learn
 about these changes.

But why should people HAVE to learn that? What kdelibs is doing is different 
from what 99% of the projects using git are doing.

Kevin Kofler



Re: The case for a kdelibs 4.8

2011-10-01 Thread Kevin Kofler
PS:

Aaron J. Seigo wrote:
 the choice that packagers have is to actually work with us instead of
 against us.

We would very much love to work with you. In fact, this is why I submitted 
my patches to KDE ReviewBoard before even getting them into Rawhide. I 
really WANT these to be upstream. Fedora doesn't like carrying downstream 
patches as a general principle.

However, working with you is only possible if you are also interested in 
working with us, which implies listening to our needs, concerns and wishes. 
By closing down the branch where our current development is necessarily 
focused on since that's what we will be shipping in the near future, you're 
already starting down the wrong road.

For those Plasma PackageKit features, sure, they're not strictly required, 
which is why we got away without them until now. But to our users, they mean 
that installing a widget through download new widgets… will actually 
install the required dependencies, so the widget they downloaded will 
actually WORK. What, to us developers, is a feature, actually fixes a bug 
from a user's perspective. We, the distributions, interact directly with 
users, and so are receptive to their needs, concerns and wishes, and tend to 
base ours on them.

And finally, I strongly doubt that my features are the only post-4.7 kdelibs 
features running into the freeze. (In fact, what's now going on with 
kactivities and nepomuk proves quite the opposite.)

Kevin Kofler



Re: The case for a kdelibs 4.8

2011-10-01 Thread Kevin Kofler
PPS:

I wrote:
 However, working with you is only possible if you are also interested in
 working with us, which implies listening to our needs, concerns and
 wishes. By closing down the branch where our current development is
 necessarily focused on since that's what we will be shipping in the near
 future, you're already starting down the wrong road.

For a prime example of what happens if you close down the version 
distributions are actually shipping in favor of long-term development, just 
look at GRUB 1. Fedora's GRUB 0.97 package is now at patch level 75! And it 
has its own git repository, i.e. essentially a fork (grub-fedora on 
git.kernel.org, it's now down because all of git.kernel.org is currently 
down).

GRUB 2 is now finally becoming production-ready, and Fedora will be 
switching to it in Fedora 16, but it was nowhere near that state when 
upstream discontinued GRUB 1. GRUB 0.97 was released on May 8, 2005 (and 
IIRC that was only a bugfix release and they stopped accepting new features 
even earlier). Distributions have been forced to ship patched versions for 
over 6 years. (GRUB 0.97 as released doesn't support many needed features, 
e.g. ext4.) As a result, it has become a morass of all-different forked 
versions. You can't tell a priori what any given GRUB 0.97 package 
actually supports.

Sure, focusing on the long term to some extent is needed, but we 
distributors need something we can ship NOW, not months or years from now.

Kevin Kofler



The case for a kdelibs 4.8

2011-09-29 Thread Kevin Kofler
. 
This unusual setup is confusing both users and developers, see e.g.
http://mail.kde.org/pipermail/kde-windows/2011-September/006070.html

So I am urging you to reconsider your decision and reopen master for those 
people willing to work on 4.8. It's not too late yet, there's a month left 
until the soft feature freeze for KDE SC 4.8.

Kevin Kofler


Re: The case for a kdelibs 4.8

2011-09-29 Thread Kevin Kofler
Rolf Eike Beer wrote:

(re the support for spellchecking with hunspell)
 Given that it is now proven and tested code, who stops you committing it
 into KDE/3.5 branch?

What for? There are no plans to do a 3.5.11 or 3.6.0 release, ever, and the 
one major distribution known to sometimes ship release branch snapshots from 
after the last release of the branch (Debian) just dropped the KDE 3 libs 
from their distribution (a decision I also don't agree with, FWIW, but 
that's off topic here). In addition, that patch is not exactly a bugfix…

The 2 patches against 3.5.10 (one for KSpell and one for KSpell2) can be 
found in Fedora dist-git, and also in KDE Bugzilla. If you think they should 
be in the 3.5 branch, feel free to commit them. But AFAIK, we aren't really 
expected to commit anything to the 3.5 branch anymore, even though AFAIK 
there's no official freeze like the one being enforced on master.

By the way, I also have a patch for K3Spell in the KDE 4 libs. We also ship 
that patch in Fedora. It's basically the same as the KSpell patch for the 
KDE 3 libs, and attached to the same bugs.kde.org report. But I don't know 
whether anything actually still uses K3Spell. (The reason I didn't commit 
that was because the Sonnet maintainer wasn't happy with the idea of adding 
a new feature to that compatibility API, even though it's needed for 
integration. And of course now I couldn't commit it anymore even if I wanted 
because kdelibs master is frozen, which brings us back to the topic…)

 The reason to stop master was (as far as I understand) to make the
 frameworks branch easily to maintain. If someone is working on 4.8
 (bugfixing, features) all this has to be ported to frameworks, too. So you
 develop a moving target on a moving target.

You'd just have to do regular merges from master instead of regular merges 
from KDE/4.7. It wouldn't change much apart from being more consistent with 
common best practices for SCM use.

 I would have wished more than once if we could have done a new release on
 an older branch, e.g. a KDE 4.6.6 where the KLineEdits in Konqueror would
 be fixed. Some sort of long-term branch, or at least one maintenance
 release more when the .0 or .1 is out. You only get severe end user
 testing on the new major release when the .0 is out, so those who don't
 want to break their system will stay on the older branch, which has bugs
 long fixed in the new one. Some sort of chicken and egg problem, of
 course, but it would be helpful for those people to get an additional
 release with bugfixes after the next .0 is out. This could be e.g. in the
 middle between .0 and .1 to not have the release workload for 2 releases
 at once. And: yes, I know, this is even more work for the release team.
 Poor Dirk.

That also makes a lot of sense (so +1 to this suggestion), but it is a 
different issue from the one at hand.

Kevin Kofler



Re: The case for a kdelibs 4.8

2011-09-29 Thread Kevin Kofler
Markus Slopianka wrote:

 On Donnerstag 29 September 2011 14:01:50 Kevin Kofler wrote:
 
 2. It will be confusing to our users, and even to some packagers, to have
 a KDE SC 4.8 on kdelibs 4.7.
 
 Since almost exactly 2 years we (esp. the promo team) are communicating
 that Platform/Frameworks, Applications and Workspaces are three separate
 products that just happen to be shipped on the same date.

But now you're changing exactly that last part, or at least what will be 
released will no longer have a common and consistent version number.

And in any case, even if this particular argument doesn't convince you, that 
does not invalidate all the other reasons why there should be a kdelibs 4.8.

 One of the reasons why the rebranding still didn't get to everybody is
 that some distributions (mostly Fedora!) still spread the wrong message
 about some “KDE 4.7” to its users. (see eg.
 https://fedoraproject.org/wiki/SIGs/KDE/KDE47Packaging#KDE_4.7_Packaging )

This is not a page targeted at our users at all, it's an internal KDE SIG 
TODO list. We sometimes use shorthand when we discuss things, it is obvious 
to us that what's really meant here is the KDE SC. But I changed it to KDE 
SC 4.7 Packaging to make you happy.

And no, we cannot use a more specific term than KDE SC here, because this 
is really about packaging all the stuff that happens to be shipped on the 
same date, with a 4.7.n version number, by the KDE Project. The modules are 
released together, so they get packaged together.

 Users of other distributions do not have such problems

Wait and see the chaos that will come up when users open their Help/About in 
Konqueror and it tells them that they're using Konqueror 4.8.0 under KDE 
4.7.6. (And yes, it still says only KDE in 4.6.5, I haven't checked 4.7 
or 4.8 for whether that's fixed there.)

 and strangely shipping Kontact 4.4.11 with SC 4.5+ was also not confusing
 to most users.

Actually, that was (and still is) quite confusing to many users. Look at the 
mailing lists and forums.

I think this was a mistake that shouldn't be repeated. What should have been 
happened when the Akonadi stuff was found not ready would have been a:
svn rm branches/KDE/4.5/kdepim
svn cp branches/KDE/4.4/kdepim branches/KDE/4.5/kdepim
(and likewise for kdepim-runtime. And yes, SVN was still used at that time), 
i.e. mass-revert the unfinished stuff and do 4.5 releases of what works. 
(But the updated KAddressBook from 4.5 should have been used. The 4.4 one 
was already Akonadi-based and unfinished, and we ended up stuck with that 
temporary solution for over a year. It was a mistake to put that into 4.4 in 
the first place, but with that done, it should have been updated. 
Renumbering the releases to 4.5 instead of continuing with 4.4.x would have 
allowed inserting such a new feature.)

 The rule so far has always been that the kdelibs
 version must be the same as the KDE SC version.
 
 There is no rule – at most a common practise

A common practice which had been enforced by minimum required kdelibs 
versions in CMakeLists.txt.

 and that was broken as well after Fedora 15 has packages for SC 4.7 with
 Kontact 4.4.

Fedora 15 actually has KDE SC 4.6.x, not 4.7.

The upcoming Fedora 16 has KDE SC 4.7.x including kdepim 4.7.x, so the 
version numbers are finally consistent again.

 Changing this will also break all our Fedora KDE SC specfiles
 
 Then fix your specfiles. AFAIK Fedora is also the last big distribution
 that still has monolithic packages like kdemultimedia, kdenetwork, and so
 on. Take the opportunity to fix that as well.

If you had actually read that KDE47Packaging page you linked to and not just 
the title, you'd know that we're actually shipping some modules split in the 
upcoming Fedora 16. Not only are the modules released as split tarballs now 
all also packaged in split form, but we also split e.g. kdeutils into 
subpackages.

 The main reason that
 was given for dropping kdelibs 4.8 is that this means one less branch to
 worry about for the people working on kdelibs, but the people who'd work
 on 4.8 and 5.0 are NOT the same people!
 
 Do you hereby declare that Red Hat will provide resources to create KDE
 Platform 4.8 and an cherrypick useful developments from the frameworks
 branch to Platform 4.8?

I cannot declare that because I don't work for Red Hat (let alone in some 
management position). I'm just a community packager for Fedora. Ask Red Hat 
directly.

Kevin Kofler



Re: The case for a kdelibs 4.8

2011-09-29 Thread Kevin Kofler
Scott Kitterman wrote:
 We did this in Kubuntu and it was confusing.  It was also technically
 challenging.  Speaking as someone investing a lot of time in trying to do
 a high quality job of distributing KDE to end users: Please.  Never, ever,
 do this to us again.

+1

 The transition from kdepim 4.4 - 4.6/4.7 was a special case.  I hope it
 stays that way.  While there wasn't another way to do what had to be done
 in that case

Actually, there would have been another way: revert the Akonadi merge by 
copying the 4.4 branch of kdepim to 4.5 and release that as kdepim 4.5. As I 
wrote in the other mail, that would also have allowed updating KAddressBook 
(which was already Akonadi-based in 4.4) to the much improved version from 
the 4.5 branch (while discarding all the other, unstable changes from 4.5). 
And it would have prevented the version number confusion, the chaos with 
translations etc.

But IMHO the best solution would have been to postpone all this Akonadi 
stuff to 5.0 right from the start (especially considering that 5.0 is 
seriously being worked on now, to the point of stopping kdelibs 4.x 
releases). The original plan was for 4.0, that train was missed. 4.5/4.6/4.7 
is way too late in the 4.x cycle for that kind of major change, it belongs 
into a major release.

We (the KDE community) are now in the paradoxical situation where we won't 
ship even small uninvasive features in kdelibs, yet we happily replaced the 
PIM apps, which handle data many users consider essential, with what was 
essentially a rewrite. Yet both those apparently contradictory decisions are 
symptoms of the same problem: We really need to get used to working with 3 
branches: next major feature release, next minor feature release and bugfix 
release. It is needed in some cases. It would have been needed for kdepim 
(next major feature release = KMail 2 etc. for 5.0, next minor feature 
release = small, incremental improvements for 4.n+1) and it's needed now for 
kdelibs. Each time we try to save a branch, the conflicting requirements of 
doing long-term refactoring vs. delivering new features to our users lead us 
straight to a disaster.

 it should not server as a general success story others should model
 themselves after.

Big +1 to that!

Kevin Kofler



Re: The case for a kdelibs 4.8

2011-09-29 Thread Kevin Kofler
On Thursday 29 September 2011, Heinz Wiesinger wrote:
 From what I remember from the desktop summit the picture you draw here is
 quite an exaggeration of what is actually happening.
 
 kdelibs 4.7 is meant to be frozen for new features, but not for bugfixes.
 Bugfix releases of kdelibs-4.7 happenend and I'm sure will continue on
 happening. As for the versioning I don't see why one of those bugfix
 releases couldn't be rebranded as 4.8.0 if that makes things easier (that
 was even briefly mentioned at the release team BoF). It does not solve
 feature backports of course.

But one of my points is that we need features too, not just bugfixes. 
Continuing 4.7.x releases solves the problem of bugfixes just fine, but 
entirely 
fails to address the issue of features.

 The KDE Frameworks 5.0 development is not meant to take forever. In fact I
 think it's meant to be finished around early 2012, which would leave us
 with a frozen kdelibs for one KDE SC release, maximum two. It's also not a
 huge *we will break everything! Kittens will die!* release, but basically
 just a restructuration of the code, with little to no adjustments
 necessary for applications. (that was how it was marketed, anyway).
 The way I understood it was that there could very well be a KDE SC 4.9
 release shipping a 4.9 workspace on top of 5.0 frameworks.

I don't think a date of early 2012 is realistic at all. With upstream already 
working on Qt 5, I think it doesn't make any sense whatsoever to break 
everything twice, once for KDE Frameworks 5 and once for Qt 5. Yet I strongly 
doubt that Qt 5 will be out so soon.

Even if the changes required for applications will be small, they will be 
needed (e.g. deprecated stuff will be dropped, and some applications are still 
using it), and I don't think it is friendly to application developers at all 
to have 2 flag days. Plus, it would mean that the KDE Frameworks and Qt major 
versions would get out of sync for the first time in KDE's history.

We should also learn from the past: Things not meant to take forever end up 
taking longer than expected anyway, and each time, we're stuck with the 
temporary kludges for much longer than initially planned:
* KDE 4 picked up delay after delay, and the long-lived 3.5 branch ended up 
becoming a mixed feature and bugfix branch (which IMHO is not necessarily a bad 
model, and which could even work for kdelibs 4.7, but it was still an 
exception).
* The Akonadi-based kdepim picked up delay after delay as well, and so first 
its merge was postponed release after release, and only small pieces merged 
each time, and then we got stuck with a long-lived 4.4 branch, including a 
temporary and incomplete Akonadi-based KAddressBook for which we were 
initially promised that it'd be much better in 4.5. And now kdepim 4.4 is 
already discontinued and many users still consider 4.7 too unstable for their 
use.
We must plan for major developments to take longer than the initial optimistic 
estimate.

Kevin Kofler


Re: No more release schedules.

2011-06-09 Thread Kevin Kofler
OK, since a lot of context apparently got lost during the message passing, let 
me just state my (personal) position clearly:

What I think is acceptable:
* Module X wants feature Y, which is non-invasive and well-tested and does not 
change the user experience nor the user interface in a significant way (for 
those not using the new feature), backported and released together with KDE SC 
4.n.x.

What I think is NOT acceptable:
* Module X wants to release a week before or after all the other modules.
* Every module wants to release at a different date.
* Module X wants to bundle its major feature release M.m.0, which introduces 
significant feature changes (maybe even feature regressions, new bugs etc.), 
together with the KDE SC bugfix release 4.n.x (x0). (We've made an exception 
for kdepim, but it should not be the rule.)
* KDE releases a new software compilation every month, with some modules 
releasing major releases and some modules releasing bugfix releases. (This is a 
problem for packagers because either we upgrade nothing and keep all the bugs, 
or we upgrade only the modules which released bugfix releases and end up with a 
unique combination nobody else is shipping, or we upgrade everything and get 
yelled at by the people who don't want feature updates pushed, who sadly have 
become dominant even in Fedora.)

(Please note that I do not speak for all packagers, nor even for all Fedora 
KDE packagers. They may or may not agree with the above.)

Kevin Kofler
 
 Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe 


Re: No more release schedules.

2011-06-09 Thread Kevin Kofler
On Thursday 09 June 2011, Eric Hameleers wrote:
 Andreas, how I agree!
 
 This now, is _exactly_ what I was afraid for when I voiced my concern
 about the break-up of this relatively small collection of coherent
 source tarballs we are used to work with, into a fragmented and
 potentially disconnected set of individual small (project-oriented)
 source tarballs. This would mean, KDE as an integrated software
 collection is dissolving into a loose collection of software perhaps
 not even branded KDE anymore.
 
 Notwithstanding the frameworks concept, which sounds appealing, you
 should focus on keeping the ecosystem together. Otherwise there will
 not be a KDE SC soon, but instead KDE for Slackware, KDE for
 Fedora, KDE for Arch, KDE for Windows, KDE for Solaris ... and every
 version will be unpredictably different from the others. This
 unpredictability kills the killer concept: that KDE transcends
 operating systems. I predict that it will end up like GNOME: one
 distro adds Gnome Shell, the next adds Unity, and yet another decides
 to stick to a forward-ported old version of the desktop manager. This
 surely is not what KDE (and its users!) deserve.

+1

My main concern with disparate releases would be that it would be impossible 
to test all the possible combinations properly. Every distribution would end 
up with its own combination of versions, with the potential for 
incompatibilities nobody else can reproduce. We're already seeing enough of 
those with libraries further down the stack which KDE (the project) has no 
direct control on, and we've had a couple with the separate kdepim releases 
too. Having KDE's own packages also released in an uncoordinated fashion would 
increase the amount of problems by a lot (e.g., in my experience, trying to 
use an older kdebase-workspace with a newer kdelibs would routinely just 
crash, even though kdelibs is supposed to be backwards-compatible; we've 
regularly had Rawhide broken by that if we didn't manage to build kdebase-
workspace quickly enough for whatever reason). Please let those separate 
kdepim releases be the exception (and get kdepim back in sync with the rest of 
the software compilation for 4.7), not the rule.

Another issue is that it would really swamp most distribution's KDE software 
packaging teams to have to package a new release of package XYZ every day.

Yes, freezes can be a problem getting features to our users quickly. But just 
letting every small module pick its own schedule surely cannot be the 
solution. (My personal suggestion would actually be to do relaxed freezes as 
in the 3.5 branch. Then let the module maintainer decide what is stable for a 
point release and what not, but not proclaim a major release at a random point 
in time. But that's just another suggestion thrown into the mix.)

I realize that this is ultimately the release team's decision, but please do 
listen to packagers' concerns.

Kevin Kofler
 
 Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe 


<    1   2