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

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 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

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:
> > 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: 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: 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: 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...