T11627: Improve KIO asynchronicity

[Andreas Hartmetz]

ahartmetz added a comment.


  In T11627#219720 , @meven wrote:
  
  > In T11627#219718 , @ahartmetz 
wrote:
  >
  > > You may want to  look at the DBus part of this with dfer-monitor, a tool 
I have written. It is especially useful for looking at latency problems.
  > >  https://cgit.kde.org/dferry.git/
  >
  >
  > It seriously lacks documentation, not even a README.
  >
  > I have been using Bustle for dbus monitoring.
  
  
  True, but here's the interesting part: You compile and install it, then you 
get a Qt application called dfer-analyzer (sorry I wrote the wrong thing, 
dfer-monitor is more like dbus-monitor) that is a nice way to look at DBus 
traffic.

TASK DETAIL
  https://phabricator.kde.org/T11627

To: meven, ahartmetz
Cc: ahartmetz, broulik, ognarb, #dolphin, #frameworks, meven, dfaure, 
pberestov, iasensio, fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, 
Codezela, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov

T11627: Improve KIO asynchronicity

[Andreas Hartmetz]

ahartmetz added a comment.


  You may want to  look at the DBus part of this with dfer-monitor, a tool I 
have written. It is especially useful for looking at latency problems.
  https://cgit.kde.org/dferry.git/

TASK DETAIL
  https://phabricator.kde.org/T11627

To: meven, ahartmetz
Cc: ahartmetz, broulik, ognarb, #dolphin, #frameworks, meven, dfaure, 
pberestov, iasensio, fprice, LeGast00n, MrPepe, fbampaloukas, alexde, GB_2, 
Codezela, feverfew, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov

Re: auto QString(Builder) considered VERY HARMFUL -> use clazy, especially before releases

[Andreas Hartmetz]

Am Freitag, 28. September 2018, 11:15:52 CEST schrieb Friedrich W. H. 
Kossebau:
> Am Freitag, 28. September 2018, 01:03:01 CEST schrieb Albert Astals 
Cid:
> > El dijous, 27 de setembre de 2018, a les 21:01:13 CEST, Friedrich W.
> > H.
> Kossebau va escriure:
> > > One would recommend to run clazy over your code at least before
> > > releases, to catch all kind of potential issues :)
> > 
> > Or since this is a crasher, just run your app and it'll crash?
> > 
> > Or even better, add autotests that exercise the code and they'll
> > crash too?
> s/Or/And/ ? :)
> 
> BTW, not necessarily a crasher, the references can point to random
> data which still can get interpreted into proper QString data, which
> will "only" deliver bogus string results (or by chance even "correct"
> one if it's still the old data at the used memory), but not trigger a
> crash, as no data is changed, as the reference is only read from. At
> least from what I remember to have seen.
> 
> ASan seems to help here though, I think I fixed at least one such bug
> due to ASan throwing up use-after-free or similar on KDE CI and thus
> pointing out the issue.
> 
To add to that, the most recent instance I fixed was from 2016 and was 
triggered by some new changes introducing (AFAICS) no fault of their 
own. They just shuffled things around enough to trigger the latent bug.

> Cheers
> Friedrich

auto QString(Builder) considered VERY HARMFUL

[Andreas Hartmetz]

Hello,

Today I fixed the third or so crash in KDE software due to the following 
pattern:

const auto str = someString + anotherString;

What happens is that the type of str ends up being QStringBuilder 
instead of QString. The QStringBuilder is destroyed after the semicolon, 
and all access to "str" produces undefined behavior.
While I'm not a particularly big fan of using auto to hide variable 
types anyway, this kind of usage is just wrong and must be avoided. 
Please take care.

Cheers,
Andreas

D14922: Assert if trying to use a KCatalog without a QCoreApplication

[Andreas Hartmetz]

ahartmetz added a subscriber: kwin.
ahartmetz added a comment.


  @kwin

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D14922

To: aacid, dfaure
Cc: kwin, ahartmetz, dfaure, ltoscano, ilic, kde-frameworks-devel, michaelh, 
ngraham, bruns

D14922: Assert if trying to use a KCatalog without a QCoreApplication

[Andreas Hartmetz]

ahartmetz added a comment.


  This change makes kwin_x11 crash on startup. Please fix kwin or degrade this 
to a warning.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D14922

To: aacid, dfaure
Cc: ahartmetz, dfaure, ltoscano, ilic, kde-frameworks-devel, michaelh, ngraham, 
bruns

Re: Fwd: KF5 CMake usage question

[Andreas Hartmetz]

On Samstag, 13. Mai 2017 23:48:33 CEST Shaheed Haque wrote:
> Hi,
> 
> On 13 May 2017 at 22:04, Sven Brauch  wrote:
> > Hi,
> > 
> > On 05/13/2017 06:06 PM, Shaheed Haque wrote:
> >> The printed output shows that the variable KF5KIO_INCLUDE_DIRS is
> >> not
> >> set. In poking around, I see references to a (new-to-me)
> >> target-based
> > 
> >> system, and using that like this:
> > The question is, why do you need to do that?
> 
The idea with the target based system aka "Modern CMake" is that you say 
you want to compile against a library, and that is all you have to do. A 
library requires you to add an include path for its own headers, include 
paths for headers of one of its dependencies, and link against a bunch 
of libraries? All covered by target properties.
If for some reason (e.g. handover to an external tool) you need those 
properties, you can still query them. Under enforced names nonetheless, 
unlike FOO_INCLUDE_DIR or was it FOO_INCLUDE_DIRS?

> I'm continuing to experiment with trying to build Python bindings for
> KF5. As part of that, the SIP tooling creates C++ wrapper code which
> must be compiled for each framework, and for that I need to know the
> header file directories. So far, I have simply been hardcoding the
> needed paths on my own system, but I now want to move to using
> standard CMake-based logic instead.
> 
> (In some sense, this might be seen as similar to the stuff that was
> added to ECM, but I'm trying for a significantly more automated
> approach).
> 
> Also, I am trying to feel my way towards a Pythonic build system for
> the KF5 bindings (as, roughly speaking, PyQt seems to be doing): in
> other words I'm interested in using CMake as a stepping stone, not the
> actual build system.
> 
I would recommend against that unless you really need to have heavy 
logic in the build system. A build system's main job is to "solve" a 
dependency tree - that is the difference between a build system and a 
script that runs the compiler. The dependency tree enables cheap 
incremental builds and correct parallel builds. Maybe not that important 
for bindings, admittedly.
Your advantage from using Python must be larger than the overhead from 
doing your own dependency resolution plus the overhead from the CMake-
Python interfacing plus the build-related facilities that CMake has and 
Python doesn't. Or were you considering scons?
PyQt may have chosen Python because qmake sucks, and it needs Python 
anyway, so it avoids any extra dependencies. I know from experience that 
you really want to avoid extra dependencies in commercial products.

> Thus, I'm after the moral equivalents of:
> 
> Foo_INCLUDE_DIRS
> Foo_COMPILE_FLAGS
> 
> Thanks, Shaheed
> 
> > The usual way is to simply call
> > 
> >   target_link_libraries(mybinary KF5::KIOCore)
> > 
> > and include paths etc. will be set up for your target automatically.
> > 
> > Best,
> > Sven

Cheers,
Andreas

[Differential] [Updated] D4060: kio: Ensure user certificate directory has been created before storing certs to it

[Andreas Hartmetz]

ahartmetz added a comment.


  Looks like a stupid mistake on my part, thanks for the fix.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D4060

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mpyne, #frameworks, apol, ahartmetz

Re: Review Request 129259: Fix the buffersize in certain situations.

[Andreas Hartmetz]

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


Ship it!




Taro, do you have push rights? Otherwise I or somebody else can push it for you.

- Andreas Hartmetz


On Oct. 27, 2016, 3:02 p.m., taro yamada wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129259/
> ---
> 
> (Updated Oct. 27, 2016, 3:02 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 369275
> https://bugs.kde.org/show_bug.cgi?id=369275
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently, KIO uses lstat to get the buffersize for readlink.
> But in certain situations, it returns inappropriate value.
> 
> For example, "/proc/self" or "/sys/bus/cpu/devices/*" returns its size is 0 , 
> and then readlink fails with EINVAL.(so link won't be shown in kde 
> application.)
> TMSU seems it returns its target's actual filesize insted of the link's 
> filesize itself.
> 
> This patch changes the buffersize to 1024 bytes if it is 0.
> And later truncate it to actual size.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/file/file.cpp 8b17d31 
> 
> Diff: https://git.reviewboard.kde.org/r/129259/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> taro yamada
> 
>

Re: Review Request 129259: Fix the buffersize in certain situations.

[Andreas Hartmetz]

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


Ship it!




In any case this is good enough already.


src/ioslaves/file/file.cpp (line 791)
<https://git.reviewboard.kde.org/r/129259/#comment67346>

You can use qMin() here. As is the compiler would complain about the 
different types (off_t and int), which can be fixed by changing the type of 
initialLimit to off_t.


- Andreas Hartmetz


On Oct. 27, 2016, 4:58 a.m., taro yamada wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129259/
> ---
> 
> (Updated Oct. 27, 2016, 4:58 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 369275
> https://bugs.kde.org/show_bug.cgi?id=369275
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently, KIO uses lstat to get the buffersize for readlink.
> But in certain situations, it returns inappropriate value.
> 
> For example, "/proc/self" or "/sys/bus/cpu/devices/*" returns its size is 0 , 
> and then readlink fails with EINVAL.(so link won't be shown in kde 
> application.)
> TMSU seems it returns its target's actual filesize insted of the link's 
> filesize itself.
> 
> This patch changes the buffersize to 1024 bytes if it is 0.
> And later truncate it to actual size.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/file/file.cpp 8b17d31 
> 
> Diff: https://git.reviewboard.kde.org/r/129259/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> taro yamada
> 
>

Re: Review Request 129259: Fix the buffersize in certain situations.

[Andreas Hartmetz]

> On Oct. 26, 2016, 2:48 p.m., Andreas Hartmetz wrote:
> > If a workaround was really necessary I'd agree with Jonathan. But it looks 
> > like a workaround to fix up st_size is not necessary, because both Krusader 
> > and ls (just two examples I could easily find) work without changing 
> > st_size. Note that they use a different way to check whether the current 
> > file is a symlink, not sure if that makes the difference though.
> > https://quickgit.kde.org/?p=krusader.git=blob=932340774d439b1ea770204903c1f8c431dc86db=f1660b241d195a31a904a1706fb8022b99e4c709=krusader%2FVFS%2Fnormal_vfs.cpp
> > http://git.savannah.gnu.org/cgit/coreutils.git/tree/src/ls.c
> 
> Jonathan Doman wrote:
> Actually, take a look at the readlink implementation that ls calls: 
> http://git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob;f=lib/areadlink-with-size.c;hb=HEAD
> 
> It does almost exactly what is proposed by this review, for the same 
> reasons ("Some buggy file systems report garbage in st_size").

Great find, thanks! Whatever ls does should be fine, because everybody who 
writes a file system is going to notice when it doesn't work in ls.


- Andreas


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


On Oct. 25, 2016, 2:51 p.m., taro yamada wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129259/
> ---
> 
> (Updated Oct. 25, 2016, 2:51 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 369275
> https://bugs.kde.org/show_bug.cgi?id=369275
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently, KIO uses lstat to get the buffersize for readlink.
> But in certain situations, it returns inappropriate value.
> 
> For example, "/proc/self" or "/sys/bus/cpu/devices/*" returns its size is 0 , 
> and then readlink fails with EINVAL.(so link won't be shown in kde 
> application.)
> TMSU seems it returns its target's actual filesize insted of the link's 
> filesize itself.
> 
> This patch changes the buffersize to 1024 bytes if it is 0.
> And later truncate it to actual size.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/file/file.cpp 8b17d31 
> 
> Diff: https://git.reviewboard.kde.org/r/129259/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> taro yamada
> 
>

Re: Review Request 129259: Fix the buffersize in certain situations.

[Andreas Hartmetz]

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



If a workaround was really necessary I'd agree with Jonathan. But it looks like 
a workaround to fix up st_size is not necessary, because both Krusader and ls 
(just two examples I could easily find) work without changing st_size. Note 
that they use a different way to check whether the current file is a symlink, 
not sure if that makes the difference though.
https://quickgit.kde.org/?p=krusader.git=blob=932340774d439b1ea770204903c1f8c431dc86db=f1660b241d195a31a904a1706fb8022b99e4c709=krusader%2FVFS%2Fnormal_vfs.cpp
http://git.savannah.gnu.org/cgit/coreutils.git/tree/src/ls.c

- Andreas Hartmetz


On Oct. 25, 2016, 2:51 p.m., taro yamada wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129259/
> ---
> 
> (Updated Oct. 25, 2016, 2:51 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 369275
> https://bugs.kde.org/show_bug.cgi?id=369275
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Currently, KIO uses lstat to get the buffersize for readlink.
> But in certain situations, it returns inappropriate value.
> 
> For example, "/proc/self" or "/sys/bus/cpu/devices/*" returns its size is 0 , 
> and then readlink fails with EINVAL.(so link won't be shown in kde 
> application.)
> TMSU seems it returns its target's actual filesize insted of the link's 
> filesize itself.
> 
> This patch changes the buffersize to 1024 bytes if it is 0.
> And later truncate it to actual size.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/file/file.cpp 8b17d31 
> 
> Diff: https://git.reviewboard.kde.org/r/129259/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> taro yamada
> 
>

Re: Scrap baloo?

[Andreas Hartmetz]

On Mittwoch, 28. September 2016 20:42:37 CEST Boudhayan Gupta wrote:
> Hi,
> 
> On 28 September 2016 at 20:33, Christoph Cullmann 
 wrote:
> > Hi,
> > 
> >> Hi,
> >> 
> >> On 28 September 2016 at 02:36, Christoph Cullmann 
 wrote:
> >>> any update?
> >> 
> >> Yep. In all the happennings of the week I just forgot to write this
> >> email.
> >> 
> >> If Baloo is going to be an integral part of the Plasma experience,
> >> do
> >> we really want to depend on an external project where we don't have
> >> control (and indeed, sentiments may prevent unrestricted
> >> contributions based only on merit). This is the political reason
> >> why I don't want to depend against Tracker. The technical reason
> >> is that it's based on SQLite, which is incredibly slow compared to
> >> what we do now.> 
> > I don't see really that it is slow compared to what we do, if you
> > have benchmarks for that, I would be pleased to see them.
> 
> So would I. You already have Tracker based code, could you spare some
> time and run some?
> 
> >> At the same time, LMDB needs to be replaced, and fast. I'm building
> >> a
> >> new KVDB as an university project (it should be able to do 256GB
> >> indexes on 32bit machines), and if that doesn't work out there's
> >> Sophia (http://sophia.systems/). I'll be evaluating both as a
> >> replacement to LMDB.
> > 
Any particular reason why you didn't consider leveldb? When I did 
research about databases for a work project (the application was using 
SQL / SQLite but didn't really need to), lmdb and leveldb were the clear 
leaders. 

Crash / power loss safety is also worth talking about - lmdb and SQLite 
are among the best of the embedded databases. leveldb is theoretically 
great because it only ever appends (and rewrites complete files and then 
replaces them), but file systems together with disks that say they wrote 
when they didn't make that somewhat susceptible to data loss.
Here is the most concise summary I could find - the same authors also 
have much more in-depth presentations, papers and other writeups.
http://docplayer.net/282512-Application-level-crash-consistency.html

> > Do we really want to maintain a own DB system?
> > IMHO that will never work out, all DB systems around need more
> > maintenance power than we have.
> 
> This is something I'm not sure about. The DB will be build anyway, my
> graduation depends on it :D And if I'm going to do something I will do
> it well, so it'll be simple and clean.
> 
> If it doesn't work out, there's always Sophia to fall back on.
> 
> >> Vishesh also wanted to separate out the engine and make it public
> >> API
> >> (apparently other projects want to make use of it as a general data
> >> storage library - and the engine offers fulltext search
> >> capabilities
> >> and other fancy logical operators that make it particularly
> >> attractive. My plan is to move towards that, and eventually also
> >> not
> >> only index files but also other kinds of objects - contacts, or
> >> people, for example.
> >> 
> >> I don't want to move back into the "semantic desktop" idea at all,
> >> but I do want some sort of infrastructure that allows for an
> >> "action on object" metaphor - file objects can be opened with an
> >> application, people objects can be sent mails, and so on.
> >> 
> >> Hope this makes sense.
> > 
> > I still not see how that should work out, atm, IMHO facts are:
> > 
> > 1) baloo is not maintained
> 
> It will, now.
> 
> > 2) lmdb will e.g. never work for us on NFS homes and the code needs
> > major overhaul to handle errors (which you confirm)
> 
> LMDB goes away, either way.
> 
> > 3) you said you have "some time" left to maintain it, but you now
> > propose in addition to maintain Baloo to write a DB system from
> > scratch, I don't really see that working
> I have a personal interest, an academic interest, and now a
> KDE-related interest in the KVDB. It *will* work, because I'm the kind
> of guy who puts a lot of time and effort into things (maybe even
> disproportionately so) into things that genuinely interest me. My
> challenge will be to make the codebase so that after I'm done with
> this (say in about 5 years or so) it'll be comprehensible to the next
> maintainer.
> 
> > 4) tracker on the other side is maintained and in use and we can
> > share the index data with GNOME and others
> > 
> > I really doubt that doing the work to remove lmdb, replace it with
> > an "own one" and then starting to fix all other issues (like
> > indexer running amok, broken file extractors, ...) will work out if
> > we don't clone some more people.
> > 
> > But that is only my opinion.
> 
> *Sigh*
> 
> I don't want to take the easy way out here. Half the fun in KDE is
> doing crazy things and seeing your baby work. That's the entire
> motivation for being here.
> 
> And right now I'm volunteering to do this.
> 
> -- Boudhayan

Re: The curious case of stuck systemd poweroff

[Andreas Hartmetz]

On Donnerstag, 14. Juli 2016 16:19:15 CEST Harald Sitter wrote:
> On Thu, Jul 14, 2016 at 3:43 PM, Andreas Hartmetz 
<ahartm...@gmail.com> wrote:
> > Hello,
> > 
> > Am Donnerstag, 14. Juli 2016, 11:11:26 CEST schrieb Harald Sitter:
> >> Hola!
> >> 
> >> ever since systemd and or sddm started not killing all our session
> >> processes we have had problems of poweroff/reboot getting hung up
> >> waiting for processes to quit.
> >> Recently systemd then started sending them TERM by default, which
> >> in
> >> theory should make things behave as before, but more often than not
> >> it doesn't.
> >> 
> >> The reason for this is meh to debug and altogether somewhat
> >> convoluted. So all that follows was partially inferred from
> >> numerous
> >> logging attempts.
> >> They all root in a simple fact: ksmserver is rubbish at its job and
> >> only terminates half the stuff in the session before handing over
> >> to
> >> the outside expecting the outside to deal with it.
> >> 
> >> I found two likely holdup scenarios caused by this:
> >> 
> >> a) procfoo is still running -> ksmserver hands over to systemd ->
> >> systemd stops sddm -> xserver stops -> procfoo now crashes because
> >> it
> >> does x-things (pretty sure [1] is an instance of this) -> kcrash
> >> jumps in -> drkonqi -> gdb -> procfoo wont react to anything but
> >> KILL now> 
> > Hah, that's a nice one. It should indeed be fixed in kcrash.
> > 
> >> b) procfoo is still running -> ksmserver hands over to systemd ->
> >> procfoo survives without X (e.g. kio slave) -> procfoo crashes for
> >> (maybe unreleated) reasons such as qt bug because network is down
> >> ->
> >> kcrash gets hung up on recursion crashes handling for kdeinit5 or
> >> some other nonesense
> > 
> > It is not even clear that surviving processes need to be killed in
> > case of logout, which one also needs to consider. See below.
> > 
> >> Long story short: if things crash, usually the TERM from systemd
> >> won't do anything.
> >> 
> >> The way I see it ksmserver needs to properly TERM everything to
> >> protect against a). Kcrash additionally ought to not do anything
> >> when
> >> its session is in shutdown to guard against both a) and b) AND
> >> allow
> >> core dumps to be collected instead so there actually can be a trace
> >> of something having gone wong.
> > 
> > It is not really ksmserver's job to SIGTERM or even SIGKILL
> > applications. It implements XSMP which involves asking application
> > nicely to die. If they didn't, they were killed just fine until
> > systemd "improved" things.
> > Not everything participates in XSMP so ksmserver doesn't see all
> > processes in any case.
> > What systemd ought to do is:
> > - when shutting down, kill everything after a short timeout
> > - when logging out, don't kill anything (think of screen sessions
> > and
> > 
> >   such)
> > 
> > This is a systemd problem. Before systemd it worked as described
> > above and it was good.
> > 
> >> Thoughts?
> >> 
> >> I have no clue how we'd implement kcrash changes since that would
> >> have to somehow know if the session is active without doing
> >> business on the heap. For ksmserver we could probably lean on
> >> systemd to give a proc list of the session.
> > 
> > So that would mean adding code on our side and integrating deeper
> > with systemd to unbreak systemd behavior. I think systemd should do
> > its job properly and get out of the way.
> 
> so no useful input then. ok.

The hell are you talking about? The action items are:
- Disable kcrash during logout
- File upstream bug in systemd to stop with its ill-advised behavior

Re: The curious case of stuck systemd poweroff

[Andreas Hartmetz]

Hello,

Am Donnerstag, 14. Juli 2016, 11:11:26 CEST schrieb Harald Sitter:
> Hola!
> 
> ever since systemd and or sddm started not killing all our session
> processes we have had problems of poweroff/reboot getting hung up
> waiting for processes to quit.
> Recently systemd then started sending them TERM by default, which in
> theory should make things behave as before, but more often than not it
> doesn't.
> 
> The reason for this is meh to debug and altogether somewhat
> convoluted. So all that follows was partially inferred from numerous
> logging attempts.
> They all root in a simple fact: ksmserver is rubbish at its job and
> only terminates half the stuff in the session before handing over to
> the outside expecting the outside to deal with it.
> 
> I found two likely holdup scenarios caused by this:
> 
> a) procfoo is still running -> ksmserver hands over to systemd ->
> systemd stops sddm -> xserver stops -> procfoo now crashes because it
> does x-things (pretty sure [1] is an instance of this) -> kcrash jumps
> in -> drkonqi -> gdb -> procfoo wont react to anything but KILL now
>
Hah, that's a nice one. It should indeed be fixed in kcrash.
 
> b) procfoo is still running -> ksmserver hands over to systemd ->
> procfoo survives without X (e.g. kio slave) -> procfoo crashes for
> (maybe unreleated) reasons such as qt bug because network is down ->
> kcrash gets hung up on recursion crashes handling for kdeinit5 or some
> other nonesense
> 
It is not even clear that surviving processes need to be killed in case 
of logout, which one also needs to consider. See below.

> Long story short: if things crash, usually the TERM from systemd won't
> do anything.
> 
> The way I see it ksmserver needs to properly TERM everything to
> protect against a). Kcrash additionally ought to not do anything when
> its session is in shutdown to guard against both a) and b) AND allow
> core dumps to be collected instead so there actually can be a trace of
> something having gone wong.
> 
It is not really ksmserver's job to SIGTERM or even SIGKILL 
applications. It implements XSMP which involves asking application 
nicely to die. If they didn't, they were killed just fine until systemd 
"improved" things.
Not everything participates in XSMP so ksmserver doesn't see all 
processes in any case.
What systemd ought to do is:
- when shutting down, kill everything after a short timeout
- when logging out, don't kill anything (think of screen sessions and
  such)

This is a systemd problem. Before systemd it worked as described above 
and it was good.

> Thoughts?
> 
> I have no clue how we'd implement kcrash changes since that would have
> to somehow know if the session is active without doing business on
> the heap. For ksmserver we could probably lean on systemd to give a
> proc list of the session.
> 
So that would mean adding code on our side and integrating deeper with 
systemd to unbreak systemd behavior. I think systemd should do its job 
properly and get out of the way.

> [1] https://bugs.kde.org/show_bug.cgi?id=364340

Cheers,
Andreas

Re: Review Request 127813: Process paths just once

[Andreas Hartmetz]

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




src/core/kconfiggroup.cpp (line 439)
<https://git.reviewboard.kde.org/r/127813/#comment64595>

Talking about same behavior, this also randomizes the order in case there 
are actually several home paths... however that happens.


- Andreas Hartmetz


On May 2, 2016, 4:32 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127813/
> ---
> 
> (Updated May 2, 2016, 4:32 p.m.)
> 
> 
> Review request for KDE Frameworks and Matthew Dawson.
> 
> 
> Repository: kconfig
> 
> 
> Description
> ---
> 
> Just process every home path once, as the 3 alternatives will probably just 
> be the same.
> Don't drop the file: prefix twice in every run.
> 
> 
> Diffs
> -
> 
>   src/core/kconfiggroup.cpp 39d2441 
> 
> Diff: https://git.reviewboard.kde.org/r/127813/diff/
> 
> 
> Testing
> ---
> 
> Tests pass, apps work.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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

Re: Misbehavior when unloading KStyle

[Andreas Hartmetz]

On Freitag, 8. April 2016 17:25:06 CEST Aleix Pol wrote:
> On Fri, Apr 8, 2016 at 3:42 PM, Andreas Hartmetz <ahartm...@gmail.com> 
wrote:
> > On Donnerstag, 7. April 2016 20:33:19 CEST Andreas Hartmetz wrote:
> >> On Mittwoch, 6. April 2016 20:10:53 CEST Aleix Pol wrote:
> >> > Hi,
> >> > I've seen a couple of times such backtrace [1] which shows Qt
> >> > waiting
> >> > for something that never happens. An easy way to reproduce is to
> >> > load
> >> > konversation (or any application with KDBusService::Unique), hide
> >> > it
> >> > and run it again. The exit call will never be fulfilled in the
> >> > second
> >> > process.
> >> > 
> >> > Any idea of what could be the cause of this? Or why it's
> >> > happening?
> >> > 
> >> > Regards,
> >> > Aleix
> >> > 
> >> > [1] https://paste.kde.org/pstps66fj
> >> > [2] (gdb) thread 2
> >> > [Switching to thread 2 (Thread 0x7fffe180f700 (LWP 1))]
> >> > #0  0x708a3c3d in poll () from /usr/lib/libc.so.6
> >> > (gdb) where
> >> > #0  0x708a3c3d in poll () from /usr/lib/libc.so.6
> >> > #1  0x7fffedb4fae2 in ?? () from /usr/lib/libxcb.so.1
> >> > #2  0x7fffedb51757 in xcb_wait_for_event () from
> >> > /usr/lib/libxcb.so.1 #3  0x7fffe513b269 in
> >> > QXcbEventReader::run
> >> > (this=0xe54730) at
> >> > /home/apol/devel/frameworks/qt5/qtbase/src/plugins/platforms/xcb/
> >> > qxc
> >> > b
> >> > connection.cpp:1321 #4  0x714ae2f9 in
> >> > QThreadPrivate::start
> >> > (arg=0xe54730) at
> >> > /home/apol/devel/frameworks/qt5/qtbase/src/corelib/thread/qthread
> >> > _un
> >> > i
> >> > x.cpp:340 #5  0x7fffee827424 in start_thread () from
> >> > /usr/lib/libpthread.so.0 #6  0x708accbd in clone () from
> >> > /usr/lib/libc.so.6
> >> > ___
> >> > Kde-frameworks-devel mailing list
> >> > Kde-frameworks-devel@kde.org
> >> > https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
> >> 
> >> I've seen this happening as well. It looks it's triggered by a
> >> certain destruction order of global statics (as in
> >> Q_GLOBAL_STATIC) and involves a QObject::destroyed() (not sure
> >> about that one) signal sent to the (new in 5.6) DBus thread (aka
> >> QDBusConnectionManager) which is moveToThread(Q_NULLPTR)-ed into
> >> some kind of limbo state when shutting down because its global
> >> static container is destroyed, where it stops event processing.
> >> The connection is a BlockingQueuedConnection, so it hangs
> >> indefinitely.
> >> If you have your environment set up in a way that Qt applications
> >> use
> >> a KStyle, you only need a main function consisting of
> >> 
> >> QApplication(argc, argv);
> >> exit(EXIT_SUCCESS /* doesn't matter */);
> >> 
> >> to trigger it.
> >> Note: you should not exit() with a QApplication around, it's asking
> >> for problems anyway. kactivitmanagerd does it extensively and hangs
> >> very reliably currently... I have a request to change this on
> >> Phabricator. Note 2: you should NEVER try to handle signals by
> >> calling
> >> QApplication::quit() to exit cleanly. That is very much not
> >> something
> >> that's safe to do in a signal context. I have seen this in two
> >> different KDE daemons so far.
> >> 
> >> I've talked to Thiago about the apparent Qt bug. He said he'd get
> >> to
> >> it next week and so I haven't filed a bug yet. I'm still planning
> >> to
> >> do that.
> > 
> > https://bugreports.qt.io/browse/QTBUG-52473
> > 
> > Good place to add any additional useful information :)
> 
> The bug report Martin pointed out leads here:
> https://bugreports.qt.io/browse/QTBUG-51648
> 
> Is it the same?
> 
Oh yes, indeed, and it has been fixed by now, too.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Misbehavior when unloading KStyle

[Andreas Hartmetz]

On Donnerstag, 7. April 2016 20:33:19 CEST Andreas Hartmetz wrote:
> On Mittwoch, 6. April 2016 20:10:53 CEST Aleix Pol wrote:
> > Hi,
> > I've seen a couple of times such backtrace [1] which shows Qt
> > waiting
> > for something that never happens. An easy way to reproduce is to
> > load
> > konversation (or any application with KDBusService::Unique), hide it
> > and run it again. The exit call will never be fulfilled in the
> > second
> > process.
> > 
> > Any idea of what could be the cause of this? Or why it's happening?
> > 
> > Regards,
> > Aleix
> > 
> > [1] https://paste.kde.org/pstps66fj
> > [2] (gdb) thread 2
> > [Switching to thread 2 (Thread 0x7fffe180f700 (LWP 1))]
> > #0  0x708a3c3d in poll () from /usr/lib/libc.so.6
> > (gdb) where
> > #0  0x708a3c3d in poll () from /usr/lib/libc.so.6
> > #1  0x7fffedb4fae2 in ?? () from /usr/lib/libxcb.so.1
> > #2  0x7fffedb51757 in xcb_wait_for_event () from
> > /usr/lib/libxcb.so.1 #3  0x7fffe513b269 in QXcbEventReader::run
> > (this=0xe54730) at
> > /home/apol/devel/frameworks/qt5/qtbase/src/plugins/platforms/xcb/qxc
> > b
> > connection.cpp:1321 #4  0x714ae2f9 in QThreadPrivate::start
> > (arg=0xe54730) at
> > /home/apol/devel/frameworks/qt5/qtbase/src/corelib/thread/qthread_un
> > i
> > x.cpp:340 #5  0x7fffee827424 in start_thread () from
> > /usr/lib/libpthread.so.0 #6  0x708accbd in clone () from
> > /usr/lib/libc.so.6
> > ___
> > Kde-frameworks-devel mailing list
> > Kde-frameworks-devel@kde.org
> > https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
> 
> I've seen this happening as well. It looks it's triggered by a certain
> destruction order of global statics (as in Q_GLOBAL_STATIC) and
> involves a QObject::destroyed() (not sure about that one) signal sent
> to the (new in 5.6) DBus thread (aka QDBusConnectionManager) which is
> moveToThread(Q_NULLPTR)-ed into some kind of limbo state when shutting
> down because its global static container is destroyed, where it stops
> event processing. The connection is a BlockingQueuedConnection, so it
> hangs indefinitely.
> If you have your environment set up in a way that Qt applications use
> a KStyle, you only need a main function consisting of
> 
> QApplication(argc, argv);
> exit(EXIT_SUCCESS /* doesn't matter */);
> 
> to trigger it.
> Note: you should not exit() with a QApplication around, it's asking
> for problems anyway. kactivitmanagerd does it extensively and hangs
> very reliably currently... I have a request to change this on
> Phabricator. Note 2: you should NEVER try to handle signals by
> calling
> QApplication::quit() to exit cleanly. That is very much not something
> that's safe to do in a signal context. I have seen this in two
> different KDE daemons so far.
> 
> I've talked to Thiago about the apparent Qt bug. He said he'd get to
> it next week and so I haven't filed a bug yet. I'm still planning to
> do that.
> 

https://bugreports.qt.io/browse/QTBUG-52473

Good place to add any additional useful information :)
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Misbehavior when unloading KStyle

[Andreas Hartmetz]

On Mittwoch, 6. April 2016 20:10:53 CEST Aleix Pol wrote:
> Hi,
> I've seen a couple of times such backtrace [1] which shows Qt waiting
> for something that never happens. An easy way to reproduce is to load
> konversation (or any application with KDBusService::Unique), hide it
> and run it again. The exit call will never be fulfilled in the second
> process.
> 
> Any idea of what could be the cause of this? Or why it's happening?
> 
> Regards,
> Aleix
> 
> [1] https://paste.kde.org/pstps66fj
> [2] (gdb) thread 2
> [Switching to thread 2 (Thread 0x7fffe180f700 (LWP 1))]
> #0  0x708a3c3d in poll () from /usr/lib/libc.so.6
> (gdb) where
> #0  0x708a3c3d in poll () from /usr/lib/libc.so.6
> #1  0x7fffedb4fae2 in ?? () from /usr/lib/libxcb.so.1
> #2  0x7fffedb51757 in xcb_wait_for_event () from
> /usr/lib/libxcb.so.1 #3  0x7fffe513b269 in QXcbEventReader::run
> (this=0xe54730) at
> /home/apol/devel/frameworks/qt5/qtbase/src/plugins/platforms/xcb/qxcb
> connection.cpp:1321 #4  0x714ae2f9 in QThreadPrivate::start
> (arg=0xe54730) at
> /home/apol/devel/frameworks/qt5/qtbase/src/corelib/thread/qthread_uni
> x.cpp:340 #5  0x7fffee827424 in start_thread () from
> /usr/lib/libpthread.so.0 #6  0x708accbd in clone () from
> /usr/lib/libc.so.6
> ___
> Kde-frameworks-devel mailing list
> Kde-frameworks-devel@kde.org
> https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

I've seen this happening as well. It looks it's triggered by a certain 
destruction order of global statics (as in Q_GLOBAL_STATIC) and involves 
a QObject::destroyed() (not sure about that one) signal sent to the (new 
in 5.6) DBus thread (aka QDBusConnectionManager) which is 
moveToThread(Q_NULLPTR)-ed into some kind of limbo state when shutting 
down because its global static container is destroyed, where it stops 
event processing. The connection is a BlockingQueuedConnection, so it 
hangs indefinitely.
If you have your environment set up in a way that Qt applications use a 
KStyle, you only need a main function consisting of

QApplication(argc, argv);
exit(EXIT_SUCCESS /* doesn't matter */);

to trigger it.
Note: you should not exit() with a QApplication around, it's asking for 
problems anyway. kactivitmanagerd does it extensively and hangs very 
reliably currently... I have a request to change this on Phabricator.
Note 2: you should NEVER try to handle signals by calling 
QApplication::quit() to exit cleanly. That is very much not something 
that's safe to do in a signal context. I have seen this in two different 
KDE daemons so far.

I've talked to Thiago about the apparent Qt bug. He said he'd get to it 
next week and so I haven't filed a bug yet. I'm still planning to do 
that.

Cheers,
Andreas

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

Re: Review Request 127501: Improve TCPSlaveBase::isConnected

[Andreas Hartmetz]

> On March 27, 2016, 10:03 a.m., David Faure wrote:
> > Given how the socket API works, you should only call error() after a call 
> > that returns false (e.g. waitForConnected, etc.). As you found out, calling 
> > error() at random points in time doesn't give useful information, you get 
> > the last error that was ever set, even after 10 successful calls following 
> > the call that had an error.
> > 
> > Calling setError ourselves is a workaround, which doesn't fit with the Qt 
> > socket API (there is no "No error" error code).
> > UnknownSocketError *means* error, it doesn't mean no error.
> > 
> > It seems to me (from the description, I didn't read the code attentively) 
> > that the right solution is to actually disconnect after a 
> > RemoteHostClosedError. Then the state won't be ConnectedState anymore, and 
> > isConnected() will work as intended :-)
> 
> Albert Astals Cid wrote:
> But it won't, even if you disconnect, the error will still be 
> RemoteHostClosedError given that Qt does not reset the error after 
> disconnectFromHost (since there's basically no way to mark "no error"), so if 
> we reconnect after disconnection we will still have RemoteHostClosedError if 
> we ask for the error even if it's not true for this session. Of you mean this 
> in connection with the above hack?

Yeah, what happens here is working around two API mistakes in QTcpSocket: that 
there is no proper "no error" code, and that the error isn't cleared when 
starting a new connection. Apart from settings that may reasonably be 
persistent like e.g. proxy or buffer size settings, a new connection should 
behave exactly the same as deleting and recreating the socket. Otherwise the 
API effectively requires you to delete and recreate a socket to get a clean 
state, which isn't very nice in such a widely used API. (I sometimes do this in 
internal interfaces, it's a nice technique to avoid errors in reset and clean 
up code paths - but it causes a little overhead and more work for consumers)
Since the check here is for a specific error code, UnknownError works as a code 
that isn't that specific one. And if we hack the error code reset, we have what 
we need. I consider that self defense against bad API, not a real hack.


- Andreas


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


On March 26, 2016, 5:29 p.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127501/
> ---
> 
> (Updated March 26, 2016, 5:29 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Qt sockets returns ConnectedState even when error is set to 
> RemoteHostClosedError after a write operation.
> 
> So make ::isConnected also check for RemoteHostClosedError.
> 
> This has the consequence that we have to create/delete the socket since Qt 
> sockets have no way to reset their error state so if we want to reuse the 
> slave to connect again it will never work with the same socket since it will 
> always say RemoteHostClosedError.
> 
> I need this for https://git.reviewboard.kde.org/r/127502/
> 
> 
> Diffs
> -
> 
>   src/core/tcpslavebase.cpp b9be69d 
> 
> Diff: https://git.reviewboard.kde.org/r/127501/diff/
> 
> 
> Testing
> ---
> 
> Seesms to work fine and the pop3 bugfix i have also works fine.
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

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

Re: Review Request 127501: Improve TCPSlaveBase::isConnected

[Andreas Hartmetz]

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



Ow. That's a pretty bad problem.
I suggest a less intrusive (code churn and regression potential) solution: 
Since QAbstractSocket *does* have setError() as a protected method, you can 
still call it:

// never instantiated
class QAbstractSocketHack: public QAbstractSocket {
public:
void setError(SocketError e) Q_DECL_OVERRIDE { 
QAbstractSocket::setError(e); }
};

// in socket reset code
static_cast(>socket)->setError(QAbstractSocket::UnknownSocketError);

- Andreas Hartmetz


On March 26, 2016, 5:29 p.m., Albert Astals Cid wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127501/
> ---
> 
> (Updated March 26, 2016, 5:29 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> Qt sockets returns ConnectedState even when error is set to 
> RemoteHostClosedError after a write operation.
> 
> So make ::isConnected also check for RemoteHostClosedError.
> 
> This has the consequence that we have to create/delete the socket since Qt 
> sockets have no way to reset their error state so if we want to reuse the 
> slave to connect again it will never work with the same socket since it will 
> always say RemoteHostClosedError.
> 
> I need this for https://git.reviewboard.kde.org/r/127502/
> 
> 
> Diffs
> -
> 
>   src/core/tcpslavebase.cpp b9be69d 
> 
> Diff: https://git.reviewboard.kde.org/r/127501/diff/
> 
> 
> Testing
> ---
> 
> Seesms to work fine and the pop3 bugfix i have also works fine.
> 
> 
> Thanks,
> 
> Albert Astals Cid
> 
>

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

Re: Review Request 126991: Try multiple authentication methods in case of failures

[Andreas Hartmetz]

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


Ship it!




Ship It!

- Andreas Hartmetz


On March 3, 2016, 9:01 p.m., Krzysztof Nowicki wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126991/
> ---
> 
> (Updated March 3, 2016, 9:01 p.m.)
> 
> 
> Review request for KDE Frameworks and Dawit Alemayehu.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> When authenticating agains a server offering multiple authentication methods 
> make sure to attempt other methods in case the best one fails.
> 
> This also fixes a connection close issue in the middle of an NTLM 
> authentication dialog due to clearing the password.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/http/http.h 621b2c7a957b9bc9cc14ff13ed3c3a72dec38190 
>   src/ioslaves/http/http.cpp a84129f1403cbf8b0f86f9fd0354bec90ac5fd39 
> 
> Diff: https://git.reviewboard.kde.org/r/126991/diff/
> 
> 
> Testing
> ---
> 
> I have performed testing on an IIS 7.5 server which offered 3 authentication 
> options: Negotiate, NTLM and Basic. Since I have Kerberos configured the 
> original code would only try Negotiate and because it failed it would retry 
> it endlessly. With this patch authentication correctly falls back to NTLM or 
> Basic (if NTLM fails too).
> 
> 
> Thanks,
> 
> Krzysztof Nowicki
> 
>

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

Re: Review Request 126991: Try multiple authentication methods in case of failures

[Andreas Hartmetz]

> On March 3, 2016, 7:43 p.m., Andreas Hartmetz wrote:
> >
> 
> Andreas Hartmetz wrote:
> OK, this looks good now overall. The comment about the indexOf() is of 
> course about the whole truncating at space thing, not the particular line.
> 
> Krzysztof Nowicki wrote:
> The idea of this line and the conditional behind it is to retrieve the 
> auth method name. Sometimes this will be just the name keyword (for example: 
> 'NTLM'), but sometimes this will be followed by some optional data (for ex. 
> 'Basic realm="Some site"'). The `indexOf()` will return the index of the 
> space in the latter case, in which the additional data along with the space 
> needs to be removed. This is of cource not needed for the first case.
> 
> I assume a short comment above the mentioned line would be sufficient?

Of course. Just something like "separate method name from additional auth info, 
e.g. nonce value" (or whatever else is a good example).


- Andreas


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


On March 3, 2016, 11:40 a.m., Krzysztof Nowicki wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126991/
> ---
> 
> (Updated March 3, 2016, 11:40 a.m.)
> 
> 
> Review request for KDE Frameworks and Dawit Alemayehu.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> When authenticating agains a server offering multiple authentication methods 
> make sure to attempt other methods in case the best one fails.
> 
> This also fixes a connection close issue in the middle of an NTLM 
> authentication dialog due to clearing the password.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/http/http.h 621b2c7a957b9bc9cc14ff13ed3c3a72dec38190 
>   src/ioslaves/http/http.cpp a84129f1403cbf8b0f86f9fd0354bec90ac5fd39 
> 
> Diff: https://git.reviewboard.kde.org/r/126991/diff/
> 
> 
> Testing
> ---
> 
> I have performed testing on an IIS 7.5 server which offered 3 authentication 
> options: Negotiate, NTLM and Basic. Since I have Kerberos configured the 
> original code would only try Negotiate and because it failed it would retry 
> it endlessly. With this patch authentication correctly falls back to NTLM or 
> Basic (if NTLM fails too).
> 
> 
> Thanks,
> 
> Krzysztof Nowicki
> 
>

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

Re: Review Request 126991: Try multiple authentication methods in case of failures

[Andreas Hartmetz]

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




src/ioslaves/http/http.cpp (line 5442)
<https://git.reviewboard.kde.org/r/126991/#comment63498>

What does this do? (I suggest adding an example of something that triggers 
this code)


- Andreas Hartmetz


On March 3, 2016, 11:40 a.m., Krzysztof Nowicki wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126991/
> ---
> 
> (Updated March 3, 2016, 11:40 a.m.)
> 
> 
> Review request for KDE Frameworks and Dawit Alemayehu.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> When authenticating agains a server offering multiple authentication methods 
> make sure to attempt other methods in case the best one fails.
> 
> This also fixes a connection close issue in the middle of an NTLM 
> authentication dialog due to clearing the password.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/http/http.h 621b2c7a957b9bc9cc14ff13ed3c3a72dec38190 
>   src/ioslaves/http/http.cpp a84129f1403cbf8b0f86f9fd0354bec90ac5fd39 
> 
> Diff: https://git.reviewboard.kde.org/r/126991/diff/
> 
> 
> Testing
> ---
> 
> I have performed testing on an IIS 7.5 server which offered 3 authentication 
> options: Negotiate, NTLM and Basic. Since I have Kerberos configured the 
> original code would only try Negotiate and because it failed it would retry 
> it endlessly. With this patch authentication correctly falls back to NTLM or 
> Basic (if NTLM fails too).
> 
> 
> Thanks,
> 
> Krzysztof Nowicki
> 
>

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

Re: Review Request 126991: Try multiple authentication methods in case of failures

[Andreas Hartmetz]

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



m_triedPasswords doesn't really tell the truth if the application didn't supply 
a password, right? It is not really a number of tried passwords, it is the 
state of a little state machine. In such cases, it is better to make it an 
enum: enum TriedCredentials { NoCredentials = 0, JobCredentials, 
CachedCredentials, UserInputCredentials };
... where the distinction of Cached and UserInput is currently unnecessary but 
more consistent, a nice piece of self-documentation and might be helpful in the 
future.
(Note: "credentials" is better than "passwords" because it's a combination of 
username and password and i some special cases something different, like with 
NTLM and Kerberos)
Additionally, it looks like m_triedPasswords will carry over failed attempts 
from proxy authentication to web server authentication if both happen in the 
same KIO get() job [which may produce several HTTP GETs]. I'm not completely 
sure though because it's been a while since I worked on that code. Did you 
consider the problem?


src/ioslaves/http/http.cpp (line 5445)
<https://git.reviewboard.kde.org/r/126991/#comment63136>

} else {



src/ioslaves/http/http.cpp (line 5476)
<https://git.reviewboard.kde.org/r/126991/#comment63135>

Please break this line, it's extremely long with the addition.


- Andreas Hartmetz


On Feb. 9, 2016, 9:53 p.m., Krzysztof Nowicki wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126991/
> ---
> 
> (Updated Feb. 9, 2016, 9:53 p.m.)
> 
> 
> Review request for KDE Frameworks and Dawit Alemayehu.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> When authenticating agains a server offering multiple authentication methods 
> make sure to attempt other methods in case the best one fails.
> 
> This also fixes a connection close issue in the middle of an NTLM 
> authentication dialog due to clearing the password.
> 
> 
> Diffs
> -
> 
>   src/ioslaves/http/http.h 621b2c7a957b9bc9cc14ff13ed3c3a72dec38190 
>   src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 
> 
> Diff: https://git.reviewboard.kde.org/r/126991/diff/
> 
> 
> Testing
> ---
> 
> I have performed testing on an IIS 7.5 server which offered 3 authentication 
> options: Negotiate, NTLM and Basic. Since I have Kerberos configured the 
> original code would only try Negotiate and because it failed it would retry 
> it endlessly. With this patch authentication correctly falls back to NTLM or 
> Basic (if NTLM fails too).
> 
> 
> Thanks,
> 
> Krzysztof Nowicki
> 
>

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

Re: Review Request 127077: kio_http: read and discard body after a 404 with errorPage=false.

[Andreas Hartmetz]

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



Still reviewing... I think this is a regression caused by
https://git.reviewboard.kde.org/r/102801/
Previously, closing the connection on any error was a crude but effective way 
to reduce the number of possible states. This might not be the only bug created 
(or exposed if you like) by that change.
Also, m_iError is a bad name for the variable, it could also mean HTTP error 
but in fact it's the KIO error code.

- Andreas Hartmetz


On Feb. 20, 2016, 5:50 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127077/
> ---
> 
> (Updated Feb. 20, 2016, 5:50 p.m.)
> 
> 
> Review request for KDE Frameworks, Dawit Alemayehu, Andreas Hartmetz, and 
> Rolf Eike Beer.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> kio_http: read and discard body after a 404 with errorPage=false.
> 
> When getting a 404 error with some content, the job succeeds and returns 
> content
> (good for webbrowsers).
> The metadata "errorPage" can be set to false so that the job fails instead
> (useful for other cases, like favicon download, file copy etc.)
> However the rest of the headers, as well as the body must still be read and
> discarded, otherwise they clobber the next request (kio_http then starts
> parsing in the middle of some headers and says "DO NOT WANT").
> 
> This is not a porting bug, I could reproduce it with kdelibs4 too.
> 
> 
> Diffs
> -
> 
>   autotests/http_jobtest.cpp PRE-CREATION 
>   src/ioslaves/http/http.cpp e1013c8705e6588729d61ed45c43dc564415c41e 
> 
> Diff: https://git.reviewboard.kde.org/r/127077/diff/
> 
> 
> Testing
> ---
> 
> Unittest.
> 
> Please review this patch carefully, I don't have much experience with this 
> code in kio_http, and the potential for regressions in cases that I didn't 
> test is likely high.
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 127121: KWallet::openWallet(Synchronous) : time out after 1 hour rather than 25 seconds.

[Andreas Hartmetz]

> On Feb. 20, 2016, 8:25 p.m., Andreas Hartmetz wrote:
> > Sorry for being the guy who says "I don't like this improvement because 
> > it's still not perfect"...
> > Why have a timeout at all and not, for example, retry after getting a 
> > timeout error? (A long timeout still makes sense to avoid most UI glitches)
> > I hope nobody uses the synchronous API in the UI thread, it would be a 
> > terrible idea to do that anyway. Is this for e.g. Akonadi agents? Those 
> > could as well wait indefinitely. If they can't -> well they shouldn't use 
> > synchronous API. Synchronous IPC to an unreliable peer (if the user is 
> > involved it's unreliable ;) is asking for problems.
> 
> David Faure wrote:
> Hi Andreas, thanks for your input.  (I would also welcome your input on 
> https://git.reviewboard.kde.org/r/127077/ even though I just pushed it).
> 
> The use cases I saw (where the timeout was a problem with a sync call) 
> are ksshaskpass (no GUI yet at that point) and the imap resource. So yes, 
> infinite wait would be good, I agree. I just don't know how big a number DBus 
> supports for a timeout. Shall we make it 2^31-1 to avoid any signedness 
> issues? I'm not going to actually test if it can wait that long though ;)

Some very large number would be fine. If something has a problem with an 
extremely long timeout it will have the same problem with an hour-long timeout. 
And if that something has a problem with it, that is its own fault.
Sorry about the other one, I noticed it too late. I can still have a look.


- Andreas


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


On Feb. 20, 2016, 2:16 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127121/
> ---
> 
> (Updated Feb. 20, 2016, 2:16 p.m.)
> 
> 
> Review request for KDE Frameworks and Valentin Rusu.
> 
> 
> Repository: kwallet
> 
> 
> Description
> ---
> 
> The default DBus timeout is 25 seconds, which means that if the user went to
> get a cup of tea during session startup, when they come back they get prompted
> with all sorts of additional non-kwallet password requests due to all kwallet
> requests having timed out.
> 
> I added setTimeout in QDBusAbstractInterface in Qt 4.8 for things like this.
> 
> Testcase:
>  eval `dbus-launch`
>  export SSH_ASKPASS=$KDEDIR/bin/ksshaskpass
>  ssh-add < /dev/null
> (but the same happens with the IMAP resource etc.)
> 
> 
> Diffs
> -
> 
>   src/api/KWallet/kwallet.cpp b72edad19840943f70755c8668668a1881f1fb39 
> 
> Diff: https://git.reviewboard.kde.org/r/127121/diff/
> 
> 
> Testing
> ---
> 
> (see commit log)
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 127121: KWallet::openWallet(Synchronous) : time out after 1 hour rather than 25 seconds.

[Andreas Hartmetz]

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



Sorry for being the guy who says "I don't like this improvement because it's 
still not perfect"...
Why have a timeout at all and not, for example, retry after getting a timeout 
error? (A long timeout still makes sense to avoid most UI glitches)
I hope nobody uses the synchronous API in the UI thread, it would be a terrible 
idea to do that anyway. Is this for e.g. Akonadi agents? Those could as well 
wait indefinitely. If they can't -> well they shouldn't use synchronous API. 
Synchronous IPC to an unreliable peer (if the user is involved it's unreliable 
;) is asking for problems.

- Andreas Hartmetz


On Feb. 20, 2016, 2:16 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127121/
> ---
> 
> (Updated Feb. 20, 2016, 2:16 p.m.)
> 
> 
> Review request for KDE Frameworks and Valentin Rusu.
> 
> 
> Repository: kwallet
> 
> 
> Description
> ---
> 
> The default DBus timeout is 25 seconds, which means that if the user went to
> get a cup of tea during session startup, when they come back they get prompted
> with all sorts of additional non-kwallet password requests due to all kwallet
> requests having timed out.
> 
> I added setTimeout in QDBusAbstractInterface in Qt 4.8 for things like this.
> 
> Testcase:
>  eval `dbus-launch`
>  export SSH_ASKPASS=$KDEDIR/bin/ksshaskpass
>  ssh-add < /dev/null
> (but the same happens with the IMAP resource etc.)
> 
> 
> Diffs
> -
> 
>   src/api/KWallet/kwallet.cpp b72edad19840943f70755c8668668a1881f1fb39 
> 
> Diff: https://git.reviewboard.kde.org/r/127121/diff/
> 
> 
> Testing
> ---
> 
> (see commit log)
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 126895: Make KGlobalAccel dependency in KXmlGui optional

[Andreas Hartmetz]

On Donnerstag, 28. Januar 2016 12:07:54 CET Andre Heinecke wrote:
> Hi,
> 
> On Wednesday 27 January 2016 21:28:36 Andreas Hartmetz wrote:
> > On Mittwoch, 27. Januar 2016 17:21:33 CET Boudewijn Rempt wrote:
> > [snip]
> > 
> > > whether kglobalshortcuts is functional or not is besides the
> > > point:
> > > the point is that whether it works or not it doesn't add any
> > > functionality to the average application. Global shortcuts are
> > > useful
> > > only for a very limited set of applications, so it should always
> > > have
> > > been an optional dependency.
> > 
> > Here is something that is never beside the point:
> > maintenance burden and continuous-integration-ability.
> > ifdefs are somewhat bad for maintainability and really, really bad
> > for a continuous integration systems's ability to detect relevant
> > build breakage. I am repeating Martin's argument here with
> > different words.
> To repeat our argument (yay) we feel that we need to have the KXmlGui
> library as one of the core libraries for "KDE ishness" not to always
> depend on KGlobalAccel on all platforms as other Platforms have
> different concepts for Global shortcuts and they don't make sense for
> our users that we see.
> 
> Wrt maintenance burden / CI effort. Please you can say you only
> maintain and Test the recommended usage. Our alternative is that we
> have to Patch it out then we have to maintain the patches and
> basically a fork.
> 
> Also this is really a very simple patch. It's not much different
> behavior it just does not allow you to configure Global Shortcuts
> through KxmlGui.
> > The most maintainable way to make a feature optional is to disable
> > it at runtime.
> 
> I disagree with that. Runtime dependencies are more likely to cause
> bugs as a developer can not easily see / know how a function behaves
> when writing code.
> 
> Runtime dependencies also don't make sense for single Application
> deployments as we control all dependencies. The way it was in the
> past users had a Global Shortcuts configuration entry through KXmlGui
> which was always dysfunctional because we did not ship the necessary
> runtime components.
> 
> And KGlobalAccel is not the only thing. I don't want to see
> spellchecking entires either that won't work because we have not
> installed a spellchecking libary or dictionaries. Etc.
> 
> > Only when this severely hurts performance or a feature plain
> > won't build on a certain platform should it be disabled via ifdefs,
> > and in the latter case, maybe it can be put into its own class that
> > gets a real and a dummy implementation.
> 
> I dislike the dummy implementation. This makes code harder to
> understand and is more effort to maintain then simple harsh ifdefs.
> 
> It's also not a pattern already established for optional library
> depdenencies. It's established in the form of different
> implementations which might be dummies in a single library for
> different platforms. But to have dummy classes for optional
> dependencies is something different and afaik not established in KDE.
> 
> > My argument is first and foremost but for putting the knife where
> > the
> > least amount of stuff needs to be cut,
> 
> I think that XMLGui is the right place for this. XMLGui is a useful
> library without Global Shortcuts. It's very little code that needs
> GlobalAccel and it was easy to cut it out. There were not even any
> header / API changes neccessary for this.
> 
> > even if that results in shipping
> > an unnecessary (small!) module.
> 
> I fail to see how this would be advantageous at all. Having code in a
> library that allows it to be dysfunctional is wrong. And I would not
> propose making such a change in GlobalAccel. Why should anyone would
> want to waste resources for something that does not add anything at
> all.
> 
> Shipping a library means that we have to include it in our build
> system, we have to update it regularly, we have to fix build errors
> for our platform, we have to ship the license, we have to include the
> source code in our source distribution etc.
> 
> > Maybe there is a better solution, though.
> > How about a dummy KGlobalShortcuts implementation in KXmlGui? There
> > may be a few places where ifdefs are still required (e.g.
> > completely hiding - and I don't mean removing - inert UI), but the
> > vast majority of things in KGlobalShortcuts can be faked very
> > easily by doing nothing at all. Most code paths don't need to be
> > removed if there is simply no way to trigger them.
> 
> See above. A dummy library would

Re: Review Request 126895: Make KGlobalAccel dependency in KXmlGui optional

[Andreas Hartmetz]

On Mittwoch, 27. Januar 2016 17:21:33 CET Boudewijn Rempt wrote:
[snip]

> whether kglobalshortcuts is functional or not is besides the point:
> the point is that whether it works or not it doesn't add any
> functionality to the average application. Global shortcuts are useful
> only for a very limited set of applications, so it should always have
> been an optional dependency.
> 
Here is something that is never beside the point:
maintenance burden and continuous-integration-ability.
ifdefs are somewhat bad for maintainability and really, really bad for a 
continuous integration systems's ability to detect relevant build 
breakage. I am repeating Martin's argument here with different words.

The most maintainable way to make a feature optional is to disable it at 
runtime. Only when this severely hurts performance or a feature plain 
won't build on a certain platform should it be disabled via ifdefs, and 
in the latter case, maybe it can be put into its own class that gets a 
real and a dummy implementation.

My argument is first and foremost but for putting the knife where the 
least amount of stuff needs to be cut, even if that results in shipping 
an unnecessary (small!) module.

Maybe there is a better solution, though.
How about a dummy KGlobalShortcuts implementation in KXmlGui? There may 
be a few places where ifdefs are still required (e.g. completely hiding 
- and I don't mean removing - inert UI), but the vast majority of things 
in KGlobalShortcuts can be faked very easily by doing nothing at all. 
Most code paths don't need to be removed if there is simply no way to 
trigger them.
The thing to be faked is a system that has no global shortcuts and where 
setting them somehow always fails. Maybe pretend to applications that it 
doesn't fail, shouldn't really matter.

[snip]
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126865: kio_http: fix waiting until the cache cleaner listens to the socket.

[Andreas Hartmetz]

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


Ship it!




Yeah... I guess what happened under regular use was that the first request by 
the first ioslave started and then failed to connect to the cache cleaner. The 
cache cleaner then became ready and started to get used after a few requests or 
so.

- Andreas Hartmetz


On Jan. 23, 2016, 11 p.m., David Faure wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126865/
> ---
> 
> (Updated Jan. 23, 2016, 11 p.m.)
> 
> 
> Review request for KDE Frameworks and Andreas Hartmetz.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> The old code was doing connectToServer+waitForConnected, but this
> fails immediately if connectToServer sets an error like 
> QLocalSocket::ConnectionRefusedError.
> waitForConnected() is only meaningful if we are in ConnectingState.
> 
> We do have to try multiple times (no way to wait until precisely
> the time when it start listening), but not using waitForConnected,
> so I used msleep(100) in the loop instead.
> 
> I could have used DBus activation of course (to block until it's ready),
> but I assume DBus is not wanted here since that's not what's used for
> communication in the first place.
> 
> Change-Id: Ib9ee4699a44daa3016a72a24c6e816b940e1ed5c
> 
> 
> Diffs
> -
> 
>   src/ioslaves/http/http.cpp 76da711cbb3eee9ba801a6e8117d78aaee467626 
> 
> Diff: https://git.reviewboard.kde.org/r/126865/diff/
> 
> 
> Testing
> ---
> 
> kill kio_http_cache_cleaner; run unittest using KIO::get; watch kio_http 
> output.
> 
> Before:
> HTTPProtocol::sendCacheCleanerCommand: Could not connect to cache cleaner, 
> not updating stats of this cache file.
> 
> After:
> HTTPProtocol::sendCacheCleanerCommand: Successfully connected to cache 
> cleaner.
> 
> 
> Thanks,
> 
> David Faure
> 
>

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

Re: Review Request 126313: Use an xcb for interaction with KStartupInfo

[Andreas Hartmetz]

> On Jan. 25, 2016, 9:36 p.m., Luca Beltrame wrote:
> > FYI, this breaks Plasma 5 startup completely for me. kinit crashes with
> > 
> > ```gdb
> > #0  0x7ff831e58c20 in QObject::thread() const () at 
> > /usr/lib64/libQt5Core.so.5
> > #1  0x7ff8329d1a20 in KWindowSystem::d_func() () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #2  0x7ff8329d2cc9 in KWindowSystem::mapViewport() () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #3  0x7ff8329dce02 in NETRootInfo::currentDesktop(bool) const () at 
> > /usr/lib64/libKF5WindowSystem.so.5
> > #4  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (this=0x8e4458)
> > at /usr/include/qt5/QtCore/qbytearray.h:485
> > #5  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (this=0x8e4458)
> > at /usr/include/qt5/QtCore/qbytearray.h:479
> > #6  0x00408d09 in launch(int, char const*, char const*, char 
> > const*, int, char const*, bool, char const*, bool, char const*) 
> > (argc=32767, _name=0x7db02e20 "\020c\215", args=0x8e446b "/home/lb", 
> > cwd=, envc=, envs=, 
> > reset_env=false, tty=0x8e55b0 "", avoid_loops=160, startup_id_str=0x1161 
> > )
> > at /home/lb/Coding/KDEsrc/kinit/src/kdeinit/kinit.cpp:643
> > #7  0x008e559d in  ()
> > #8  0x in  ()
> > ```
> > 
> > I had to revert it or kdeinit would crash and prevent me from having a 
> > functional session. ;)

Same here.


- Andreas


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


On Jan. 25, 2016, 2:34 p.m., Martin Gräßlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126313/
> ---
> 
> (Updated Jan. 25, 2016, 2:34 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kinit
> 
> 
> Description
> ---
> 
> By changing to xcb we can use NETRootInfo to get the current desktop
> and don't need to duplicate the logic. Also it means that more code
> is ported from XLib to xcb and might allow us to drop the XLib
> dependency in future.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt eeecab775cfb5dfe12cf9c4991c658cc261ed727 
>   src/config-kdeinit.h.cmake cfcfc61a1bb560ff9a902b89bd3b8fb27273ebf2 
>   src/kdeinit/CMakeLists.txt f94db717f2403b602648286eac243837d8714069 
>   src/kdeinit/kinit.cpp a18008a11bf00a35aa0cab450180926217cd58f5 
> 
> Diff: https://git.reviewboard.kde.org/r/126313/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

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

Re: Deadlock in kded whith Qt 5.6

[Andreas Hartmetz]

On Dienstag, 19. Januar 2016 10:02:33 CET Aleix Pol wrote:
> On Mon, Jan 18, 2016 at 9:23 AM, David Faure  wrote:
> > On Friday 15 January 2016 13:48:14 Aleix Pol wrote:
> >> On Fri, Jan 15, 2016 at 1:02 PM, Aleix Pol  
wrote:
> >> > Hi,
> >> > I realized earlier last week that some of my applications were
> >> > freezing, it turns out it's because my kded is freezing and any
> >> > dbus
> >> > calls there need to time out (they take 30s to time out btw).
> >> > 
> >> > Anyway, here's the backtrace: https://paste.kde.org/pajbfmoh8
> >> > 
> >> > I saw David did something in that direction earlier but I
> >> > couldn't
> >> > spot exactly where's the problem, so I decided to report it here,
> >> > hoping he'd know what's going on. :D
> >> > 
> >> > Thanks!!
> >> > 
> >> > Aleix
> >> 
> >> Now that I know how to report a bug to kded5, I reported it. Here
> >> it
> >> is, for completion:
> >> https://bugs.kde.org/show_bug.cgi?id=358017
> > 
> > Thiago: indeed the BlockingQueuedConnection call from messageFilter
> > to the main thread occasionnally leads to deadlocks, we need your
> > "postpone incoming messages" idea.
> 
> It's good to know there's a plan, would it maybe be possible to expand
> a bit? I'd be interested in looking into it as my system is quite
> unusable and I can expect other people will become soon enough.
> 
I briefly talked about this with David yesterday and looked at the code.
What happened previously in kded5:
- it installs a message filter using libdbus API
- when a message to a not-yet-loaded module comes along, the module
  is loaded on the fly and directly from the message filter function 
  before the message is allowed to continue on its way

What happens now:
- the D-Bus connection lives in its own thread, in order to facilitate 
  using the connection from arbitrary threads in the application
- this puts the message filter in the D-Bus thread
- multithreading issues ensue

At the moment, you can pick between various poisons and eventually you 
have two threads waiting for each other and kded5 is mostly deadlocked.
The suggested solution is probably to *queue* messages to not-yet-loaded 
modules and to continue processing other D-Bus message while the 
required modules for the queued message are loading. That would break 
reasonable expectations about message ordering and blocking the least, I 
guess.

Ceteri censeo...
If you do anything blocking in D-Bus, you are going to have a bad time 
unless you have made sure that it is not so. "What could possibly go 
wrong?" is not good enough. Exceptions are calls to quickly replying 
services that are guaranteed to never make D-Bus calls of their own (*), 
and messages to the D-Bus daemon itself.
Please do not use blocking D-Bus calls unless you REALLY know what you 
are doing.
Now I've shamed myself into writing that documentation patch for QtDBus 
that I have wanted to write and submit since a year or three. It should 
have a big fat warning.
Asynchronous code is more tedious but it is in the general case the only 
correct way to make D-Bus calls.

Regarding the current situation, the synchronous module loading was okay 
before the Qt change unless modules somehow made D-Bus calls to 
something else than the daemon (to register their address) just by being 
loaded.
We have had lots of kded5 issues due to blocking calls over the years 
but I don'r remember seeing module loading in any backtraces.

(*) That is where deadlocks happen - directly or indirectly calling out 
to something that is in turn waiting for you.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126660: Avoid finding the same package multiple times from different paths.

[Andreas Hartmetz]

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

(Updated Jan. 8, 2016, 7:11 p.m.)


Review request for KDE Frameworks, kdelibs, Plasma, and Marco Martin.


Changes
---

Don't look the category anymore and reduce the amount of indirection, now more 
feasible with the previous simplification.


Repository: kpackage


Description
---

That was a problem in a scenario such as mine, where I have distro
packages and self compiled packages. There were (from the UI)
indistinguishable, duplicate packages in the panel's add applets
UI, in the tray config dialog's "additional items" section, and
probably in other places, too.
Note that requiring to have no duplicate packages in XDG_DATA_DIRS
by removing entries from XGD_DATA_DIRS doesn't fly because /usr
cannot be removed for the non-KF5 things that it brings.


Diffs (updated)
-

  src/kpackage/packageloader.cpp 8b802d6 

Diff: https://git.reviewboard.kde.org/r/126660/diff/


Testing
---

Checked for duplicate entries in "add applets" and tray config dialog, no 
duplicates anymore. Also no duplicates anymore in KWin effects KCM.


Thanks,

Andreas Hartmetz

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

Re: Review Request 126660: Avoid finding the same package multiple times from different paths.

[Andreas Hartmetz]

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

(Updated Jan. 8, 2016, 7:11 p.m.)


Review request for KDE Frameworks, kdelibs, Plasma, and Marco Martin.


Changes
---

Don't look the category anymore and reduce the amount of indirection, now more 
feasible with the previous simplification.


Repository: kpackage


Description
---

That was a problem in a scenario such as mine, where I have distro
packages and self compiled packages. There were (from the UI)
indistinguishable, duplicate packages in the panel's add applets
UI, in the tray config dialog's "additional items" section, and
probably in other places, too.
Note that requiring to have no duplicate packages in XDG_DATA_DIRS
by removing entries from XGD_DATA_DIRS doesn't fly because /usr
cannot be removed for the non-KF5 things that it brings.


Diffs (updated)
-

  src/kpackage/packageloader.cpp 8b802d6 

Diff: https://git.reviewboard.kde.org/r/126660/diff/


Testing
---

Checked for duplicate entries in "add applets" and tray config dialog, no 
duplicates anymore. Also no duplicates anymore in KWin effects KCM.


Thanks,

Andreas Hartmetz

Re: Review Request 126660: Avoid finding the same package multiple times from different paths.

[Andreas Hartmetz]

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

(Updated Jan. 8, 2016, 11:29 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, kdelibs, Plasma, and Marco Martin.


Changes
---

Submitted with commit 85cc90b65ed8a9aa110018b01813fe1fd6772d48 by Andreas 
Hartmetz to branch master.


Repository: kpackage


Description
---

That was a problem in a scenario such as mine, where I have distro
packages and self compiled packages. There were (from the UI)
indistinguishable, duplicate packages in the panel's add applets
UI, in the tray config dialog's "additional items" section, and
probably in other places, too.
Note that requiring to have no duplicate packages in XDG_DATA_DIRS
by removing entries from XGD_DATA_DIRS doesn't fly because /usr
cannot be removed for the non-KF5 things that it brings.


Diffs
-

  src/kpackage/packageloader.cpp 8b802d6 

Diff: https://git.reviewboard.kde.org/r/126660/diff/


Testing
---

Checked for duplicate entries in "add applets" and tray config dialog, no 
duplicates anymore. Also no duplicates anymore in KWin effects KCM.


Thanks,

Andreas Hartmetz

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

Re: Review Request 126660: Avoid finding the same package multiple times from different paths.

[Andreas Hartmetz]

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

(Updated Jan. 8, 2016, 11:29 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, kdelibs, Plasma, and Marco Martin.


Changes
---

Submitted with commit 85cc90b65ed8a9aa110018b01813fe1fd6772d48 by Andreas 
Hartmetz to branch master.


Repository: kpackage


Description
---

That was a problem in a scenario such as mine, where I have distro
packages and self compiled packages. There were (from the UI)
indistinguishable, duplicate packages in the panel's add applets
UI, in the tray config dialog's "additional items" section, and
probably in other places, too.
Note that requiring to have no duplicate packages in XDG_DATA_DIRS
by removing entries from XGD_DATA_DIRS doesn't fly because /usr
cannot be removed for the non-KF5 things that it brings.


Diffs
-

  src/kpackage/packageloader.cpp 8b802d6 

Diff: https://git.reviewboard.kde.org/r/126660/diff/


Testing
---

Checked for duplicate entries in "add applets" and tray config dialog, no 
duplicates anymore. Also no duplicates anymore in KWin effects KCM.


Thanks,

Andreas Hartmetz

Review Request 126660: Avoid finding the same package multiple times from different paths.

[Andreas Hartmetz]

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

Review request for kde-workspace, KDE Frameworks and kdelibs.


Repository: kpackage


Description
---

That was a problem in a scenario such as mine, where I have distro
packages and self compiled packages. There were (from the UI)
indistinguishable, duplicate packages in the panel's add applets
UI, in the tray config dialog's "additional items" section, and
probably in other places, too.
Note that requiring to have no duplicate packages in XDG_DATA_DIRS
by removing entries from XGD_DATA_DIRS doesn't fly because /usr
cannot be removed for the non-KF5 things that it brings.


Diffs
-

  src/kpackage/packageloader.cpp 9f7dd48 

Diff: https://git.reviewboard.kde.org/r/126660/diff/


Testing
---

Checked for duplicate entries in "add applets" and tray config dialog, no 
duplicates anymore. Also no duplicates anymore in KWin effects KCM.


Thanks,

Andreas Hartmetz

Re: Review Request 126660: Avoid finding the same package multiple times from different paths.

[Andreas Hartmetz]

> On Jan. 7, 2016, 3:31 p.m., Sebastian Kügler wrote:
> > src/kpackage/packageloader.cpp, line 190
> > <https://git.reviewboard.kde.org/r/126660/diff/1/?file=428735#file428735line190>
> >
> > What does the category have to do with this? We should only be going by 
> > the id (the plugin name).
> 
> Andreas Hartmetz wrote:
> Depends on the scope in which an ID is guaranteed to be unique. If it's 
> guaranteed to be globally unique and independent of category, sure, I'm all 
> for not making it more complicated than necessary.

Given that the ID falls back to the metadata file name without extension, and 
that you can have the same file name in different directories, it seems like 
you *could* have a package of the same name in different categories. The same 
goes for names supplied by the data in the metadata files. Some names make 
sense in several places.


- Andreas


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


On Jan. 7, 2016, 3:21 p.m., Andreas Hartmetz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126660/
> ---
> 
> (Updated Jan. 7, 2016, 3:21 p.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs, Plasma, and Marco Martin.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> That was a problem in a scenario such as mine, where I have distro
> packages and self compiled packages. There were (from the UI)
> indistinguishable, duplicate packages in the panel's add applets
> UI, in the tray config dialog's "additional items" section, and
> probably in other places, too.
> Note that requiring to have no duplicate packages in XDG_DATA_DIRS
> by removing entries from XGD_DATA_DIRS doesn't fly because /usr
> cannot be removed for the non-KF5 things that it brings.
> 
> 
> Diffs
> -
> 
>   src/kpackage/packageloader.cpp 9f7dd48 
> 
> Diff: https://git.reviewboard.kde.org/r/126660/diff/
> 
> 
> Testing
> ---
> 
> Checked for duplicate entries in "add applets" and tray config dialog, no 
> duplicates anymore. Also no duplicates anymore in KWin effects KCM.
> 
> 
> Thanks,
> 
> Andreas Hartmetz
> 
>

Re: Review Request 126660: Avoid finding the same package multiple times from different paths.

[Andreas Hartmetz]

> On Jan. 7, 2016, 3:31 p.m., Sebastian Kügler wrote:
> > src/kpackage/packageloader.cpp, line 190
> > <https://git.reviewboard.kde.org/r/126660/diff/1/?file=428735#file428735line190>
> >
> > What does the category have to do with this? We should only be going by 
> > the id (the plugin name).
> 
> Andreas Hartmetz wrote:
> Depends on the scope in which an ID is guaranteed to be unique. If it's 
> guaranteed to be globally unique and independent of category, sure, I'm all 
> for not making it more complicated than necessary.

Given that the ID falls back to the metadata file name without extension, and 
that you can have the same file name in different directories, it seems like 
you *could* have a package of the same name in different categories. The same 
goes for names supplied by the data in the metadata files. Some names make 
sense in several places.


- Andreas


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


On Jan. 7, 2016, 3:21 p.m., Andreas Hartmetz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126660/
> ---
> 
> (Updated Jan. 7, 2016, 3:21 p.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs, Plasma, and Marco Martin.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> That was a problem in a scenario such as mine, where I have distro
> packages and self compiled packages. There were (from the UI)
> indistinguishable, duplicate packages in the panel's add applets
> UI, in the tray config dialog's "additional items" section, and
> probably in other places, too.
> Note that requiring to have no duplicate packages in XDG_DATA_DIRS
> by removing entries from XGD_DATA_DIRS doesn't fly because /usr
> cannot be removed for the non-KF5 things that it brings.
> 
> 
> Diffs
> -
> 
>   src/kpackage/packageloader.cpp 9f7dd48 
> 
> Diff: https://git.reviewboard.kde.org/r/126660/diff/
> 
> 
> Testing
> ---
> 
> Checked for duplicate entries in "add applets" and tray config dialog, no 
> duplicates anymore. Also no duplicates anymore in KWin effects KCM.
> 
> 
> Thanks,
> 
> Andreas Hartmetz
> 
>

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

Re: Review Request 126660: Avoid finding the same package multiple times from different paths.

[Andreas Hartmetz]

> On Jan. 7, 2016, 3:31 p.m., Sebastian Kügler wrote:
> > Wouldn't it be way easier to simply check if the plugin is already in lst?
> > 
> > Your approach looks very complex for just that...

O(n) vs. O(n^2)


- Andreas


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


On Jan. 7, 2016, 3:21 p.m., Andreas Hartmetz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126660/
> ---
> 
> (Updated Jan. 7, 2016, 3:21 p.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs, Plasma, and Marco Martin.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> That was a problem in a scenario such as mine, where I have distro
> packages and self compiled packages. There were (from the UI)
> indistinguishable, duplicate packages in the panel's add applets
> UI, in the tray config dialog's "additional items" section, and
> probably in other places, too.
> Note that requiring to have no duplicate packages in XDG_DATA_DIRS
> by removing entries from XGD_DATA_DIRS doesn't fly because /usr
> cannot be removed for the non-KF5 things that it brings.
> 
> 
> Diffs
> -
> 
>   src/kpackage/packageloader.cpp 9f7dd48 
> 
> Diff: https://git.reviewboard.kde.org/r/126660/diff/
> 
> 
> Testing
> ---
> 
> Checked for duplicate entries in "add applets" and tray config dialog, no 
> duplicates anymore. Also no duplicates anymore in KWin effects KCM.
> 
> 
> Thanks,
> 
> Andreas Hartmetz
> 
>

Re: Review Request 126660: Avoid finding the same package multiple times from different paths.

[Andreas Hartmetz]

> On Jan. 7, 2016, 3:31 p.m., Sebastian Kügler wrote:
> > src/kpackage/packageloader.cpp, line 187
> > <https://git.reviewboard.kde.org/r/126660/diff/1/?file=428735#file428735line187>
> >
> > This doesn't actually add anything. A better name would IMO be: 
> > alreadyListed() or something along those lines.

It also adds it to the list, though.


> On Jan. 7, 2016, 3:31 p.m., Sebastian Kügler wrote:
> > src/kpackage/packageloader.cpp, line 190
> > <https://git.reviewboard.kde.org/r/126660/diff/1/?file=428735#file428735line190>
> >
> > What does the category have to do with this? We should only be going by 
> > the id (the plugin name).

Depends on the scope in which an ID is guaranteed to be unique. If it's 
guaranteed to be globally unique and independent of category, sure, I'm all for 
not making it more complicated than necessary.


- Andreas


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


On Jan. 7, 2016, 3:21 p.m., Andreas Hartmetz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126660/
> ---
> 
> (Updated Jan. 7, 2016, 3:21 p.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs, Plasma, and Marco Martin.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> That was a problem in a scenario such as mine, where I have distro
> packages and self compiled packages. There were (from the UI)
> indistinguishable, duplicate packages in the panel's add applets
> UI, in the tray config dialog's "additional items" section, and
> probably in other places, too.
> Note that requiring to have no duplicate packages in XDG_DATA_DIRS
> by removing entries from XGD_DATA_DIRS doesn't fly because /usr
> cannot be removed for the non-KF5 things that it brings.
> 
> 
> Diffs
> -
> 
>   src/kpackage/packageloader.cpp 9f7dd48 
> 
> Diff: https://git.reviewboard.kde.org/r/126660/diff/
> 
> 
> Testing
> ---
> 
> Checked for duplicate entries in "add applets" and tray config dialog, no 
> duplicates anymore. Also no duplicates anymore in KWin effects KCM.
> 
> 
> Thanks,
> 
> Andreas Hartmetz
> 
>

Re: Review Request 126660: Avoid finding the same package multiple times from different paths.

[Andreas Hartmetz]

> On Jan. 7, 2016, 3:31 p.m., Sebastian Kügler wrote:
> > src/kpackage/packageloader.cpp, line 190
> > <https://git.reviewboard.kde.org/r/126660/diff/1/?file=428735#file428735line190>
> >
> > What does the category have to do with this? We should only be going by 
> > the id (the plugin name).
> 
> Andreas Hartmetz wrote:
> Depends on the scope in which an ID is guaranteed to be unique. If it's 
> guaranteed to be globally unique and independent of category, sure, I'm all 
> for not making it more complicated than necessary.
> 
> Andreas Hartmetz wrote:
> Given that the ID falls back to the metadata file name without extension, 
> and that you can have the same file name in different directories, it seems 
> like you *could* have a package of the same name in different categories. The 
> same goes for names supplied by the data in the metadata files. Some names 
> make sense in several places.

Sorry, the part about the filename doesn't apply. It comes from the 
documentation of KPluginMetaData::pluginId() which doesn't apply here, given 
that the files are all called metadata.desktop or metadata.json.


- Andreas


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


On Jan. 7, 2016, 3:21 p.m., Andreas Hartmetz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126660/
> ---
> 
> (Updated Jan. 7, 2016, 3:21 p.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs, Plasma, and Marco Martin.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> That was a problem in a scenario such as mine, where I have distro
> packages and self compiled packages. There were (from the UI)
> indistinguishable, duplicate packages in the panel's add applets
> UI, in the tray config dialog's "additional items" section, and
> probably in other places, too.
> Note that requiring to have no duplicate packages in XDG_DATA_DIRS
> by removing entries from XGD_DATA_DIRS doesn't fly because /usr
> cannot be removed for the non-KF5 things that it brings.
> 
> 
> Diffs
> -
> 
>   src/kpackage/packageloader.cpp 9f7dd48 
> 
> Diff: https://git.reviewboard.kde.org/r/126660/diff/
> 
> 
> Testing
> ---
> 
> Checked for duplicate entries in "add applets" and tray config dialog, no 
> duplicates anymore. Also no duplicates anymore in KWin effects KCM.
> 
> 
> Thanks,
> 
> Andreas Hartmetz
> 
>

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

Re: Review Request 126660: Avoid finding the same package multiple times from different paths.

[Andreas Hartmetz]

> On Jan. 7, 2016, 3:31 p.m., Sebastian Kügler wrote:
> > Wouldn't it be way easier to simply check if the plugin is already in lst?
> > 
> > Your approach looks very complex for just that...

O(n) vs. O(n^2)


- Andreas


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


On Jan. 7, 2016, 3:21 p.m., Andreas Hartmetz wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126660/
> ---
> 
> (Updated Jan. 7, 2016, 3:21 p.m.)
> 
> 
> Review request for KDE Frameworks, kdelibs, Plasma, and Marco Martin.
> 
> 
> Repository: kpackage
> 
> 
> Description
> ---
> 
> That was a problem in a scenario such as mine, where I have distro
> packages and self compiled packages. There were (from the UI)
> indistinguishable, duplicate packages in the panel's add applets
> UI, in the tray config dialog's "additional items" section, and
> probably in other places, too.
> Note that requiring to have no duplicate packages in XDG_DATA_DIRS
> by removing entries from XGD_DATA_DIRS doesn't fly because /usr
> cannot be removed for the non-KF5 things that it brings.
> 
> 
> Diffs
> -
> 
>   src/kpackage/packageloader.cpp 9f7dd48 
> 
> Diff: https://git.reviewboard.kde.org/r/126660/diff/
> 
> 
> Testing
> ---
> 
> Checked for duplicate entries in "add applets" and tray config dialog, no 
> duplicates anymore. Also no duplicates anymore in KWin effects KCM.
> 
> 
> Thanks,
> 
> Andreas Hartmetz
> 
>

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

Re: Causes of session management problems in Plasma 5

[Andreas Hartmetz]

On Donnerstag, 26. November 2015 08:55:19 CET Thomas Lübking wrote:
> On Donnerstag, 26. November 2015 05:19:34 CEST, Nicolás Alvarez wrote:
> > What do you mean with "konsole asks"? Things like "You have
> > multiple tabs open, are you sure you want to quit?" and "You
> > have unsaved changes"?
> 
> Yes.
> 
> > If so, the scenario you describe is bad regardless of session
> > restoration.
> 
> Yes. Unfortunaltely.
> 
> > If konsole has to ask the user and the user has a
> > chance to say no and cancel the logout at that point, then
> > kwrite shouldn't have exited yet!
> 
> That's what the spec says, but the ksmserver change suggestion is
> about buggy clients that behave exactly this way.
> > If "app canceling logout" is a thing, then logout should feel
> > transactional to the user. Either logout happens and all apps
> > exit, or logout doesn't happen and nothing exits.
> 
> Yes. That's what the spec says ...
> 
> > I guess the implementation of that would be: all apps should be
> > given a chance to ask their questions and approve or stop the
> > logout
> That'S exactly what happens, but ...
> 
> > before *any* app exits.
> 
> We cannot stop process from doing stupid things, like exiting in the
> wrong moment.
> > This is how MS Windows works, by the way (or used to work).
> 
> No, it's how not buggy applications work on windows.
> 
> Cheers,
> Thomas

I was going to write pretty much the same things, Thomas was just faster 
:)
XSMP is a fairly well thought out spec although the full text, which I 
haven't read in its entirety, seems to be too long for what the spec is 
supposed to achieve. Logout being transactional is a big item in it.

Remove legacy session management support?

[Andreas Hartmetz]

Hello,

While looking at ksmserver, which is already a bit too big to be easy to 
understand, I noticed that it still has about 500 lines of code to deal 
with applications that don't yet speak the "modern" (from 1993) XSMP 
session management protocol.
kwin seems to have some more code for legacy session management. I was 
planning to remove it from ksmserver but it only makes sense if it's 
also removed from kwin.
So, does anybody know of any relevant application that supports only 
legacy session management? The scheme seems to be called
WM_COMMAND / WM_SAVE_YOURSELF for lack of a better name.
KDE applications switched to XSMP with KDE 2.0.

There is also some chance that the legacy support was broken anyway 
during 2->3, 3->4 or 4->5 porting because it hasn't been used...

Cheers,
Andreas

Re: Causes of session management problems in Plasma 5

[Andreas Hartmetz]

That's fine with me. No action is much easier to implement than blocking 
of specific events ;)

On Dienstag, 24. November 2015 02:31:30 CET Henry Miller wrote:
> I'll object to no interaction after logout. More than once I've asked
> "why is logout taking so long, Jumped to a terminal (not always
> konsole) to look.
> 
>  Also, on Windows (at least) a running terminal application will block
> logout so I may need to kill the application while in a logout
> context.  I'm not sure how this relates to konsole on other
> platforms, but it seems like the use case exists.
> On November 23, 2015 7:02:46 PM CST, Andreas Hartmetz 
<ahartm...@gmail.com> wrote:
> >Hello,
> >
> >As apparently one of the last users of session management, because I
> >shut down my computers about once every day, I run into problems
> >about as often as I log into a session that is supposed to be
> >restored. The number one problem is Konsole just not restoring.
> >So I took some time to investigate the problem. The result is that
> >there
> >are several bugs that conspire to break session restore. It goes
> >about like this:
> >
> >- ksmserver (the session manager) sends clients the "SaveYourself"
> >
> >  message and collects the responses. This works fine.
> >
> >- In Qt applications, this results in a call to
> >
> >  QGuiApplicationPrivate::commitData(), which calls
> >  QApplicationPrivate::tryCloseAllWindows() after the part that sends
> >  the SaveYourselfDone response to the session manager.
> >  When QGuiApplication::quitOnLastWindowClosed() is true (the
> >  default),
> >  this results in the application quitting.
> >
> >- ksmserver notices that (e.g.) konsole has terminated and purges it
> >
> >  from its internal data
> >
> >- ksmserver rounds up remaining processes, which at this point do not
> >
> > include konsole, and saves their restore data. konsole thus has
> > saved
> > 
> >  its state, but ksmserver forgot about it and doesn't remember to do
> >  anything with konsole when restoring the session later.
> >
> >The two most obvious errors are thus:
> >
> >- QGuiApplicationPrivate::commitData() calling
> >
> >  QApplicationPrivate::tryCloseAllWindows(), together with
> >
> >QGuiApplication::quitOnLastWindowClosed() being true by default.
> >Quote>
> >  from documentation of signal QGuiApplication::commitDataRequest():
> >  "You should not exit the application within this signal. Instead,
> >  the
> >  session manager may or may not do this afterwards, depending on the
> >  context."
> >  Note that it says session manager and afterwards, not
> >  QGuiApplication
> >  and virtually immediately.
> >
> >- The session manager not "locking down" or better copying the list
> >of>
> >  clients *while* logging out. This would arguably only help buggy
> >  clients, but may still be a net positive.
> >  Why copy the list? Logout may be canceled, so it is valuable to
> >  keep
> >  the main client list updated for after logout cancellation.
> >
> >- Bonus: I've found that KMainWindowPrivate::init() calls
> >
> >  QCoreApplication::setQuitLockEnabled(true), which is equivalent to
> >  QGuiApplication::setQuitOnLastWindowClosed(true), which is either
> >  redundant with the default or overrides something the user has
> >  explicitly changed. I noticed this while trying to narrow down the
> >  problem with a call to
> >  QGuiApplication::setQuitOnLastWindowClosed(false) from konsole's
> >  Application class. Which is not a good solution, but sufficient as
> >  a
> >  proof of concept fix. That fix works as far as session save and
> >  restore are concerned.
> >
> >So now I wonder what the chances are to fix those bugs.
> >
> >- Make ksmserver more robust in the face of clients dying too early,
> >
> >  sure. I hope to get around to it Soon(TM).
> >
> >- Remove QCoreApplication::setQuitLockEnabled(true) from
> >
> > KMainWindowPrivate::init() - seems like a good idea to me,
> > objections?>
> >- Remove any window closing from QGuiApplicationPrivate::commitData()
> >->
> >  this is actually an old feature that was even modified in 2014 to
> >  fix a problem on Windows(?!) - I guess that one is there to prevent
> >  interaction after session saving, but it's a very crude way to do
> >  that. IMO it would be better to do nothing, it would be even better
> >  to block user input and possibly I/O handling for the duration of
> >  logout and unblock them when logout is canceled.
> >  Note: the Windows fix is about the method being expected to *kill*
> > 
> > the application, which probably comes from a lack of knowledge about
> > X> 
> >  session management which is the main purpose of that method. Commit
> >  9835a63dde06a1e5d2f in qtbase.
> >
> >I'd be grateful for any additional insight and / or historical
> >perspective.
> >
> >Cheers,
> >Andreas

Re: Causes of session management problems in Plasma 5

[Andreas Hartmetz]

On Mittwoch, 25. November 2015 18:05:26 CET Thomas Lübking wrote:
> On Dienstag, 24. November 2015 02:02:46 CEST, Andreas Hartmetz wrote:
> > - The session manager not "locking down" or better copying the list
> > of> 
> >   clients *while* logging out. This would arguably only help buggy
> >   clients, but may still be a net positive.
> 
> It would falsely restore clients that do not want to be restored
> (maybe also because they've gotten some autostart entry)

No, I don't think so. Clients that don't want to be restored don't 
respond to "SaveYourself" with "SaveYourselfDone". More details below.

> >   Why copy the list? Logout may be canceled, so it is valuable to
> >   keep
> >   the main client list updated for after logout cancellation.
> 
> So if I logout, kwrite exits, konsole asks, I cancel, I restart
> kwrite, I logout, kwrite exits, konsole asks, I cancel, I restart
> kwrite, ... I end up with n iterations of kwrite on the restorage?
> Nahhh the SM should restore what the user left behind on logout,
> not what he had running in the former session "somewhen" ;-)
> 
I haven't explained all the details.
First, by copy I mean a temporary copy that is never merged back into 
the main list, it's kept around only to know which processes have 
already agreed to have their session saved and submitted the 
corresponding data.
The saved process states are saved in files in ~/.config/session. 
However, those files do nothing if ksmserver doesn't also *restart* the 
respective process, usually with the -session  command-line 
argument. ksmserver only saves the list of processes to restore when 
logout has been "confirmed", which means when nothing can cancel it 
anymore. The list of processes to restore is saved in ~/.config/
ksmserverrc.
ksmserver even has or can have enough data to be able to clean up stale, 
already saved session files when logout is canceled. Currently it leaks 
konsole session files...

The current problem is that some applications die between submitting 
their state to ksmserver (SaveYourselfDone message) and ksmserver saving 
the list of processes to restart (writing ksmserverrc). It is AFAICS 
safe to assume that an application that has submitted its session data 
is really supposed to be restored.

> Other than that, thanks and kudos for the Investigation and get
> QGuiApplication fixed itr. =)
> 
Thanks! Wish me luck :)
It's going to be a behavior change, you know :/

> Cheers,
> Thomas

Causes of session management problems in Plasma 5

[Andreas Hartmetz]

Hello,

As apparently one of the last users of session management, because I 
shut down my computers about once every day, I run into problems about 
as often as I log into a session that is supposed to be restored.
The number one problem is Konsole just not restoring.
So I took some time to investigate the problem. The result is that there 
are several bugs that conspire to break session restore. It goes about 
like this:

- ksmserver (the session manager) sends clients the "SaveYourself" 
  message and collects the responses. This works fine.
- In Qt applications, this results in a call to 
  QGuiApplicationPrivate::commitData(), which calls 
  QApplicationPrivate::tryCloseAllWindows() after the part that sends
  the SaveYourselfDone response to the session manager.
  When QGuiApplication::quitOnLastWindowClosed() is true (the default),
  this results in the application quitting.
- ksmserver notices that (e.g.) konsole has terminated and purges it
  from its internal data
- ksmserver rounds up remaining processes, which at this point do not 
  include konsole, and saves their restore data. konsole thus has saved 
  its state, but ksmserver forgot about it and doesn't remember to do
  anything with konsole when restoring the session later.

The two most obvious errors are thus:

- QGuiApplicationPrivate::commitData() calling 
  QApplicationPrivate::tryCloseAllWindows(), together with
  QGuiApplication::quitOnLastWindowClosed() being true by default. Quote 
  from documentation of signal QGuiApplication::commitDataRequest():
  "You should not exit the application within this signal. Instead, the
  session manager may or may not do this afterwards, depending on the
  context."
  Note that it says session manager and afterwards, not QGuiApplication
  and virtually immediately.

- The session manager not "locking down" or better copying the list of
  clients *while* logging out. This would arguably only help buggy
  clients, but may still be a net positive.
  Why copy the list? Logout may be canceled, so it is valuable to keep
  the main client list updated for after logout cancellation.

- Bonus: I've found that KMainWindowPrivate::init() calls 
  QCoreApplication::setQuitLockEnabled(true), which is equivalent to
  QGuiApplication::setQuitOnLastWindowClosed(true), which is either
  redundant with the default or overrides something the user has
  explicitly changed. I noticed this while trying to narrow down the 
  problem with a call to 
  QGuiApplication::setQuitOnLastWindowClosed(false) from konsole's
  Application class. Which is not a good solution, but sufficient as a
  proof of concept fix. That fix works as far as session save and 
  restore are concerned.

So now I wonder what the chances are to fix those bugs.

- Make ksmserver more robust in the face of clients dying too early,
  sure. I hope to get around to it Soon(TM).

- Remove QCoreApplication::setQuitLockEnabled(true) from 
  KMainWindowPrivate::init() - seems like a good idea to me, objections?

- Remove any window closing from QGuiApplicationPrivate::commitData() -
  this is actually an old feature that was even modified in 2014 to
  fix a problem on Windows(?!) - I guess that one is there to prevent 
  interaction after session saving, but it's a very crude way to do 
  that. IMO it would be better to do nothing, it would be even better 
  to block user input and possibly I/O handling for the duration of 
  logout and unblock them when logout is canceled.
  Note: the Windows fix is about the method being expected to *kill*
  the application, which probably comes from a lack of knowledge about X
  session management which is the main purpose of that method. Commit
  9835a63dde06a1e5d2f in qtbase.

I'd be grateful for any additional insight and / or historical 
perspective.

Cheers,
Andreas

Re: Freeze on nested eventloops (QFileDialog::getOpen*)

[Andreas Hartmetz]

On Sunday 14 June 2015 15:00:57 Milian Wolff wrote:
 Hey all,
 
 in massif-visualizer's framework branch, and now also kate, I
 sometimes notice GUI lockups when I try to open files via Ctrl + O.
 The backtraces look like the following:
 
 https://paste.kde.org/pbzhkgx8r
 
 Any idea what might be going on here? Is it not a good idea to mix
 system compiled KF5-based stuff (e.g. KDEPlatformTheme.so) with
 kdesrc-build compiled KF5 libraries (/home/milian/projects/kf5/*)?
 
 The applications do not recover from this lockup.
 
Another interesting thing just happened: I opened a file dialog in Kate 
and it froze. I was editing (with rebase -i) a commit message in the 
same repository in which Kate had open files. After saving the commit 
message, Kate went back to normal. So it looks KDirWatch related.

Re: Freeze on nested eventloops (QFileDialog::getOpen*)

[Andreas Hartmetz]

On Monday 22 June 2015 01:09:59 Boudhayan Gupta wrote:
 Hi,
 
 If you're talking about the static QFileDialog::getOpen* methods, I
 may have hit a similar issue with the static QFileDialog::getSaveFile*
 methods. I was working on KScreenGenie, and the app just crashed
 maybe one out of every three or four times the Save File dialog
 opened. Not a lockup, but an outright crash.
 
 At that point I thought I was making an error on my part, so I just
 ended up manually creating a QFileDialog and working with it. Just
 reporting this, in case it's related.
 
 -- Boudhayan Gupta
 

Hello!

You have something valuable on your hands, a way to reproduce the 
problem that will dump you into a debugger (usually) close to the 
location where the bug is happening. If you could take a backtrace of 
the crash, that could get us to a solution more quickly. It seems like 
an important bug.

(formal stuff)
While the kind of your message is suitable for top-posting on its own, I 
suggest that you still keep bottom-posting on a technical mailing list 
such as this. At some point, it's going to get into technical details 
again and then it's good to have everything in chronological / topical 
order top to bottom.

 On 21 June 2015 at 20:29, Andreas Hartmetz ahartm...@gmail.com 
wrote:
  On Sunday 14 June 2015 15:00:57 Milian Wolff wrote:
  Hey all,
  
  in massif-visualizer's framework branch, and now also kate, I
  sometimes notice GUI lockups when I try to open files via Ctrl + O.
  The backtraces look like the following:
  
  https://paste.kde.org/pbzhkgx8r
  
  Any idea what might be going on here? Is it not a good idea to mix
  system compiled KF5-based stuff (e.g. KDEPlatformTheme.so) with
  kdesrc-build compiled KF5 libraries (/home/milian/projects/kf5/*)?
  
  The applications do not recover from this lockup.
  
  I recently got the apparently same hang in Kate twice in one day.
  The
  application froze while IIRC not using CPU cycles and didn't
  recover. I did not attach gdb. To the best of my knowledge,
  everything from qtbase up (5.5 branch) is self compiled so there is
  no mixing with distro provided libraries.
  I have checked info shared in gdb and found nothing Qt related
  coming from /usr. This is now a few days after the hangs so there
  is a small probability that something changed.

Re: Freeze on nested eventloops (QFileDialog::getOpen*)

[Andreas Hartmetz]

On Sunday 14 June 2015 15:00:57 Milian Wolff wrote:
 Hey all,
 
 in massif-visualizer's framework branch, and now also kate, I
 sometimes notice GUI lockups when I try to open files via Ctrl + O.
 The backtraces look like the following:
 
 https://paste.kde.org/pbzhkgx8r
 
 Any idea what might be going on here? Is it not a good idea to mix
 system compiled KF5-based stuff (e.g. KDEPlatformTheme.so) with
 kdesrc-build compiled KF5 libraries (/home/milian/projects/kf5/*)?
 
 The applications do not recover from this lockup.
 
I recently got the apparently same hang in Kate twice in one day. The 
application froze while IIRC not using CPU cycles and didn't recover. I 
did not attach gdb. To the best of my knowledge, everything from qtbase 
up (5.5 branch) is self compiled so there is no mixing with distro 
provided libraries.
I have checked info shared in gdb and found nothing Qt related coming 
from /usr. This is now a few days after the hangs so there is a small 
probability that something changed.

Re: KDiagram libs (KChart, KGantt) in KDE Review

[Andreas Hartmetz]

On Wednesday 11 February 2015 14:59:50 Adriaan de Groot wrote:
 On Monday 09 February 2015 01:50:03 Friedrich W. H. Kossebau wrote:
  Yes, nearly copypaste: the copies of KDChart in Calligra  KMyMoney
  are older  (2.4.1, based on Qt4) versions, while the copy of
  KDChart in Massif-Visualizer matches the version of the KChart lib
  in KDiagram.
 I've tried compiling the code on FreeBSD 10.1-RELEASE with Clang 3.4.1
 (I'm assuming that's a supported compiler -- on techbase, searching
 for supported compiler doesn't give me much compatibility
 information newer than KDE 4.2).
 
  - I need to add /usr/local/include to the include search path; this
 is not kdiagram specific, but seems to be a general Qt5 issue on
 FreeBSD.
 
  - TestDrawIntoPainter seems to hang; after 2 min at 100% CPU I killed
 it. I ran it separately by hand and get output about missing OpenGL
 drivers (which is true, I'm building kdiagram in a restricted
 environment; I didn't originally expect to need to have DBUS running
 to be able to do the tests either) and debug output like:
 QDEBUG : TestDrawIntoPainter::testTest() Time for drawing pixmap
 :/test: 53682 ms
   Is that test supposed to take so much longer (minutes) than the
 other tests (deciseconds)?
 
I guess so. The test is commented out in the .pro file in the KDAB 
version - the reason is probably that it takes so long.

 So from a it-compiles-and-the-tests-pass point of view on my platform,
 it looks good.
 
 [ade]

Re: Relicensing parts of kglobalacceld

[Andreas Hartmetz]

On Monday 15 December 2014 18:59:56 Martin Klapetek wrote:
 Hey guys,
 
 we're in the process of moving kglobalacceld as part of the
 kglobalaccel framework.
 The framework is licensed under LGPLv2.1+ and the runtime daemon was
 licensed
 under LGPLv2+ so it got relicensed to LGPLv2.1+.
 
 But one file, the main.cpp, is still LGPLv2 and you're the copyright
 holders.
 May I have your permission to relicense this one file to LGPLv2.1+
 [1]?
 
You have my permission to relicense to LGPLv2.1+

 Also CC'ing the kde-frameworks-devel for public archiving purposes,
 please keep that
 in the CC (or answer there directly).
 
 [1] https://www.gnu.org/licenses/lgpl-2.1.html ...or later.
 
 Thank you

Cheers,
Andreas Hartmetz
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 120139: kill warning

[Andreas Hartmetz]

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


It looks like the warning is indeed misguided, but it is not clear that just 
ignoring that an assumption was violated is the right thing to do.

- Andreas Hartmetz


On Sept. 11, 2014, 10:48 vorm., Sune Vuorela wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120139/
 ---
 
 (Updated Sept. 11, 2014, 10:48 vorm.)
 
 
 Review request for KDE Frameworks, Andreas Hartmetz and Stephen Kelly.
 
 
 Repository: kitemviews
 
 
 Description
 ---
 
 The warninng is even triggered by from internal KWidgetItemDelegatePrivate, 
 so seems to be wrong
 
 Try out the kwidgetitemdelegate test application
 
 
 Diffs
 -
 
   src/kwidgetitemdelegatepool.cpp d61802e 
 
 Diff: https://git.reviewboard.kde.org/r/120139/diff/
 
 
 Testing
 ---
 
 Warning not printed.
 
 
 Thanks,
 
 Sune Vuorela
 


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

Re: nepomuk/baloo

[Andreas Hartmetz]

On Friday 14 March 2014 07:13:16 Lindsay Mathieson wrote:
 Further to a question on kdepim - should nepomuk_file_indexer be running as
 well as baloo_file?
 
 Or is nepomuk_file_indexer obsolete?

The Nepomuk indexer is obsolete if you are using Baloo, which is
intended to replace Nepomuk.
There is also a Baloo / Akonadi indexer agent, which you might not see
running because it's so fast :)
The file_indexer processes are both only for actual files AFAIK, not
Akonadi resources. The Akonadi indexers are called indexing agents
and have agent somewhere in their names.
There is or used to be (a recent commit should have fixed it) a bug
where even if your self-compiled KDE does not include Nepomuk,
something starts the Nepomuk / Akonadi agent that usually comes with
your distro packaging of KDE.

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

Re: Announce: A useful d-bus analyzer

[Andreas Hartmetz]

On Friday 24 January 2014 14:21:15 Àlex Fiestas wrote:
 On Tuesday 14 January 2014 19:54:14 Andreas Hartmetz wrote:
  Hello,
  
  I've worked on and off on an implementation of the d-bus protocol
  which, of course, needs a proof of concept. That proof of concept is
  a d-bus analyzer, probably the best d-bus analyzer around if your use
  case isn't exactly what Bustle covers - in that case, Bustle is the
  best.
  The analyzer can:
  - filter messages on various header fields
  - group calls with returns
  - displays call/return latency as observed from the d-bus daemon
  - show a list of currently unanswered calls (helps find those that
  
take really long or time out)
  
  - pause, continue and reset capturing
  - save and load captures
  - display the full types and contents of messages in a nice format
  - associate calls with returns in order to obtain nicer sender and
  
receiver addresses - in the screenshot, the address parts in
parentheses are obtained that way
  
  Screenshot: http://i.imgur.com/CK1ejcb.png
  
  With the library part, I tried to make it nice to use and fast because
  those features must be there from the beginning if they are ever going
  to be there at all. What it is not is complete (not even close) or even
  memory leak proof under some circumstances. The parts that are not
  core marshalling are also not as optimized as they could be.
  
  The thing is called Dferry and can be found under Playground/SDK
  on projects.kde.org or just kde:dferry in git if you have that git
  shortcut set up.
  After building and installing, the analyzer is called dfer-analyzer.
  
  Cheers,
  Andreas
 
 This looks awesome !
 
 I remember a conversation between you and Thiago to make use of your
 marshaller instead of libdbus, is that still in place?
 
Well, kdbus has switched to GVariant marshalling in the meantime.
I heard that about a week ago and I'm not particularly happy about it.
The serialization format is incompatible (although somewhat similar)
and there are even some semantic differences like an option type and a
fixed length array type that don't exist in libdbus. That is obviously
a problem if you try to bridge the formats both ways.
I guess I could implement the GVariant format: it's not more difficult
than the old format, most of the unit tests will be applicable, and I
know how to handle the tricky cases that can occur in that kind of
serialization format.
Also the GVariant code is not crap unlike libdbus, so it can serve as
an example.

For the benefit of any semi-involved onlookers let me state very
clearly here:
- kdbus is a multicast local socket type in the Linux kernel.
  It knows almost nothing about d-bus. It's just a few hundred lines.
- the user-space part that implements the d-bus protocol lives in the
  systemd repository and is called libystemd (no kidding). It's about
  23k lines. That is what people mean most of the time when they say
  kdbus. I'm using it like that as well because it has no proper name
  otherwise.

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

Re: Announce: A useful d-bus analyzer

[Andreas Hartmetz]

On Friday 24 January 2014 21:40:30 Andreas Hartmetz wrote:
 On Friday 24 January 2014 14:21:15 Àlex Fiestas wrote:
  On Tuesday 14 January 2014 19:54:14 Andreas Hartmetz wrote:
   Hello,
   
   I've worked on and off on an implementation of the d-bus protocol
   which, of course, needs a proof of concept. That proof of concept is
   a d-bus analyzer, probably the best d-bus analyzer around if your use
   case isn't exactly what Bustle covers - in that case, Bustle is the
   best.
   The analyzer can:
   - filter messages on various header fields
   - group calls with returns
   - displays call/return latency as observed from the d-bus daemon
   - show a list of currently unanswered calls (helps find those that
   
 take really long or time out)
   
   - pause, continue and reset capturing
   - save and load captures
   - display the full types and contents of messages in a nice format
   - associate calls with returns in order to obtain nicer sender and
   
 receiver addresses - in the screenshot, the address parts in
 parentheses are obtained that way
   
   Screenshot: http://i.imgur.com/CK1ejcb.png
   
   With the library part, I tried to make it nice to use and fast because
   those features must be there from the beginning if they are ever going
   to be there at all. What it is not is complete (not even close) or even
   memory leak proof under some circumstances. The parts that are not
   core marshalling are also not as optimized as they could be.
   
   The thing is called Dferry and can be found under Playground/SDK
   on projects.kde.org or just kde:dferry in git if you have that git
   shortcut set up.
   After building and installing, the analyzer is called dfer-analyzer.
   
   Cheers,
   Andreas
  
  This looks awesome !
  
  I remember a conversation between you and Thiago to make use of your
  marshaller instead of libdbus, is that still in place?
 
 Well, kdbus has switched to GVariant marshalling in the meantime.
 I heard that about a week ago and I'm not particularly happy about it.
 The serialization format is incompatible (although somewhat similar)
 and there are even some semantic differences like an option type and a
 fixed length array type that don't exist in libdbus. That is obviously
 a problem if you try to bridge the formats both ways.
 I guess I could implement the GVariant format: it's not more difficult
 than the old format, most of the unit tests will be applicable, and I
 know how to handle the tricky cases that can occur in that kind of
 serialization format.
 Also the GVariant code is not crap unlike libdbus, so it can serve as
 an example.
 
 For the benefit of any semi-involved onlookers let me state very
 clearly here:
 - kdbus is a multicast local socket type in the Linux kernel.
   It knows almost nothing about d-bus. It's just a few hundred lines.
 - the user-space part that implements the d-bus protocol lives in the
   systemd repository and is called libystemd (no kidding). It's about

Eh, of course it's libsystemd.

   23k lines. That is what people mean most of the time when they say
   kdbus. I'm using it like that as well because it has no proper name
   otherwise.


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

Announce: A useful d-bus analyzer

[Andreas Hartmetz]

Hello,

I've worked on and off on an implementation of the d-bus protocol
which, of course, needs a proof of concept. That proof of concept is
a d-bus analyzer, probably the best d-bus analyzer around if your use
case isn't exactly what Bustle covers - in that case, Bustle is the
best.
The analyzer can:
- filter messages on various header fields
- group calls with returns
- displays call/return latency as observed from the d-bus daemon
- show a list of currently unanswered calls (helps find those that
  take really long or time out)
- pause, continue and reset capturing
- save and load captures
- display the full types and contents of messages in a nice format
- associate calls with returns in order to obtain nicer sender and
  receiver addresses - in the screenshot, the address parts in
  parentheses are obtained that way
Screenshot: http://i.imgur.com/CK1ejcb.png

With the library part, I tried to make it nice to use and fast because
those features must be there from the beginning if they are ever going
to be there at all. What it is not is complete (not even close) or even
memory leak proof under some circumstances. The parts that are not
core marshalling are also not as optimized as they could be.

The thing is called Dferry and can be found under Playground/SDK
on projects.kde.org or just kde:dferry in git if you have that git
shortcut set up.
After building and installing, the analyzer is called dfer-analyzer.

Cheers,
Andreas

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

Re: Nepomuk in 4.13 and beyond

[Andreas Hartmetz]

On Thursday 19 December 2013 16:16:32 Sebastian Kügler wrote:
 On Thursday, December 19, 2013 14:48:57 Vishesh Handa wrote:
  On Wednesday 18 Dec 2013 12:09:02 François K. wrote:
   Hi Vishesh, hi guys,
   
   
   
   I'm sorry to short-circuit the thread. I deleted Vishesh's original
   email
   by mistake...
   
   
   
   Well, that sounds really exciting ! Thanks again for your work.
   
   
   
   Here are a few thoughts/questions I have since you've made the
   announcement. They might be a bit technical, I hope that's not a problem
   (should I start a new thread ?).
   
   
   
   * What are the plans to store tags ? On OSX, tags are stored in files
   xattrs which is -IMHO- very nice : - Metadata live and die with the file
   ;
   
 - No store query when you move or copy a file ;
 - You don't rely on a store to tag files ;
 - You also don't end with a huge store full of unuseful things like it
   
   used to happen with Nepomuk some time ago (no offense) ; - You can
   easily
   backup the metadata (at least files metadata) : you just have to use a
   decent backup tool that handles xattrs ; - It's CLI-friendly ;
   
 - ...
  
  +1
  
  I'm leaning towards this as well.
 
 To my knowledge, the list of filesystems with proper xattr support is rather
 short. 
From Wikipedia In Linux, the ext2, ext3, ext4, JFS, ReiserFS, XFS, Btrfs and 
OCFS2 1.6 filesystems support extended attributes.
That list includes all filesystems you are going to use on a desktop system.

 This means, a fallback mechanism is needed, at which time one has to
 ask if the primary mechanism is really needed. Programs like cp don't
 seem to consider xattr by default, so the CLI friendly is limited to if
 you remember to copy xattr as well.
 
 The advantages of xattr are quite limited due to this, it's definitely not a
 silver bullet.
 
A big advantage that I see is performance, both I/O and CPU. A database
of attributes means
- File changes (moves, renames etc) need to be tracked
- Every file change triggers a read from and (if it has tags) a write
  to the database, which is an effective I/O multiplier or put another
  way an I/O performance divisor.
  Note that while writes can be cached, reads from blocks that aren't
  accidentally already in RAM cannot be cached. If the data is needed
  ASAP, it must be read right away. So the database reads when moving
  files could easily generate 10x the disk traffic of the easily
  cacheable and mergeable original write I/O.
It may or may not be possible to avoid the I/O multiplication problem
for full-text search. At least there one doesn't absolutely have to
keep track of all changes since the data is in the file itself, so the
file can be asynchronously re-indexed in its new location. If the file
is excluded from indexing, it's even easier. This is not possible for
tags where you need to check for every moved file if it has tags.
It is hard to overstate the severity of this problem.

Another general consideration is that, if some change in the kernel
would help us tremendously (e.g. in file change tracking APIs), we
should try to effect such a change. We in the KDE community tend to
treat the kernel as out of our influence, which does not need to be so.

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

Re: Review Request 111335: Fix for one of the oldest KIO bugs: multiple dialogs when KIO encounters SSL errors

[Andreas Hartmetz]

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



kio/kio/scheduler.cpp
http://git.reviewboard.kde.org/r/111335/#comment26051

Just return a QString. With the pointer you have more verbosity in the 
caller, and the state of key is unclear after the call because it may or may 
not have been changed.

The Request could be passed as a const reference here, so why not do that?

Would it make sense to have this method in the Request class as
QString key() const ?



kio/kio/scheduler.cpp
http://git.reviewboard.kde.org/r/111335/#comment26055

braces



kio/kio/scheduler.cpp
http://git.reviewboard.kde.org/r/111335/#comment26043

kdelibs style: always use braces (even if this code is copied from 
somewhere else)



kio/kio/scheduler.cpp
http://git.reviewboard.kde.org/r/111335/#comment26050

What the heck did I do there? :O
(/*H4X*/ is something I use to mark temporary hacks)



kio/kio/scheduler.cpp
http://git.reviewboard.kde.org/r/111335/#comment26046

m_cachedResults is never cleaned - I admit that this is not much different 
from the SSL config file never losing exceptions stored there.

So I suggest cleaning m_cachedResults, but the current patch is not 
significantly worse than what we have.

This is related to making an SSL exception for current sessions only, 
which is actually for 30 minutes. Unless you also make it really for current 
sessions, after 30 minutes would be a good time to clear such entries.



kio/kio/scheduler.cpp
http://git.reviewboard.kde.org/r/111335/#comment26048

Should be easier to just make it a child of the public class.



kio/kio/slaveinterface.cpp
http://git.reviewboard.kde.org/r/111335/#comment26049

Early return for less nesting


- Andreas Hartmetz


On July 1, 2013, 5:10 a.m., Dawit Alemayehu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/111335/
 ---
 
 (Updated July 1, 2013, 5:10 a.m.)
 
 
 Review request for kdelibs.
 
 
 Description
 ---
 
 The attached patch addresses one of the oldest bugs in KIO. Due to the 
 muti-process nature of KIO, if any of the ioslaves encounter something that 
 requires user input, the user might end up getting prompted multiple times. 
 The best example of this is SSL error warnings sent to the client by 
 kio_http. The patch completely resolves this problem using the same approach 
 as kpasswdserver, but without the need for an additional kded process.
 
 
 This addresses bugs 154100 and 265228.
 http://bugs.kde.org/show_bug.cgi?id=154100
 http://bugs.kde.org/show_bug.cgi?id=265228
 
 
 Diffs
 -
 
   kio/kio/scheduler.h 04edb40 
   kio/kio/scheduler.cpp 802f8b8 
   kio/kio/scheduler_p.h d68f645 
   kio/kio/slaveinterface.h 4bfcec8 
   kio/kio/slaveinterface.cpp aa0fc44 
   kio/kio/slaveinterface_p.h e31ec5e 
 
 Diff: http://git.reviewboard.kde.org/r/111335/diff/
 
 
 Testing
 ---
 
 Visit a site that throws up SSL warnings and causes KIO to show more than one 
 error dialog.
 
 
 Thanks,
 
 Dawit Alemayehu
 

Re: Review Request 111370: system_tray: initialize uninitialized members

[Andreas Hartmetz]

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



plasma/generic/applets/systemtray/ui/widgetitem.cpp
http://git.reviewboard.kde.org/r/111370/#comment26060

Two style issues: put the m_applet initialization on a new line (with the 
comma on the previous line), and use 0, not NULL.


- Andreas Hartmetz


On July 2, 2013, 6:15 p.m., Jiri Slaby wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/111370/
 ---
 
 (Updated July 2, 2013, 6:15 p.m.)
 
 
 Review request for kde-workspace and Kai Uwe Broulik.
 
 
 Description
 ---
 
 A member in WidgetItem is used unitialized. Fix this.
 
 See https://bugs.kde.org/show_bug.cgi?id=321828
 
 
 Diffs
 -
 
   plasma/generic/applets/systemtray/ui/widgetitem.cpp 
 1f922c921c0f3763e8ffeb4466fbffb527fca2f2 
 
 Diff: http://git.reviewboard.kde.org/r/111370/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jiri Slaby
 

Re: Review Request 111370: system_tray: initialize uninitialized members

[Andreas Hartmetz]

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



plasma/generic/applets/systemtray/ui/widgetitem.cpp
http://git.reviewboard.kde.org/r/111370/#comment26086

Please don't use tabs and align it with the previous item.


- Andreas Hartmetz


On July 3, 2013, 10:54 a.m., Jiri Slaby wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/111370/
 ---
 
 (Updated July 3, 2013, 10:54 a.m.)
 
 
 Review request for kde-workspace and Kai Uwe Broulik.
 
 
 Description
 ---
 
 A member in WidgetItem is used unitialized. Fix this.
 
 See https://bugs.kde.org/show_bug.cgi?id=321828
 
 
 Diffs
 -
 
   plasma/generic/applets/systemtray/ui/widgetitem.cpp 
 1f922c921c0f3763e8ffeb4466fbffb527fca2f2 
 
 Diff: http://git.reviewboard.kde.org/r/111370/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jiri Slaby
 

Re: Review Request 111370: system_tray: initialize uninitialized members

[Andreas Hartmetz]

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



plasma/generic/applets/systemtray/ui/widgetitem.cpp
http://git.reviewboard.kde.org/r/111370/#comment26087

Sorry about the ridiculous ping-pong, but that still isn't right. I should 
maybe have pointed you to some other file with an example. In any case, this is 
the usual style:

Foo::Foo(QObject *parent)
: QObject(parent),
  m_someVariable(0),
  m_someOtherVariable(bar)
{
}


- Andreas Hartmetz


On July 3, 2013, 11:58 a.m., Jiri Slaby wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/111370/
 ---
 
 (Updated July 3, 2013, 11:58 a.m.)
 
 
 Review request for kde-workspace and Kai Uwe Broulik.
 
 
 Description
 ---
 
 A member in WidgetItem is used unitialized. Fix this.
 
 See https://bugs.kde.org/show_bug.cgi?id=321828
 
 
 Diffs
 -
 
   plasma/generic/applets/systemtray/ui/widgetitem.cpp 
 1f922c921c0f3763e8ffeb4466fbffb527fca2f2 
 
 Diff: http://git.reviewboard.kde.org/r/111370/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jiri Slaby
 

Re: Review Request 111370: system_tray: initialize uninitialized members

[Andreas Hartmetz]

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

Ship it!


Congratulations :P

- Andreas Hartmetz


On July 3, 2013, 3:53 p.m., Jiri Slaby wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/111370/
 ---
 
 (Updated July 3, 2013, 3:53 p.m.)
 
 
 Review request for kde-workspace and Kai Uwe Broulik.
 
 
 Description
 ---
 
 A member in WidgetItem is used unitialized. Fix this.
 
 See https://bugs.kde.org/show_bug.cgi?id=321828
 
 
 Diffs
 -
 
   plasma/generic/applets/systemtray/ui/widgetitem.cpp 
 1f922c921c0f3763e8ffeb4466fbffb527fca2f2 
 
 Diff: http://git.reviewboard.kde.org/r/111370/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Jiri Slaby
 

Re: Finalized proposal for i18n in KF5, second iteration

[Andreas Hartmetz]

On Tuesday 02 April 2013 15:46:52 Chusslove Illich wrote:
 After considering comments from the previous iteration:
 
  
 http://mail.kde.org/pipermail/kde-frameworks-devel/2012-December/001358.htm
 l
 
 here is the updated version:
 
   http://nedohodnik.net/misc/ki18n-kf5-02/html/prg_guide.html
   http://nedohodnik.net/misc/ki18n-kf5-02/klocalizedstring.h
   http://nedohodnik.net/misc/ki18n-kf5-02/kuitmarkup.h
 
 and the diffs:
 
   http://nedohodnik.net/misc/ki18n-kf5-02/ki18n_doc-01-to-02.diff
   http://nedohodnik.net/misc/ki18n-kf5-02/klocalizedstring-01-to-02.diff
 
 The non-trivial changes and additions are these:
 
 * New section Connecting to Catalogs in Non-Code Files, that is in .ui,
   .rc and .kcfg files.
 
 * Macros I18N_NOOP2 and I18N_NOOP2_NOSTRIP are now deprecated, replaced with
 I18NC_NOOP.
 
 * Renamed the TRANSLATION_CATALOG define to TRANSLATION_DOMAIN, to be more
   in line with Gettext terminology.
 
 * Reduced capitalization in markup-related identifiers from KUIT* to Kuit*,
   similar to other acronym-containing identifiers in KDE.
 
 One important questions is still open, namely whether the variable-argument
 macro expansion is supported for all target compilers. I understood that
 there is no C++03 standard for this, while in C99 and C++0x it is:
 
   #define i18n(...) i18nd(TRANSLATION_DOMAIN, __VA_ARGS__)

It's supported in GCC and reasonably recent MSVC, around 2004 IIRC. It is used 
in the kDebug() macro, if available. You can find the details in kdebug.h.
What else do we support: Solaris, embedded toolchains? Most embedded 
toolchains are based on GCC (Android, QNX/Blackberry) or Clang (iOS). While I 
haven't found something official about support in Clang, __VA_ARGS__ occurs 
over 
a hundred times in Clang's source code and Clang is self-hosting. So it seems 
to be supported.
That only leaves Solaris and possibly exotic embedded compilers where we 
don't know yet.

Re: Review Request 109748: kioslave/sftp: Show correct port number when connecting to default port

[Andreas Hartmetz]

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

Ship it!


Ship It!

- Andreas Hartmetz


On March 26, 2013, 8:18 p.m., Thomas Fischer wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/109748/
 ---
 
 (Updated March 26, 2013, 8:18 p.m.)
 
 
 Review request for KDE Runtime.
 
 
 Description
 ---
 
 Currently, if you make a sftp connection through the kio slave, this slave 
 shows port number 0 to the user in the status bar (and as debug output) 
 instead of the ssh port number 22. This is purely an UI issue, as internally 
 the port number is tested on 0 using the default port number as a fall back 
 if the test fails.
 The attached patch address this issue by testing for port number 0. If the 
 port number is invalid, the default port number is shown instead.
 
 
 Diffs
 -
 
   kioslave/sftp/kio_sftp.cpp f493477 
 
 Diff: http://git.reviewboard.kde.org/r/109748/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Thomas Fischer
 

Review Request 109675: Make sure that the KDE prefix comes first in XDG_DATA_DIRS

[Andreas Hartmetz]

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

Review request for kdelibs.


Description
---

Planned commit message:

Make sure that the KDE prefix comes first in XDG_DATA_DIRS.

I tracked down a Nepomuk problem to this. Nepomuk file indexing didn't
work because the ontologies were too old. Nepomuk loaded ontologies
from /usr/share instead of my KDE prefix /opt/kde4/share, because
/opt/kde4 was the very last entry in the respective search list in
KStandardDirs. The first entries in that search list all came from
XDG_DATA_DIRS, which in my case (Kubuntu) is set by the X session
initialization scripts. That is before startkde runs, so startkde
never touched XDG_DATA_DIRS. But it should, and now it does.


Diffs
-

  startkde.cmake 8361fe0 

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


Testing
---


Thanks,

Andreas Hartmetz

Re: Review Request 109675: Make sure that the KDE prefix comes first in XDG_DATA_DIRS

[Andreas Hartmetz]

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

(Updated March 23, 2013, 6:37 p.m.)


Review request for kdelibs and Vishesh Handa.


Description
---

Planned commit message:

Make sure that the KDE prefix comes first in XDG_DATA_DIRS.

I tracked down a Nepomuk problem to this. Nepomuk file indexing didn't
work because the ontologies were too old. Nepomuk loaded ontologies
from /usr/share instead of my KDE prefix /opt/kde4/share, because
/opt/kde4 was the very last entry in the respective search list in
KStandardDirs. The first entries in that search list all came from
XDG_DATA_DIRS, which in my case (Kubuntu) is set by the X session
initialization scripts. That is before startkde runs, so startkde
never touched XDG_DATA_DIRS. But it should, and now it does.


Diffs
-

  startkde.cmake 8361fe0 

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


Testing
---


Thanks,

Andreas Hartmetz

Re: ktcpsockettest again

[Andreas Hartmetz]

On Saturday 19 January 2013 12:21:57 David Faure wrote:
 Hi Andreas,
 
 Can you shed some light on the intent of this famous test?
 
 s-write(HTTPREQUEST);
 s-waitForReadyRead();
 s-close();
 //What happens is that during waitForReadyRead() the write buffer is
 written out //completely so that the socket can shut down without having to
 wait for writeout. QCOMPARE(stateToString(s-state()),
 stateToString(KTcpSocket::UnconnectedState));
 
 What the comment says above, doesn't appear to be true. We get
 ClosingState now and then. If I check s-bytesAvailable() after
 waitForReadyRead(), it's 4344.
 
 Here's debug output from when it works:
 
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::close()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost()
 emits stateChanged()(ClosingState) QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::disconnectFromHost() disconnecting immediately QDEBUG :
 KTcpSocketTest::statesIana() QAbstractSocketPrivate::resetSocketLayer()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::bytesAvailable() ==
 4344 QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::disconnectFromHost() disconnected! QDEBUG :
 KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost() closed!
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::close()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::~QAbstractSocket()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::~QAbstractSocket()
 
 And here's debug output from when it fails:
 
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::close()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost()
 emits stateChanged()(ClosingState) QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::disconnectFromHost() delaying disconnect QDEBUG :
 KTcpSocketTest::statesIana() QAbstractSocket::close()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost()
 return from delayed close QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::disconnectFromHost() delaying disconnect FAIL!  :
 KTcpSocketTest::statesIana() Compared values are not the same Actual
 (stateToString(s-state())): ClosingState
Expected (stateToString(KTcpSocket::UnconnectedState)): UnconnectedState
Loc: [/d/kde/src/4/kde/kdelibs/kdecore/tests/ktcpsockettest.cpp(245)]
 PASS   : KTcpSocketTest::cleanupTestCase()
 
 We go into delaying disconnect because  d-writeBuffer.size()==38 (the
 whole HTTP request is still in the write buffer).
 
 Hmm, isn't the problem that the test makes a first request, waits for data
 to be available, but doesn't read it, so readyRead is still true when
 making the 2nd request? Added a s-readData() ... not better. Here's the
 full log, please advise:
 
 PASS   : KTcpSocketTest::initTestCase()
 QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::QAbstractSocket(TcpSocket, QAbstractSocketPrivate ==
 0x1c50d90, parent == 0x0) QDEBUG : KTcpSocketTest::statesIana()
 ktcpsockettest  ( 21234 ) connects to bus qt_default_session_bus
 QDBusConnectionPrivate(0x1cf3c90) QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::QAbstractSocket(TcpSocket, QAbstractSocketPrivate ==
 0x1cfdfb0, parent == 0x1c49de0) QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::connectToHost(www.iana.org, 80, 3)... QDEBUG :
 KTcpSocketTest::statesIana() QAbstractSocket::connectToHost(www.iana.org,
 80) == false (connection in progress) QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::writeData(0x1cff0e8 GET / HTTP/1.1\nHost:
 www.example..., 38) == 38 QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::waitForBytesWritten(2500) QDEBUG :
 KTcpSocketTest::statesIana() QAbstractSocket::waitForConnected(2500) QDEBUG
 : KTcpSocketTest::statesIana() QAbstractSocket::waitForConnected(2500)
 doing host name lookup QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocketPrivate::_q_startConnecting(hostInfo == {192.0.32.8,
 2620:0:2D0:200::8}) QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocketPrivate::_q_connectToNextAddress(), connecting to
 192.0.32.8:80, 3 left to try QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocketPrivate::resetSocketLayer() QDEBUG :
 KTcpSocketTest::statesIana()
 QAbstractSocketPrivate::initSocketLayer(TcpSocket, IPv4Protocol) success
 QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::waitForConnected(2500) waiting 2.50 secs for connection
 attempt #1 QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocketPrivate::fetchConnectionParameters() connection to
 192.0.32.8:80 established QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::waitForConnected(2500) == true QDEBUG :
 KTcpSocketTest::statesIana() QAbstractSocketPrivate::canWriteNotification()
 flushing QDEBUG : 

Re: ktcpsockettest again

[Andreas Hartmetz]

On Saturday 19 January 2013 12:21:57 David Faure wrote:
 Hi Andreas,
 
 Can you shed some light on the intent of this famous test?
 
 s-write(HTTPREQUEST);
 s-waitForReadyRead();
 s-close();
 //What happens is that during waitForReadyRead() the write buffer is
 written out //completely so that the socket can shut down without having to
 wait for writeout. QCOMPARE(stateToString(s-state()),
 stateToString(KTcpSocket::UnconnectedState));
 
 What the comment says above, doesn't appear to be true. We get
 ClosingState now and then. If I check s-bytesAvailable() after
 waitForReadyRead(), it's 4344.
 
 Here's debug output from when it works:
 
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::close()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost()
 emits stateChanged()(ClosingState) QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::disconnectFromHost() disconnecting immediately QDEBUG :
 KTcpSocketTest::statesIana() QAbstractSocketPrivate::resetSocketLayer()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::bytesAvailable() ==
 4344 QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::disconnectFromHost() disconnected! QDEBUG :
 KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost() closed!
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::close()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::~QAbstractSocket()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::~QAbstractSocket()
 
 And here's debug output from when it fails:
 
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::close()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost()
 emits stateChanged()(ClosingState) QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::disconnectFromHost() delaying disconnect QDEBUG :
 KTcpSocketTest::statesIana() QAbstractSocket::close()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost()
 QDEBUG : KTcpSocketTest::statesIana() QAbstractSocket::disconnectFromHost()
 return from delayed close QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::disconnectFromHost() delaying disconnect FAIL!  :
 KTcpSocketTest::statesIana() Compared values are not the same Actual
 (stateToString(s-state())): ClosingState
Expected (stateToString(KTcpSocket::UnconnectedState)): UnconnectedState
Loc: [/d/kde/src/4/kde/kdelibs/kdecore/tests/ktcpsockettest.cpp(245)]
 PASS   : KTcpSocketTest::cleanupTestCase()
 
 We go into delaying disconnect because  d-writeBuffer.size()==38 (the
 whole HTTP request is still in the write buffer).
 
 Hmm, isn't the problem that the test makes a first request, waits for data
 to be available, but doesn't read it, so readyRead is still true when
 making the 2nd request? Added a s-readData() ... not better. Here's the
 full log, please advise:
 
 PASS   : KTcpSocketTest::initTestCase()
 QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::QAbstractSocket(TcpSocket, QAbstractSocketPrivate ==
 0x1c50d90, parent == 0x0) QDEBUG : KTcpSocketTest::statesIana()
 ktcpsockettest  ( 21234 ) connects to bus qt_default_session_bus
 QDBusConnectionPrivate(0x1cf3c90) QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::QAbstractSocket(TcpSocket, QAbstractSocketPrivate ==
 0x1cfdfb0, parent == 0x1c49de0) QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::connectToHost(www.iana.org, 80, 3)... QDEBUG :
 KTcpSocketTest::statesIana() QAbstractSocket::connectToHost(www.iana.org,
 80) == false (connection in progress) QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::writeData(0x1cff0e8 GET / HTTP/1.1\nHost:
 www.example..., 38) == 38 QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::waitForBytesWritten(2500) QDEBUG :
 KTcpSocketTest::statesIana() QAbstractSocket::waitForConnected(2500) QDEBUG
 : KTcpSocketTest::statesIana() QAbstractSocket::waitForConnected(2500)
 doing host name lookup QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocketPrivate::_q_startConnecting(hostInfo == {192.0.32.8,
 2620:0:2D0:200::8}) QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocketPrivate::_q_connectToNextAddress(), connecting to
 192.0.32.8:80, 3 left to try QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocketPrivate::resetSocketLayer() QDEBUG :
 KTcpSocketTest::statesIana()
 QAbstractSocketPrivate::initSocketLayer(TcpSocket, IPv4Protocol) success
 QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::waitForConnected(2500) waiting 2.50 secs for connection
 attempt #1 QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocketPrivate::fetchConnectionParameters() connection to
 192.0.32.8:80 established QDEBUG : KTcpSocketTest::statesIana()
 QAbstractSocket::waitForConnected(2500) == true QDEBUG :
 KTcpSocketTest::statesIana() QAbstractSocketPrivate::canWriteNotification()
 flushing QDEBUG : 

Re: RFC: Enabling -DKDE4_BUILD_TESTS=ON by default

[Andreas Hartmetz]

On Monday 08 October 2012 22:40:55 Albert Astals Cid wrote:
 Hi fellow kde-core-develers, i was thinking that from time to time someone
 commits some code that breaks the building of the tests.
 
 This happens because people is not aware that those parts are unit tested.
 
 I think that by enabling the building of the tests by default we at least
 make sure the tests will build and maybe when people see their code
 actually breaking the compilation of a unit test we get them to run them
 and maybe even improve them :-)
 
 Of course, enabling building the tests by defaults makes compilation times a
 bit longer, but I do think that the benefits far surpass the problems and
 for people with really slow machines and/or needs to recompile stuff a lot
 there is always the possibility that they can ask cmake not to build the
 tests.
 
 Comments?
 
Good idea, let's do it.
I have to fix a unit test once in a while to make things build for me again, 
because my build script does compile with tests.

 Cheers,
   Albert

Re: Review Request: Cleanup the use of HTTPProtocol::httpClose

[Andreas Hartmetz]

 On Oct. 12, 2011, 10:51 p.m., Andreas Hartmetz wrote:
  This changeset changes some important parts without obvious (to me) gain.
  Before I spend an hour or two thinking through all the cases, which may or 
  may not catch potential regressions, I'd like to know what this does for us.
  The current approach of acting a bit dumb seems more robust.
 
 Dawit Alemayehu wrote:
 Here is the gains we get out of this change:
 
 #1. Keep persistent connection on non-connection related errors. There is 
 no reason to tear down our connection to a server on errors such as 
 ERR_USR_CANCELED.
 #2. Avoid unnecessary performance hit by not calling httpClose multiple 
 times. This is especially true for ::get which is by the far the most called 
 function in this ioslave. For no good reason however, httpClose is called at 
 least two times everytime ::get is successfully invoked.
 
 BTW, if you tell me the use cases which I should test, then I will be 
 willing to do the regression testing. I have already gone through each of the 
 ioslave functions to see if this change will affect any of them. I have even 
 tested the most commonly used ones (get, post, put) through a proxy server to 
 see if there are any regressions. So far I have not encountered one.
 
 Dawit Alemayehu wrote:
 Can I commit this change ? Or do you still have objections to it ?

I am quite busy right now. If I don't get around to having a look until Monday 
morning, feel free to just ship it then. I'll try to do it, though.


- Andreas


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


On Oct. 25, 2011, 3:55 p.m., Dawit Alemayehu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/102801/
 ---
 
 (Updated Oct. 25, 2011, 3:55 p.m.)
 
 
 Review request for kdelibs and Andreas Hartmetz.
 
 
 Description
 ---
 
 This patch cleans up where and under what circumstances httpClose gets 
 called. This is done to avoid unnecessary invocation of httpClose. With this 
 patch the function will only get called under the following circumstances:
 
 #1. from functions that only call proceedUntilResponseHeader directly.
 #2. from proceedUntilResponseContent.
 #3. from error
 #4. from davFinished.
 
 The main purpose of this change is to avoid httpClose being called multiple 
 times on every GET request which is by far the most invoked call.
 
 
 Diffs
 -
 
   kioslave/http/http.h 4c62841 
   kioslave/http/http.cpp 235ce7d 
 
 Diff: http://git.reviewboard.kde.org/r/102801/diff/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Dawit Alemayehu
 

Re: Review Request: Cleanup the use of HTTPProtocol::httpClose

[Andreas Hartmetz]

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


This changeset changes some important parts without obvious (to me) gain.
Before I spend an hour or two thinking through all the cases, which may or may 
not catch potential regressions, I'd like to know what this does for us.
The current approach of acting a bit dumb seems more robust.

- Andreas Hartmetz


On Oct. 8, 2011, 5:16 a.m., Dawit Alemayehu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/102801/
 ---
 
 (Updated Oct. 8, 2011, 5:16 a.m.)
 
 
 Review request for kdelibs and Andreas Hartmetz.
 
 
 Description
 ---
 
 This patch cleans up where and under what circumstances httpClose gets 
 called. This is done to avoid unnecessary invocation of httpClose. With this 
 patch the function will only get called under the following circumstances:
 
 #1. from functions that only call proceedUntilResponseHeader directly.
 #2. from proceedUntilResponseContent.
 #3. from error
 #4. from davFinished.
 
 The main purpose of this change is to avoid httpClose being called multiple 
 times on every GET request which is by far the most invoked call.
 
 
 Diffs
 -
 
   kioslave/http/http.h 4c62841 
   kioslave/http/http.cpp 2862707 
 
 Diff: http://git.reviewboard.kde.org/r/102801/diff/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Dawit Alemayehu
 

Re: Review Request: kio_http: fix keepalive timeout parsing

[Andreas Hartmetz]

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

Ship it!



kioslave/http/http.cpp
http://git.reviewboard.kde.org/r/102822/#comment6377

I guess the parser just lowercases the key (keep-alive), not the values. 
Lowercasing the keys is okay because per the spec they are case-insensitive, 
and it has the advantage that you can look up keys in more or less constant 
time when using a hashtable.
In many cases the values are case sensitive (usernames, something 
Base64-encoded for example), so the parser better leaves them alone. So you 
need to normalize the case yourself.


- Andreas Hartmetz


On Oct. 10, 2011, 10:35 p.m., Andrea Iacovitti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/102822/
 ---
 
 (Updated Oct. 10, 2011, 10:35 p.m.)
 
 
 Review request for kdelibs, Andreas Hartmetz and Dawit Alemayehu.
 
 
 Description
 ---
 
 Keep-alive header can specify multiple, comma-separated, value pairs.
 For example what apache web server normally sends is something like that:
 
 Keep-Alive: timeout=5, max=99
 
 Actually kio_http fails to extract timeout value because it assumes
 keep-alive header can contain only a single value pair.
 In the case of example above what it end up to do is
 m_request.keepAliveTimeout = QString(5, max=99).toInt(), that returns 0 
 (wrong!).
 
 
 Diffs
 -
 
   kioslave/http/http.cpp 2862707 
   kioslave/http/parsinghelpers.cpp fc75d68 
 
 Diff: http://git.reviewboard.kde.org/r/102822/diff/diff
 
 
 Testing
 ---
 
 -Patched code compiles
 -Hacked a web server and made tests against following keep-alive header 
 variants:
  Keep-Alive: timeout=5, max=99
  Keep-Alive: Timeout=5, max=99 (uppercase 'T')
  Keep-Alive: Timeout=5 , max=99(extra space before comma)
 
 
 Thanks,
 
 Andrea Iacovitti
 

Re: Review Request: kio_http: fix keepalive timeout parsing

[Andreas Hartmetz]

 On Oct. 12, 2011, 10:58 p.m., Andreas Hartmetz wrote:
  kioslave/http/http.cpp, line 3107
  http://git.reviewboard.kde.org/r/102822/diff/1/?file=38514#file38514line3107
 
  I guess the parser just lowercases the key (keep-alive), not the 
  values. Lowercasing the keys is okay because per the spec they are 
  case-insensitive, and it has the advantage that you can look up keys in 
  more or less constant time when using a hashtable.
  In many cases the values are case sensitive (usernames, something 
  Base64-encoded for example), so the parser better leaves them alone. So you 
  need to normalize the case yourself.

Small addition: The values are key-value pairs again here, but that isn't 
universally so in HTTP headers. The header parser simply doesn't know about 
such details.


- Andreas


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


On Oct. 10, 2011, 10:35 p.m., Andrea Iacovitti wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/102822/
 ---
 
 (Updated Oct. 10, 2011, 10:35 p.m.)
 
 
 Review request for kdelibs, Andreas Hartmetz and Dawit Alemayehu.
 
 
 Description
 ---
 
 Keep-alive header can specify multiple, comma-separated, value pairs.
 For example what apache web server normally sends is something like that:
 
 Keep-Alive: timeout=5, max=99
 
 Actually kio_http fails to extract timeout value because it assumes
 keep-alive header can contain only a single value pair.
 In the case of example above what it end up to do is
 m_request.keepAliveTimeout = QString(5, max=99).toInt(), that returns 0 
 (wrong!).
 
 
 Diffs
 -
 
   kioslave/http/http.cpp 2862707 
   kioslave/http/parsinghelpers.cpp fc75d68 
 
 Diff: http://git.reviewboard.kde.org/r/102822/diff/diff
 
 
 Testing
 ---
 
 -Patched code compiles
 -Hacked a web server and made tests against following keep-alive header 
 variants:
  Keep-Alive: timeout=5, max=99
  Keep-Alive: Timeout=5, max=99 (uppercase 'T')
  Keep-Alive: Timeout=5 , max=99(extra space before comma)
 
 
 Thanks,
 
 Andrea Iacovitti
 

Re: Review Request: Proxy overhaul Part 4: More proxy changes and fixes for KProtocolManager

[Andreas Hartmetz]

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

Ship it!


Ship It!

- Andreas Hartmetz


On Sept. 25, 2011, 4:15 p.m., Dawit Alemayehu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/102691/
 ---
 
 (Updated Sept. 25, 2011, 4:15 p.m.)
 
 
 Review request for kdelibs.
 
 
 Description
 ---
 
 This patch is the 4th in the serious of patches designed to resolve bugs and 
 missing functionality in KDE's proxy manager. The changes made with this 
 patch are as follows:
 
 * Add code that resolves a request url's hostname before attempting to 
 match
   it against the no proxy for list so long as the 
 ResolveHostNamesBeforeProxyCheck
   option is set.
 
 * Allow DIRECT as a special keyword in the list of proxy server 
 addresses
   returned in slaveProtocol(const QString protocol, QStringList proxy).
 
 * Change KProtocolManager::proxyFor to properly handle the changes in the 
 new
   proxy management dialog (KDE 4.8) where the proxy server port, in the
   manual proxy configuration mode, will be saved separated from the 
 address with
   a white space.
 
 * Move the code that accounts for SOCKS proxy from 
 KProtocolManager::proxyFor
   to KProtocolManager::proxyForUrl where it belongs. The current 
 implementation
   only works correctly under one circumstance while breaking the previous 
 behavior
   of the function.
 
 * Fix KProtocoManager::proxiesForUrl so that it accounts for the proxy
   exception list.
 
 * Update API documentation to reflect the changes above.
 
 
 Diffs
 -
 
   kio/kio/kprotocolmanager.h 11e43fe 
   kio/kio/kprotocolmanager.cpp 50ebb6e 
 
 Diff: http://git.reviewboard.kde.org/r/102691/diff/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Dawit Alemayehu
 

Re: Review Request: Proxy overhaul Part 4: More proxy changes and fixes for KProtocolManager

[Andreas Hartmetz]

 On Sept. 25, 2011, 2:20 p.m., Andreas Hartmetz wrote:
  I'd actually be interested to hear which testing you did.
  
  The ResolveHostNamesBeforeProxyCheck option seems strange. In which 
  situations is this supposed to be set / not set?
 
 Dawit Alemayehu wrote:
 The ResolveHostNamesBeforeProxyCheck option is used to give the user 
 the ability to turn DNS lookups of request URLs on or off before checking 
 them against the No Proxy For list. This makes it possible for us to let 
 people enter IP address ranges, e.g. 192.168.0.1/24 in the NoProxyFor 
 list while at the same time protecting those people that do not want to have 
 any form of name resolution to happen at the client application level. The 
 default behavior is what it currently is, no lookup, since we do not support 
 IP address ranges right now.
 
 As far as testing goes, I created a basic SOCKS server using ssh, ssh -D 
  127.0.0.1, and a very basic PAC script file that contains the following:
 
 function FindProxyForURL( url, host )
 {
 var resolved_ip = dnsResolve(host);
 
 if (isInNet(resolved_ip, 127.0.0.1, 255.255.255.0))
 return DIRECT;
 
 return SOCKS 127.0.0.1:; DIRECT;
 }
 
 I am also in the process of testing all these changes agains a real proxy 
 sever. I am going to test against bother privoxy and squid. To test this 
 however, you also need the next patch in the series, 
 https://git.reviewboard.kde.org/r/102696/.

OK, I see what ResolveHostNamesBeforeProxyCheck does now. Thanks.


 On Sept. 25, 2011, 2:20 p.m., Andreas Hartmetz wrote:
  kio/kio/kprotocolmanager.cpp, line 471
  http://git.reviewboard.kde.org/r/102691/diff/1/?file=37173#file37173line471
 
  This looks more like
  if no proxy scheme is given, assume SOCKS
 
 Dawit Alemayehu wrote:
 Ahh... Did you mean, if no proxy could be found for the scheme of the 
 given url, assume SOCKS ? Even then that comment belongs to the if statement 
 above the one where this comment currently resides. Perhaps I should move the 
 comment down inside the if statement.

I am now more confused than before.
What I (still) think the code is doing is: if there is a proxy URL given with 
no scheme, prepend socks:// to the proxy URL.


- Andreas


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


On Sept. 25, 2011, 4:15 p.m., Dawit Alemayehu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/102691/
 ---
 
 (Updated Sept. 25, 2011, 4:15 p.m.)
 
 
 Review request for kdelibs.
 
 
 Description
 ---
 
 This patch is the 4th in the serious of patches designed to resolve bugs and 
 missing functionality in KDE's proxy manager. The changes made with this 
 patch are as follows:
 
 * Add code that resolves a request url's hostname before attempting to 
 match
   it against the no proxy for list so long as the 
 ResolveHostNamesBeforeProxyCheck
   option is set.
 
 * Allow DIRECT as a special keyword in the list of proxy server 
 addresses
   returned in slaveProtocol(const QString protocol, QStringList proxy).
 
 * Change KProtocolManager::proxyFor to properly handle the changes in the 
 new
   proxy management dialog (KDE 4.8) where the proxy server port, in the
   manual proxy configuration mode, will be saved separated from the 
 address with
   a white space.
 
 * Move the code that accounts for SOCKS proxy from 
 KProtocolManager::proxyFor
   to KProtocolManager::proxyForUrl where it belongs. The current 
 implementation
   only works correctly under one circumstance while breaking the previous 
 behavior
   of the function.
 
 * Fix KProtocoManager::proxiesForUrl so that it accounts for the proxy
   exception list.
 
 * Update API documentation to reflect the changes above.
 
 
 Diffs
 -
 
   kio/kio/kprotocolmanager.h 11e43fe 
   kio/kio/kprotocolmanager.cpp 50ebb6e 
 
 Diff: http://git.reviewboard.kde.org/r/102691/diff/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Dawit Alemayehu
 

Re: Review Request: Proxy overhaul Part 4: More proxy changes and fixes for KProtocolManager

[Andreas Hartmetz]

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


I'd actually be interested to hear which testing you did.

The ResolveHostNamesBeforeProxyCheck option seems strange. In which 
situations is this supposed to be set / not set?


kio/kio/kprotocolmanager.h
http://git.reviewboard.kde.org/r/102691/#comment6014

Returns a comma-separated list of hosts that should be contacted...



kio/kio/kprotocolmanager.h
http://git.reviewboard.kde.org/r/102691/#comment6015

overriding other proxy settings



kio/kio/kprotocolmanager.h
http://git.reviewboard.kde.org/r/102691/#comment6008

What is a partial host name? I know that the wording isn't your fault, but 
maybe you know what it means.



kio/kio/kprotocolmanager.h
http://git.reviewboard.kde.org/r/102691/#comment6013

Good.



kio/kio/kprotocolmanager.h
http://git.reviewboard.kde.org/r/102691/#comment6007

This is not a method that allocates a resource, for which request is the 
right word. It's a method that answers a question.
I would suggest something like Returns the proxy server to use to contact 
@p url, or DIRECT if no proxy should be used.

I don't understand the part about the empty string at all. How is it 
different from DIRECT?



kio/kio/kprotocolmanager.h
http://git.reviewboard.kde.org/r/102691/#comment6009

QString::null usage is frowned upon. Just say empty string.



kio/kio/kprotocolmanager.h
http://git.reviewboard.kde.org/r/102691/#comment6010

See above. Client programmers don't request a proxy (the meaning of that is 
closer to create me one), they ask for the address of an existing one.



kio/kio/kprotocolmanager.h
http://git.reviewboard.kde.org/r/102691/#comment6011

What's the use case here? Do you expect ioslaves to try all proxies from 
the list? Is the list ordered in some way?

How is the list of proxy server addresses related to @p url? It's obvious 
from the name, but it is good practice to state all the important facts in the 
apidox.



kio/kio/kprotocolmanager.cpp
http://git.reviewboard.kde.org/r/102691/#comment6012

This looks more like
if no proxy scheme is given, assume SOCKS


- Andreas Hartmetz


On Sept. 25, 2011, 2:08 a.m., Dawit Alemayehu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/102691/
 ---
 
 (Updated Sept. 25, 2011, 2:08 a.m.)
 
 
 Review request for kdelibs.
 
 
 Description
 ---
 
 This patch is the 4th in the serious of patches designed to resolve bugs and 
 missing functionality in KDE's proxy manager. The changes made with this 
 patch are as follows:
 
 * Add code that resolves a request url's hostname before attempting to 
 match
   it against the no proxy for list so long as the 
 ResolveHostNamesBeforeProxyCheck
   option is set.
 
 * Allow DIRECT as a special keyword in the list of proxy server 
 addresses
   returned in slaveProtocol(const QString protocol, QStringList proxy).
 
 * Change KProtocolManager::proxyFor to properly handle the changes in the 
 new
   proxy management dialog (KDE 4.8) where the proxy server port, in the
   manual proxy configuration mode, will be saved separated from the 
 address with
   a white space.
 
 * Move the code that accounts for SOCKS proxy from 
 KProtocolManager::proxyFor
   to KProtocolManager::proxyForUrl where it belongs. The current 
 implementation
   only works correctly under one circumstance while breaking the previous 
 behavior
   of the function.
 
 * Fix KProtocoManager::proxiesForUrl so that it accounts for the proxy
   exception list.
 
 * Update API documentation to reflect the changes above.
 
 
 Diffs
 -
 
   kio/kio/kprotocolmanager.h 11e43fe 
   kio/kio/kprotocolmanager.cpp 50ebb6e 
 
 Diff: http://git.reviewboard.kde.org/r/102691/diff/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Dawit Alemayehu
 

Re: Review Request: Proxy overhaul Part 5: Add support for trying multiple proxies to KIO HTTP

[Andreas Hartmetz]

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


Again, I'd like to know which testing you did.


kio/kio/tcpslavebase.h
http://git.reviewboard.kde.org/r/102696/#comment6021

if not a null pointer, *errorString will be set to contain more 
information about a connect error, or to the empty string if there was no 
error.



kio/kio/tcpslavebase.cpp
http://git.reviewboard.kde.org/r/102696/#comment6022

Please always clear *errorString on no error. That makes the API nicer to 
use because the value of *errorString will depend strictly on the result of 
connectToHost, not on its previous value if any.



kioslave/http/http.cpp
http://git.reviewboard.kde.org/r/102696/#comment6016

int connectError = 0;



kioslave/http/http.cpp
http://git.reviewboard.kde.org/r/102696/#comment6018

proxyType



kioslave/http/http.cpp
http://git.reviewboard.kde.org/r/102696/#comment6017

AFAIU the proxy list can't contain a DIRECT entry somewhere; if it contains 
DIRECT it contains only DIRECT.
Does it even make sense to put the DIRECT case into the loop? It looks like 
a special case to me that could and should be handled outside the loop.



kioslave/http/http.cpp
http://git.reviewboard.kde.org/r/102696/#comment6019

Not sure if that would be correct, but I'd prefer something like:
if (isDirectConnect) {
Q_ASSERT(proxyType == QNetworProxy::NoProxy);
(...)

because you have two variables here that should make sense together, and 
it's better to check that they do instead of risking confusion later when 
something goes wrong.



kioslave/http/http.cpp
http://git.reviewboard.kde.org/r/102696/#comment6020

Why is this here and not in a more general place? The path between the 
place where the variable is set and the place where it is used is unclear to 
me. If there is too much in between, it is hard to guarantee that the variable 
has the correct value where it is used.


- Andreas Hartmetz


On Sept. 25, 2011, 2:28 a.m., Dawit Alemayehu wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/102696/
 ---
 
 (Updated Sept. 25, 2011, 2:28 a.m.)
 
 
 Review request for kdelibs.
 
 
 Description
 ---
 
 This 5th patch in a serious of patches meant to improve proxy support in KDE 
 deals with support at the kioslave level. More specifically the http ioslave. 
 The patch is necessary to provide proper support for PAC script based proxy 
 configuration. Namely allowing aleternate proxy servers to be specified and 
 used as necessary. Here are the change this patch makes:
 
  * Add a new function in TCPSlaveBase to connect to the a remote server 
 without 
 automatically sending error notitification to the client. [NEW API]
 
  * Move the proxy related code from 'resetSessionSettings' to 
 'httpOpenConnection'.
Proxy information will now be only set from 'setHost' and reset from 
'reparseConfiguration' as it should have been from the beginning. No 
 need to 
reparse proxy related information on every request.
 
  * Added a new variable, proxyUrls, to HTTPRequest to store the multiple 
 proxy URLs
 obtained from the ProxyUrls meta-data.
 
  * Modified 'httpShouldCloseConnection' to account for multiple proxy 
 addresses.
 
  * Modified 'sendQuery' to connect to the remote server before formatting 
 the HTTP
headers so that the headers can properly reflect the correct proxy 
 settings.
 
 
 Diffs
 -
 
   kio/kio/tcpslavebase.h 3f87ea8 
   kio/kio/tcpslavebase.cpp ec70559 
   kioslave/http/http.h d8c47c7 
   kioslave/http/http.cpp 6d41a13 
 
 Diff: http://git.reviewboard.kde.org/r/102696/diff/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Dawit Alemayehu
 

Re: Qt5 - KDE5?

[Andreas Hartmetz]

On Monday 09 May 2011 18:03:18 Olivier Goffart wrote:
 Hi,
 
 With Qt5 around the corner[1], I think it is time to start thinking about
 KDE5
 
 
 Raw summary:
  - Qt5 is planed to be released in about one year from now if everything
 goes well.
  - It should be mostly source compatible with Qt4, and is just an
 opportunity to break binary compatibility.
  - QWidget just stay for compatibility. All focus is put on QML. Do not
 expect new development on QWidgets from Nokia.
  - The OpenGouvernance should finaly come into light, meaning we (as
 KDE), can contribute easier to Qt.
 
 
 I guess it make sens, as Qt breaks binary compatibility, to do the same in
 kdelibs.
 Does that mean KDE 5? or KDE SC 5? Not necessarily.
 We can break binary compatibility, and change the .so version of our
 library without changing the major version of KDE itself.
 But I think it would anyway still be a smart move to do it.
 
 And I think this is a perfect opportunity to get some KDE class in Qt as we
 planed. [2]
 
 Some item we might want to think about:
 
  - Do we want KDE 5 to be a big change, or just a small increment?
 
  - Do we want to focus on QML, or stay with QWidget?
 
I think this one is both obvious and difficult: Everything else being equal QML 
because it is, for all we know now, more future compatible.

There are many open questions:
- Can we migrate QWidget-based code in some way?
- Which QWidget-based code is considered important? I'm thinking of
  KXmlGuiWindow and such things here.
- Will QML do what we need? Layouts, custom painting and event handling, one
  language for everything (we probably won't get that one).
- Can QML work on older hardware? (see Maksim's message)
- Can QML look and act native on the desktop? I don't care if it looks
  different, but it should be suitable for the given screen dimensions and
  input devices.
- Can we realistically pull off a minimal migration in time for the planned
   release date?
- What will the users think? - We should not lose too many user-visible
  features.

I think it's not looking good for mostly switching to QML and releasing KDE5 
shortly after Qt5. Even when QML's own issues have been fixed there will be a 
huge amount of work for KDE, part of which can only happen once QML is there.
Maybe we should simply stay at KDE4/Qt4 for a while and release KDE5 for Qt5 
when it's done, but no later than in ~2 years if possible.
For developers releasing KDE5 with QT5 and later KDE6 with Qt5 and more QML 
seems tempting, but users won't like it.


  - Shall we try drive Qt5 based on KDE5's need?
 
  - Do we have more visions for what KDE5 should or should not be?
 
 I guess there is as many opinions as people involved :-)
 Many things to think about, and that can be discussed further, and decided
 in Platform11 [3] (I will be there)
 
 But in my opinion, if there is something we should learn from the KDE4
 transition, it is not to be too ambitious.

Re: Review Request: Fix for a couple of KIO put-slave-on-hold bugs

[Andreas Hartmetz]

On Thursday 28 April 2011 16:05:51 Dawit Alemayehu wrote:
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/101244/
 ---
 
 (Updated April 28, 2011, 2:05 p.m.)
 
 
 Review request for kdelibs.
 
 
 Changes
 ---
 
 Removed the m_ignoreOnHoldListChanged flag per discussion with David.
 
 
 Summary
 ---
 
 The attached patch address a couple of bugs in the KIO put-slave-on-hold
 functionality:
 
 Problem:
 ==
 #1. If a user clicks a link on a page, e.g.
 ftp://ftp.kde.org/pub/kde/README_UPLOAD, then KRun will first issue a get
 (CMD_GET) to determine its content-type and put the ioslave on hold so
 that the application associated with the returned content-type can handle
 it. In case of our example that would be kate/kwrite. Unfortunately, the
 ioslave put on hold will not be reused because almost all applications
 will use KIO::file_copy (CMD_COPY) to make a local copy before opening it.
 Even then for ioslaves that do not optimize their copy command, the call
 to KIO::file_copy will properly reuse the ioslave on hold so long as it do
 not have an optimized copying method like the ftp ioslave.
 
 #2. If a user types a web address, e.g. www.kde.org, into KRunner to open
 it in Konqueror and repeats the same process with another address, then
 whether the ioslave put on hold the second time around will get reused or
 not depends how the application works. If the application allows multiple
 documents or tabs and opens the second url as in the already running
 instance, then the ioslave on hold will NOT be reused the second time
 around.
 
 Solution:
 ==
 #1. Simply force the KIO::file_copy call not to do the optimized copying if
 there is a slave on hold for the request. This will force it to use two
 separate jobs, KIO::get (CMD_GET) and KIO::put (CMD_PUT) ensuring the
 reuse of the ioslave that was put on hold.
 
 #2. Whenever a call to KIO::Scheduler::publishSlaveOnHold is made, send a
 dbus message to update all the other scheduler so that they can look for
 an ioslave-on-hold to service the next request.
 
 
 Diffs (updated)
 -
 
   kio/kio/job.cpp e34f562
   kio/kio/scheduler.h 9be9db0
   kio/kio/scheduler.cpp 4cb33d1
 
 Diff: http://git.reviewboard.kde.org/r/101244/diff
 
 
 Testing
 ---
 
 
 Thanks,
 
 Dawit

The previous slave-on-hold patches trigger a -probably- previously existing 
job accounting bug in the scheduler that was hidden before due to the mostly 
non-working state of the slave-on-hold mechanism. AFAICS this bug was present 
before my scheduler rewrite where I tried not to change anything I didn't 
completely understand.
After enabling SCHEDULER_DEBUG I get the following debug output from 
konqueror:

konqueror(5203)/kio (Scheduler) KIO::ProtoQueue::startAJob: 
m_runningJobsCount: 3

while some jobs stall and websites don't load. I can trigger this by running 
e.g. konqueror http://paste.kde.org/39121/ (unrelated paste).
The bug is probably somewhere in KIO::SchedulerPrivate::putSlaveOnHold(),
publishSlaveOnHold(), or heldSlaveForJob(). The job's slave is never marked as 
idle or not idle in any of these methods and it probably should be, somewhere.

Please take care of this before you continue. I know it's not really your 
bug, but it's probably not really mine either.
Contact me if you'd prefer me to have a look. I am trying not to spend much 
time on coding right now so I can spend more time on university stuff.

Cheers,
Andreas

Re: 4.6 branches created in git again

[Andreas Hartmetz]

On Sunday 20 March 2011 15:17:11 Ian Monroe wrote:
 On Sun, Mar 20, 2011 at 09:09, Andreas Hartmetz ahartm...@gmail.com wrote:
  On Sunday 20 March 2011 14:38:40 Albert Astals Cid wrote:
  Can you please be careful and do not create incorrectly 4.6 branches in
  places where the branch is called KDE/4.6 (e.g kdelibs and kde-runtime)
  
  Ian can you kill them?
  
  Albert
  
  Maybe we can go through with renaming the branches to n.m (without KDE/)
  prefix?
  Some repositories have the prefix, some don't, I think it doesn't make
  sense to have the prefix, and it was decided (by tossing a coin...) that
  the prefix should be removed.
 
 This would just cause more chaos until we block the creation of
 whatever the non-favored naming scheme is.
 
I'd suggest doing exactly that no matter if the branch names are changed or 
not.

Re: 4.6 branches created in git again

[Andreas Hartmetz]

On Sunday 20 March 2011 15:59:55 Parker Coates wrote:
 On Sun, Mar 20, 2011 at 10:09, Andreas Hartmetz wrote:
  On Sunday 20 March 2011 14:38:40 Albert Astals Cid wrote:
  Can you please be careful and do not create incorrectly 4.6 branches in
  places where the branch is called KDE/4.6 (e.g kdelibs and kde-runtime)
  
  Ian can you kill them?
  
  Maybe we can go through with renaming the branches to n.m (without KDE/)
  prefix?
 
 I think keeping the prefix is the superior option. For kdelibs,
 kderuntime, kdeworkspace, etc. it really doesn't make any difference,
 but for something like, say, Konsole the difference is important. If
 one sees a branch named 4.6 in the Konsole repository, one is likely
 to assume that it represents Konsole 4.6, when in fact it represents
 Konsole 2.6 which is part of KDE SC 4.6. Keeping the KDE/ prefix
 helps clarify this.
 
Right, I didn't think of that...
AFAIK we have some modules where the branch is called 4.6. They aren't that 
many, and can we fix them then?

Re: Review Request: Fix handling of FTP urls whose path name ends with a type code

[Andreas Hartmetz]

On Friday 18 March 2011 20:58:52 David Faure wrote:
 On Wednesday 16 March 2011, Thiago Macieira wrote:
  Suggestion:
  path.left(path.length() - sizeof ;type=);
 
 Unreadable.
 
  sizeof(;type=) == strlen(;type=X)
 
 A hidden off-by-one, how nice to have in our code ;)

The best solution is... strlen(), actually. GCC [-O2] optimizes out strlen() 
with a constant argument and replaces it with the result. If you copy the 
string, say one for strcpy() and once for strlen(), that's fine because the 
instances will either be merged or the instance passed to strlen() disappears 
anyway due to strlen() being replaced with its result.

Re: Review Request: Fix for connecting to FTP sites through HTTP proxy

[Andreas Hartmetz]

On Thursday 10 March 2011 08:14:04 Dawit A wrote:
 On Wed, Mar 9, 2011 at 6:32 PM, Andreas Hartmetz ahartm...@gmail.com 
wrote:
  On Wednesday 09 March 2011 23:43:23 Dawit A wrote:
 [snipped]
 
  Yes, it is, but not for those reasons. That entry was not put there
  simply because we had an HTTP proxy implementation, but because FTP
  requests can be routed through an HTTP proxy server just like HTTP and
  HTTPS requests. IOW, what do you do when the user specifies a HTTP
  proxy server as the FTP proxy. It is legal and valid to do that. What
  is happening in KDE now is that functionality is not supported for
  some unknown reason.
  
  Do you mean CONNECT? CONNECT is implemented in TcpSlaveBase via Qt
  sockets.
 
 Nope. I am not saying FTP over HTTP is like HTTPS. I was only saying
 that just like you can obviously send HTTP requests through HTTP
 proxies, you can do the same with FTP requests. Yes, you can send ftp
 requests through an HTTP Proxy server. The request line header will
 look some like: GET ftp://ftp.kde.org/ HTTP/1.1. The proxy server will
 then speak to the FTP server on the client behalf and return the
 response as HTML.
 
  CONNECT is entirely unlike real HTTP proxying (see below), it's very much
  like SOCKS - it's application-level port forwarding and IIRC not even
  standardized in a real Internet standard.
  The HTTP ioslave only handles actual HTTP proxying, where you talk HTTP
  to the proxy server and the proxy returns the data as if it was the
  originating server.
 
 First I am aware of how the proxy stuff works because largely I am
 responsible for its creation. ;) Second and more seriously, SOCKS v5
 is an Internet standard defined through RFC 1928. Though I have not
 read it, I am sure it
 
Yeah, SOCKS is standardized, but CONNECT isn't AFAIK.
I'll repeat here how it works, for the three other persons reading this :)
CONNECT is a pseudo-HTTP request that you send to an HTTP proxy to make it 
forward a port, SOCKS-like.
After the CONNECT command the proxy just forwards data to (and from) the 
remote server until the connection is closed, it's not really HTTP anymore.

  I don't see how the HTTP ioslave could help the FTP ioslave somehow
  unless there is some scheme in which the client talks HTTP to the proxy,
  which talks FTP to an FTP server, while the client still says it's FTP
  to the user / sets up the whole thing as described if the user requests
  FTP and an HTTP proxy is set.
 
 See  my explanation above. HTTP ioslave does not help the FTP ioslave.
 The ftp request is sent to and handled by the HTTP ioslave. The FTP
 ioslave does not come into the equation for FTP over HTTP proxy cases
 at all. See the documentation on FTP support in your favorite open
 source http proxy server, e.g. Squid.
 
Hm? That sounds exactly like the scenario I described. Note that I didn't 
mention ioslaves, just the protocols. Client talks HTTP to proxy, proxy talks 
FTP to FTP server.

  If such a scheme exists a one-off hack somewhere would be appropriate.
 
 That is exactly the wrong way to go about it for several reasons:
 
 #.1 Assumption is a mother of all f??? ups, especially when it comes
 to core libraries like KIO. There is no way you can guarantee that FTP
 over HTTP proxy is the only thing that would require such
 functionality to make a one off exception. That is why it was
 implemented the way it is in the first place. If another protocol
 requires a similar change which would be easier to do ? Change code to
 add yet another one-off hack or simply add a ProxiedBy= entry into
 the ioslave's protocol file ?
 
Well okay, it makes no sense to change something that already works / only 
needs some fixing. However I don't know of another such constellation in which 
an application protocol is transported over another application protocol.

 #2. Why change something that does not break or mess up anything in
 the first place ? You do understand that removing that parameter from
 the protocol file only takes away the optimization that was added to
 speed up the discovery of whether a request with one protocol (ftp)
 has the potential to be handled by an ioslave of different protocol
 (http). Its removal does not change the fact that the scheduler will
 still send the ftp request to the http ioslave. That decision is made
 by KProtocolManager::slaveProtocol. The property entry in the protocol
 file was meant to actually avoid calling latter slower function unless
 it is necessary to do so, alas the changes in
 KParts::BrowserRun::scanFile.
 
 #3. If a user only has access to the Internet through a HTTP proxy,
 how would you suggest they access and download stuff from an ftp site
 ? I am sure you are not advocating that we implement HTTP proxy
 handling in the FTP ioslave.
 
I was thinking of using CONNECT to talk real FTP. That doesn't seem to be the 
canonical way to do it, though...

  Since there is no input for specifying SOCKS proxy in the proxy
  configuration dialog, then I

Re: Future of KSysguard - removing remote monitoring

[Andreas Hartmetz]

On Wednesday 09 March 2011 15:00:27 Aaron J. Seigo wrote:
 On Tuesday, March 8, 2011, John Tapsell wrote:
So unless anyone can talk me out of it now, I am going to remove the
  
  ability to monitor remote hosts entirely
 
 imho:
 
 instead of eviscerating the application of the primary feature that makes
 it useful to a significant group of users of that application (sys
 admins), i'd suggest starting a new application (even if it shares a lot
 of the same code) if you feel you must go this route.
 
 remote monitoring is _the_ reason for using ksysguard for many of its
 users. without that feature, it becomes completely and entirely useless.
 
 it may also be possible that a clever refactoring wuld accomplish the
 desired simplification as well, though i assume you've looked at that
 possibility already.

I've looked at the code and it looks very clean and nice already.
Maybe those prospective contributors simply weren't up to par (yet)?

Re: Review Request: Fix for connecting to FTP sites through HTTP proxy

[Andreas Hartmetz]

On Wednesday 09 March 2011 20:06:40 Dawit A wrote:
 On Wed, Mar 9, 2011 at 12:46 PM, David Faure fa...@kde.org wrote:
  On Wednesday 09 March 2011, Dawit Alemayehu wrote:
  proxiedBy always returns true since ftp.protocol no longer contains a
  ProxiedBy= entry.
  
  Yes it does, of course. But the point of the additional if() is to not go
  into that code block for other procotols, like HTTP, FISH, and so on.
 
 I meant to say always returns false. KProtocolInfo::proxiedBy(...)
 always returns an empty string for FTP. I just looked at the log for
 ftp.protocol file and it seems that Andreas had mistakenly removed the
 proxiedBy= statement from the ftp.protocol file. See
 
 http://quickgit.kde.org/?p=kdelibs.gita=commith=7b61621c023db80af888a12be
 7fcb9fd6c9a6fb8
 
 As such the changes the fix I committed current does not fix the
 problem. FTP over HTTP proxy still does not work. Anyhow, if the
 incorrect commit mentioned above gets reverted, the problem away. Any
 objections with me doing that ?
 
I was thinking proxiedBy= was there to somehow involve the HTTP ioslave in 
proxied FTP transactions because the HTTP (and SOCKS IIRC) proxy 
implementation was there.
What I gather from this thread is that some HTTP proxies present FTP in HTML 
format... I think you have to be careful not to break FTP over SOCKS proxy if 
you make any changes here.
proxiedBy should probably be per proxy protocol now.

Re: Review Request: Fix for connecting to FTP sites through HTTP proxy

[Andreas Hartmetz]

On Wednesday 09 March 2011 23:43:23 Dawit A wrote:
 On Wed, Mar 9, 2011 at 2:53 PM, Andreas Hartmetz ahartm...@gmail.com 
wrote:
  On Wednesday 09 March 2011 20:06:40 Dawit A wrote:
  On Wed, Mar 9, 2011 at 12:46 PM, David Faure fa...@kde.org wrote:
   On Wednesday 09 March 2011, Dawit Alemayehu wrote:
   proxiedBy always returns true since ftp.protocol no longer contains a
   ProxiedBy= entry.
   
   Yes it does, of course. But the point of the additional if() is to not
   go into that code block for other procotols, like HTTP, FISH, and so
   on.
  
  I meant to say always returns false. KProtocolInfo::proxiedBy(...)
  always returns an empty string for FTP. I just looked at the log for
  ftp.protocol file and it seems that Andreas had mistakenly removed the
  proxiedBy= statement from the ftp.protocol file. See
  
  http://quickgit.kde.org/?p=kdelibs.gita=commith=7b61621c023db80af888a1
  2be 7fcb9fd6c9a6fb8
  
  As such the changes the fix I committed current does not fix the
  problem. FTP over HTTP proxy still does not work. Anyhow, if the
  incorrect commit mentioned above gets reverted, the problem away. Any
  objections with me doing that ?
  
  I was thinking proxiedBy= was there to somehow involve the HTTP ioslave
  in proxied FTP transactions because the HTTP (and SOCKS IIRC) proxy
  implementation was there.
 
 Yes, it is, but not for those reasons. That entry was not put there
 simply because we had an HTTP proxy implementation, but because FTP
 requests can be routed through an HTTP proxy server just like HTTP and
 HTTPS requests. IOW, what do you do when the user specifies a HTTP
 proxy server as the FTP proxy. It is legal and valid to do that. What
 is happening in KDE now is that functionality is not supported for
 some unknown reason.
 
Do you mean CONNECT? CONNECT is implemented in TcpSlaveBase via Qt sockets.
CONNECT is entirely unlike real HTTP proxying (see below), it's very much like 
SOCKS - it's application-level port forwarding and IIRC not even standardized 
in a real Internet standard.
The HTTP ioslave only handles actual HTTP proxying, where you talk HTTP to the 
proxy server and the proxy returns the data as if it was the originating 
server.

I don't see how the HTTP ioslave could help the FTP ioslave somehow unless 
there is some scheme in which the client talks HTTP to the proxy, which talks 
FTP to an FTP server, while the client still says it's FTP to the user / sets 
up the whole thing as described if the user requests FTP and an HTTP proxy is 
set.
If such a scheme exists a one-off hack somewhere would be appropriate.

  What I gather from this thread is that some HTTP proxies present FTP in
  HTML format... I think you have to be careful not to break FTP over
  SOCKS proxy if you make any changes here.
 
 Since there is no input for specifying SOCKS proxy in the proxy
 configuration dialog, then I assume that the user is supposed to enter
 a non standard url, socks://host:[port], for specifying SOCKS
 proxy, correct ? If so, then ensuring FTP over SOCKS is not treated as
 FTP over HTTP is a simple one line fix in
 KProtocolManager::slaveProtocol.
 
 BTW, as it stands right now the ftp protocol does not support FTP over
 SOCKS.
 
SOCKS proxy support should be transparent to ioslaves.
FTP has the unique problem that it needs two ports to work, but TCPSlaveBase 
only allows one network connection per ioslave.
I think this should be solved by using a KTcpSocket for the data connection, 
if necessary with manual tweaks to use the right proxy settings, in the FTP 
ioslave code.
Passive FTP only, active would require asking the proxy to open a port etc., 
way too ugly. Passive has become the usual way to do FTP.

  proxiedBy should probably be per proxy protocol now.
 
 Sorry, but that would only serve to complicate proxy handling much
 more than the big mess it already is.
 
It's not that messy: network-level proxies are handled by TcpSlaveBase, 
application protocol-level proxying (of which there is only HTTP) is 
implemented in protocol ioslaves.

After writing the above questions and answers I think proxiedBy should stay 
dead.

Re: Profiles for all KDE-applications

[Andreas Hartmetz]

On Saturday 26 February 2011 10:58:23 Jonathan Schmidt-Dominé wrote:
 Hi!
 
 In Mozilla-applications so called “profiles”, sets of user-configuration,
 are a quite common feature, Konqueror implements them, too. But wouldn't it
 be possibe to provide this feature for all KDE-applications by changing
 KApplication, KConfig and friends? It should be possible to pass some
 parameters at startup to use a specific profile, KApplication would
 recognize the paths and KConfig would adjust its directories and
 independendent sets of configuration would be used. Those configurations
 could be saved somewhere in .kde/extra-profiles or something like that. It
 should work for most applications, only a small minority does not keep its
 configuraion in the user's .desktop-files. Some simple GUI-dialogs and it
 would be at least as good as Mozilla's support. I think it would be useful
 for many applications, e.g. Kopete or Amarok. Any comments?
 
Tha's a really advanced feature, and it's already available to advanced users. 
You only have to set a different $KDEHOME before starting an application.

Re: git workflow draft

[Andreas Hartmetz]

On Thursday 17 February 2011 09:01:05 Johannes Sixt wrote:
 Am 2/16/2011 22:10, schrieb Stephen Kelly:
  As one of the people asked to describe my idea of the workflow (which
  should focus on rebasing, not merging) I put  write up here:
  
  http://community.kde.org/20110213_GitWorkflowAgenda/StevesIdea
  
  http://thread.gmane.org/gmane.comp.kde.scm-interest/2007
  
  The emphasis is on creating a 'nice' stream of commits which it is
  possible to review (if applicable) and which apply against the target
  branch. This way a reviewer (or archeologist looking for the source ofg
  a bug) doesn't have to analyse commits in the branch which are
  workarounds for bugs in master, find the merge from master and the
  removal of the workaround, analyse the master branch in the same
  timeframe to see what needed to be merged in etc etc.
 
 Now I get it: The reason why you are advocating a rebase based workflow is
 because the goal should be a series of self-contained, easy-to-review,
 tested(!) commits.
 
 That is, when someone (an individual or a group of collaborators) produces
 a new feature, the result should be a (not too long) series of clean
 commits. To achieve this, it is necessary to use git-rebase in order to
 fold, split, and rearrange the commits. This sort of rebasing should
 happen after a group of collaborators has produced a potentially messy
 history with merges and fixups of earlier commits, and before the feature
 nears publication to a wider audience. (You mentioned an example elsewhere
 in this thread.)
 
 So far, we are in the same boat, absolutely.
 
 HOWEVER:
  The topic branch is an addition to the master branch. It should be
  relevant against the current master and 'clean' and nice. Happy and
  good. No useless merges. There should be no recommendation in
  documentation to 'merge a topic branch into master'. The recommendation
  should be to rebase instead. This is my recommendation. There has been
  some discussion on the scm interest list, but I wouldn't call it
  concensus at this point.
 
 We disagree here: How a topic branch (that was prepared like above) ends
 up in the master branch is an *entirely* different story.
 
 When you develop a new feature, it is a very important choice on which
 version of the software you base it on. I am advocating to base a feature
 on a well-known, stable state. If you choose to base the feature on
 today's master, you are actually building a house on moving grounds. How
 can you be sure that your product will be stable?
 
 The corollary is that a new topic should be based on a project state that
 is no younger than necessary (to have all prerequisites that you need), so
 that you can be sure that it was already tested by a as many people as
 possible. Ideally, today, every feature targeted for 4.7 should be based
 on the 4.6.0 release tag, whenever possible.
 
 As a consequence, you must not rebase the finished topic onto master, but
 you must merge it.
 
As somebody who usually runs master, I see a bit of a problem here. Everybody 
who is working on a feature will not be testing master (aspects related and 
unrelated to the branch) while doing so. This may or may not turn out to be a 
problem in practice.

Maybe we'll find rules of thumb to choose the right base for topic branches. 
Like usually latest stable unless you rely on features of master or usually 
master unless changes in master would interfere with your work...
A checklist with obvious and not so obvious arguments for / against choosing a 
particular base could also help make decisions on a case-by-case basis.

Does that sound reasonable or is it just my lack of git experience making me 
suggest such things?

Re: Merge or Cherry-Pick?

[Andreas Hartmetz]

On Thursday 03 February 2011 08:57:00 Johannes Sixt wrote:
 Am 2/3/2011 6:10, schrieb Dawit A:
  What happens if I tested a bug fix and wanted it to push it upstream
  so that it can receive even wider testing, but it just so happens the
  time to tag the next bug fix release is right around the corner ? The
  original intent is to at least leave the bug fix in the trunk for
  several days to see if it causes any unforeseen regression. It is an
  impossible task for any developer, specially one working on the
  plumbing libraries, to test every possible combination or use case. If
  the push is done into a stable branch, then there is a real
  possibility that users are going to get exposed to unintended
  regressions.
 
 I'll assume you meant to say push is done into a stable branch *too
 early*... (otherwise, the last sentence in this paragraph is incompatible
 with the first part of the paragraph).
 
 You do it this way:
 
 1. You fork off a topic branch from stable, and place your fix there.
 
 2. Compile-test the fix. (Bonus points, if you can test the fix in action
 with the stable version.)
 
 3. Merge that topic into master. Compile and test.
 
 4. Publish the merge. Let it cook for a week in master.
 
 5. Looks OK? Good. Merge the topic into the stable branch as well. (The
 stable branch might have progressed in the past week, hence, you don't
 necessarily fast-forward, but get a real merge.)
 
 6. Merge stable into master and publish both.
 
 However, if you cannot test your fix with the stable version yourself, you
 should ask someone to do Step 5, additionally Step 5b test with stable
 version, and Step 6 for you.
 
This seems to be, except for the origin of the merge into stable which is 
irrelevant to the actual code in the repository, what Dawit and I want.
AFAIU simple bug fixes like null pointer checks go into stable first and then 
immediately into master with that approach, as suggested earlier in this 
thread.
As long as I can test fixes in master in some way I'm (for now) fine with 
trying 
any approach - it will probably work about equally well for everyone, so if it 
won't work for me it won't work for others and we can think again.

Re: Merge or Cherry-Pick?

[Andreas Hartmetz]

On Wednesday 02 February 2011 21:15:31 Aaron J. Seigo wrote:
 On Wednesday, February 2, 2011, Thiago Macieira wrote:
  I still think the current procedure is wrong. You're not testing the
  stable release, there's no guarantee that you're solving the problem at
  all, or worse, that you're not making it worse.
 
 and, imho, the stable branch is the more important thing to test: if it
 goes wrong in master due to a bad or unecessary merge from stable, you
 usually have months to notice and fix it. certain you or your teammates
 that also track master will notice it faster than if it sits in the stable
 branch where primary devel isn't happening anymore. with our monthly x.y.z
 releases, you have at most a few weeks with fewer people tracking the
 stable branch to catch a bad merge from master.
 
 so, again at least imho, the risk is higher when backporting compared to
 forward porting.
 
 and finally we have a tool that makes it reasonably painless to do it. :)

I have two reasons to test in master:
- I run master myself all the time.
- If a fix really is dangerous I don't want it to appear in a bugfix release 
by 
accident.

If nobody was running master who'd make sure its quality was even remotely 
decent? Somehow I don't buy that we should all be testing the latest stable 
branch while developing against master. Switching environments all the time is 
a major hassle and not as effective at finding and making people care about 
bugs in master.

Re: KDE library compile error (on the ARM CORTEX-A8)

[Andreas Hartmetz]

On Monday 17 January 2011 22:56:47 Thiago Macieira wrote:
 On Monday, 17 de January de 2011 22:04:46 АлексаМЎр КузеЌкП 
wrote:
  Hello!
  I have beagleboard-xm with Gentoo distributive on it. I compile and can
  run Qt 4.7.1 demo. Now I decide install kde from 4.6 branch svn. When I
  emerge kde-base/kdelibs-4.6. I recive error.
  http://bugs.gentoo.org/show_bug.cgi?id51934
 
 That's only a warning. Unfortunately, there's no error condition in your
 paste.

The standard KDE compiler flags contain -Wl,--fatal-warnings, i.e. abort on 
linker *warnings*.
I know because I'm using the gold linker, which is more picky, and I've had to 
remove the flag from kdelibs/cmake/modules/FindKDE4Internal.cmake occasionally 
to make some things build. The usefulness of this flag is debatable in that 
context...

Re: Suspending mailinglists due to lack of moderators.

[Andreas Hartmetz]

On Saturday 01 January 2011 22:25:23 Tom Albers wrote:
 Hi,
 
 These are the top mailinglists with the most amount of moderation requests.
 For most of them we have tried to contact the current moderators without
 much success last year. So now I suggest we get rid of these mailinglists.
 
 Note that if you object, you are volunteering for taking over moderation.
 
 312 decibel
 150 kde-imaging
 102 kjsembed
  80 kmobiletools
  46 kde-contests
  42 quanta
  42 kde-i18n-pt
  40 kdelibs-bugs

That one was my idea (IIRC ;) and I recently pondered its usefulness again - 
did I start a useless list? It's not meant to receive mails from humans, it's 
just an activity report of bugs.kde.org that you can search in your mail 
client.
The main argument I have for it is that I've fixed a few things (maybe 5x in 
the last 2 years) and took adminstrative actions in bugzilla for a few other 
bugs after seeing a bug appear in the kdelibs-bugs folder of my kmail. So I 
think it would be preferable to keep it and let /dev/null moderate it, or if 
it causes human or computer load we could use better somewhere else, close it.

  36 cervisia
  32 quanta-devel
  32 kde-enterprise
  29 klik-devel
  29 kde-java
  28 plasma-bugs
  28 dot-stories
 
 Best,
 
 Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe 

Re: [Kde-hardware-devel] KDE/kdelibs/solid/tests

[Andreas Hartmetz]

On Thu, Feb 25, 2010 at 12:35 AM, David Faure fa...@kde.org wrote:
 On Thursday 25 February 2010, Andreas Hartmetz wrote:
 SVN commit 1095733 by ahartmetz:

 Fix the build (with tests).
 I've been waiting for a situation to apply the #define private public hack
 for years :)
 CCMAIL: kde-hardware-devel@kde.org

 What about declaring the test class as friend, instead?
 That seems much cleaner.

It was *supposed* to be a throwaway hack ;)
I was unsure which part of Solid was wrong, so I just made the test compile
and waited for somebody else to fix it properly. I guess I should have made
that last part more explicit in the commit message.
I wasn't sure if the method was supposed to be private or not because a
commit message made it look like it was - OTOH, making a public
method private in a mature module does of course look wrong.
___
Kde-hardware-devel mailing list
Kde-hardware-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-hardware-devel