Re: Review Request 125725: Make KCrash optional for kservice

2015-10-22 Thread David Faure 

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


For the record, I would have approved this. This is really just extra 
rarely-needed safety, because nowadays the apps themselves rebuild sycoca 
directly, so the use of kcrash here is really just for the corner case of "all 
my apps crash because of ksycoca being corrupted" and then you try to run 
kbuildsycoca to repair that, it crashes - and the crash handler deletes the bad 
file. Corner case, because you could have use --noincremental instead, or just 
deleted the cache by hand.

- David Faure


On Oct. 20, 2015, 2 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125725/
> ---
> 
> (Updated Oct. 20, 2015, 2 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> kservice depends on KCrash only for kbuildsycoca.
> make this optional and link only against it, if around. Move check to 
> kbuildsyscoca file.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 4c0f269 
>   src/kbuildsycoca/CMakeLists.txt 19bdc84 
>   src/kbuildsycoca/kbuildsycoca_main.cpp 03619cc 
> 
> Diff: https://git.reviewboard.kde.org/r/125725/diff/
> 
> 
> Testing
> ---
> 
> Seems to compile fine without it.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

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

Re: Review Request 125725: Make KCrash optional for kservice

2015-10-20 Thread Aleix Pol Gonzalez 

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



src/kbuildsycoca/kbuildsycoca_main.cpp (line 121)


Maybe it would make sense to handle it using signal() directly here, rather 
than just ifdef'ing KCrash?


- Aleix Pol Gonzalez


On Oct. 20, 2015, 3:41 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125725/
> ---
> 
> (Updated Oct. 20, 2015, 3:41 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> kservice depends on KCrash only for kbuildsycoca.
> make this optional and link only against it, if around. Move check to 
> kbuildsyscoca file.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 4c0f269 
>   src/kbuildsycoca/CMakeLists.txt 19bdc84 
>   src/kbuildsycoca/kbuildsycoca_main.cpp 03619cc 
> 
> Diff: https://git.reviewboard.kde.org/r/125725/diff/
> 
> 
> Testing
> ---
> 
> Seems to compile fine without it.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

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

Re: Review Request 125725: Make KCrash optional for kservice

2015-10-20 Thread Christoph Cullmann 


> On Oct. 20, 2015, 1:52 p.m., Aleix Pol Gonzalez wrote:
> > src/kbuildsycoca/kbuildsycoca_main.cpp, line 122
> > 
> >
> > Maybe it would make sense to handle it using signal() directly here, 
> > rather than just ifdef'ing KCrash?

Perhaps, but then I would have to reimplement more or less what 
setEmergencySaveFunction does, or? I am not sure if that makes sense, if you 
want crash recovery, you can have KCrash.


- Christoph


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


On Oct. 20, 2015, 1:41 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125725/
> ---
> 
> (Updated Oct. 20, 2015, 1:41 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> kservice depends on KCrash only for kbuildsycoca.
> make this optional and link only against it, if around. Move check to 
> kbuildsyscoca file.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 4c0f269 
>   src/kbuildsycoca/CMakeLists.txt 19bdc84 
>   src/kbuildsycoca/kbuildsycoca_main.cpp 03619cc 
> 
> Diff: https://git.reviewboard.kde.org/r/125725/diff/
> 
> 
> Testing
> ---
> 
> Seems to compile fine without it.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

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

Re: Review Request 125725: Make KCrash optional for kservice

2015-10-20 Thread Christoph Cullmann 

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

(Updated Oct. 20, 2015, 2 p.m.)


Status
--

This change has been discarded.


Review request for KDE Frameworks and David Faure.


Repository: kservice


Description
---

kservice depends on KCrash only for kbuildsycoca.
make this optional and link only against it, if around. Move check to 
kbuildsyscoca file.


Diffs
-

  CMakeLists.txt 4c0f269 
  src/kbuildsycoca/CMakeLists.txt 19bdc84 
  src/kbuildsycoca/kbuildsycoca_main.cpp 03619cc 

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


Testing
---

Seems to compile fine without it.


Thanks,

Christoph Cullmann

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

Re: Review Request 125725: Make KCrash optional for kservice

2015-10-20 Thread Aleix Pol Gonzalez 


> On Oct. 20, 2015, 3:52 p.m., Aleix Pol Gonzalez wrote:
> > src/kbuildsycoca/kbuildsycoca_main.cpp, line 122
> > 
> >
> > Maybe it would make sense to handle it using signal() directly here, 
> > rather than just ifdef'ing KCrash?
> 
> Christoph Cullmann wrote:
> Perhaps, but then I would have to reimplement more or less what 
> setEmergencySaveFunction does, or? I am not sure if that makes sense, if you 
> want crash recovery, you can have KCrash.
> 
> Christoph Cullmann wrote:
> On the other side: Ok, that was anyway not that useful, given 
> kglobalaccel + kinit use KCrash, too.
> Perhaps I should better take a look at KCrash to make it less verbose, 
> e.g. not warn about missing drkonqui which won't be there on mac or win in 
> the normal case.

Sure.

Or maybe some of the code can become part of KCoreAddons?


- Aleix


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


On Oct. 20, 2015, 4 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125725/
> ---
> 
> (Updated Oct. 20, 2015, 4 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> kservice depends on KCrash only for kbuildsycoca.
> make this optional and link only against it, if around. Move check to 
> kbuildsyscoca file.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 4c0f269 
>   src/kbuildsycoca/CMakeLists.txt 19bdc84 
>   src/kbuildsycoca/kbuildsycoca_main.cpp 03619cc 
> 
> Diff: https://git.reviewboard.kde.org/r/125725/diff/
> 
> 
> Testing
> ---
> 
> Seems to compile fine without it.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

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

Re: Review Request 125725: Make KCrash optional for kservice

2015-10-20 Thread David Faure 


> On Oct. 20, 2015, 1:52 p.m., Aleix Pol Gonzalez wrote:
> > src/kbuildsycoca/kbuildsycoca_main.cpp, line 122
> > 
> >
> > Maybe it would make sense to handle it using signal() directly here, 
> > rather than just ifdef'ing KCrash?
> 
> Christoph Cullmann wrote:
> Perhaps, but then I would have to reimplement more or less what 
> setEmergencySaveFunction does, or? I am not sure if that makes sense, if you 
> want crash recovery, you can have KCrash.
> 
> Christoph Cullmann wrote:
> On the other side: Ok, that was anyway not that useful, given 
> kglobalaccel + kinit use KCrash, too.
> Perhaps I should better take a look at KCrash to make it less verbose, 
> e.g. not warn about missing drkonqui which won't be there on mac or win in 
> the normal case.
> 
> Aleix Pol Gonzalez wrote:
> Sure.
> 
> Or maybe some of the code can become part of KCoreAddons?

I've been thinking for quite some time that a very basic "run this function on 
crash" and "restart on crash" functionality would be good to have in QtCore. 
Could be KCoreAddons too indeed, otherwise.
No bug reporting and all that, just the basics for daemons and command-line 
tools.

The trick however is how would KCrash work on top of that (would it be "use one 
or the other, they are fully independent and exclusive", or would it extend 
this mechanism - well, by setting that crash-handler function, maybe).


- David


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


On Oct. 20, 2015, 2 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125725/
> ---
> 
> (Updated Oct. 20, 2015, 2 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> kservice depends on KCrash only for kbuildsycoca.
> make this optional and link only against it, if around. Move check to 
> kbuildsyscoca file.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 4c0f269 
>   src/kbuildsycoca/CMakeLists.txt 19bdc84 
>   src/kbuildsycoca/kbuildsycoca_main.cpp 03619cc 
> 
> Diff: https://git.reviewboard.kde.org/r/125725/diff/
> 
> 
> Testing
> ---
> 
> Seems to compile fine without it.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

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

Re: Review Request 125725: Make KCrash optional for kservice

2015-10-20 Thread Christoph Cullmann 


> On Oct. 20, 2015, 1:52 p.m., Aleix Pol Gonzalez wrote:
> > src/kbuildsycoca/kbuildsycoca_main.cpp, line 122
> > 
> >
> > Maybe it would make sense to handle it using signal() directly here, 
> > rather than just ifdef'ing KCrash?
> 
> Christoph Cullmann wrote:
> Perhaps, but then I would have to reimplement more or less what 
> setEmergencySaveFunction does, or? I am not sure if that makes sense, if you 
> want crash recovery, you can have KCrash.
> 
> Christoph Cullmann wrote:
> On the other side: Ok, that was anyway not that useful, given 
> kglobalaccel + kinit use KCrash, too.
> Perhaps I should better take a look at KCrash to make it less verbose, 
> e.g. not warn about missing drkonqui which won't be there on mac or win in 
> the normal case.
> 
> Aleix Pol Gonzalez wrote:
> Sure.
> 
> Or maybe some of the code can become part of KCoreAddons?
> 
> David Faure wrote:
> I've been thinking for quite some time that a very basic "run this 
> function on crash" and "restart on crash" functionality would be good to have 
> in QtCore. Could be KCoreAddons too indeed, otherwise.
> No bug reporting and all that, just the basics for daemons and 
> command-line tools.
> 
> The trick however is how would KCrash work on top of that (would it be 
> "use one or the other, they are fully independent and exclusive", or would it 
> extend this mechanism - well, by setting that crash-handler function, maybe).
> 
> Martin Gräßlin wrote:
> > given kglobalaccel + kinit use KCrash, too.
> 
> isn't kglobalaccel only in the runtime part? If yes we can certainly make 
> the complete runtime optional (which would turn it to tier1).

Yes, that is true, its only the "daemon".
Btw., I see that there are backends for mac + windows for that daemon, but I am 
not that sure it is that useful, as you have no dbus around to talk with it, in 
the normal case.


- Christoph


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


On Oct. 20, 2015, 2 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125725/
> ---
> 
> (Updated Oct. 20, 2015, 2 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> kservice depends on KCrash only for kbuildsycoca.
> make this optional and link only against it, if around. Move check to 
> kbuildsyscoca file.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 4c0f269 
>   src/kbuildsycoca/CMakeLists.txt 19bdc84 
>   src/kbuildsycoca/kbuildsycoca_main.cpp 03619cc 
> 
> Diff: https://git.reviewboard.kde.org/r/125725/diff/
> 
> 
> Testing
> ---
> 
> Seems to compile fine without it.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

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

Re: Review Request 125725: Make KCrash optional for kservice

2015-10-20 Thread Martin Gräßlin 


> On Oct. 20, 2015, 3:52 p.m., Aleix Pol Gonzalez wrote:
> > src/kbuildsycoca/kbuildsycoca_main.cpp, line 122
> > 
> >
> > Maybe it would make sense to handle it using signal() directly here, 
> > rather than just ifdef'ing KCrash?
> 
> Christoph Cullmann wrote:
> Perhaps, but then I would have to reimplement more or less what 
> setEmergencySaveFunction does, or? I am not sure if that makes sense, if you 
> want crash recovery, you can have KCrash.
> 
> Christoph Cullmann wrote:
> On the other side: Ok, that was anyway not that useful, given 
> kglobalaccel + kinit use KCrash, too.
> Perhaps I should better take a look at KCrash to make it less verbose, 
> e.g. not warn about missing drkonqui which won't be there on mac or win in 
> the normal case.
> 
> Aleix Pol Gonzalez wrote:
> Sure.
> 
> Or maybe some of the code can become part of KCoreAddons?
> 
> David Faure wrote:
> I've been thinking for quite some time that a very basic "run this 
> function on crash" and "restart on crash" functionality would be good to have 
> in QtCore. Could be KCoreAddons too indeed, otherwise.
> No bug reporting and all that, just the basics for daemons and 
> command-line tools.
> 
> The trick however is how would KCrash work on top of that (would it be 
> "use one or the other, they are fully independent and exclusive", or would it 
> extend this mechanism - well, by setting that crash-handler function, maybe).

> given kglobalaccel + kinit use KCrash, too.

isn't kglobalaccel only in the runtime part? If yes we can certainly make the 
complete runtime optional (which would turn it to tier1).


- Martin


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


On Oct. 20, 2015, 4 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125725/
> ---
> 
> (Updated Oct. 20, 2015, 4 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> kservice depends on KCrash only for kbuildsycoca.
> make this optional and link only against it, if around. Move check to 
> kbuildsyscoca file.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 4c0f269 
>   src/kbuildsycoca/CMakeLists.txt 19bdc84 
>   src/kbuildsycoca/kbuildsycoca_main.cpp 03619cc 
> 
> Diff: https://git.reviewboard.kde.org/r/125725/diff/
> 
> 
> Testing
> ---
> 
> Seems to compile fine without it.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

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

Re: Review Request 125725: Make KCrash optional for kservice

2015-10-20 Thread Martin Gräßlin 


> On Oct. 20, 2015, 3:52 p.m., Aleix Pol Gonzalez wrote:
> > src/kbuildsycoca/kbuildsycoca_main.cpp, line 122
> > 
> >
> > Maybe it would make sense to handle it using signal() directly here, 
> > rather than just ifdef'ing KCrash?
> 
> Christoph Cullmann wrote:
> Perhaps, but then I would have to reimplement more or less what 
> setEmergencySaveFunction does, or? I am not sure if that makes sense, if you 
> want crash recovery, you can have KCrash.
> 
> Christoph Cullmann wrote:
> On the other side: Ok, that was anyway not that useful, given 
> kglobalaccel + kinit use KCrash, too.
> Perhaps I should better take a look at KCrash to make it less verbose, 
> e.g. not warn about missing drkonqui which won't be there on mac or win in 
> the normal case.
> 
> Aleix Pol Gonzalez wrote:
> Sure.
> 
> Or maybe some of the code can become part of KCoreAddons?
> 
> David Faure wrote:
> I've been thinking for quite some time that a very basic "run this 
> function on crash" and "restart on crash" functionality would be good to have 
> in QtCore. Could be KCoreAddons too indeed, otherwise.
> No bug reporting and all that, just the basics for daemons and 
> command-line tools.
> 
> The trick however is how would KCrash work on top of that (would it be 
> "use one or the other, they are fully independent and exclusive", or would it 
> extend this mechanism - well, by setting that crash-handler function, maybe).
> 
> Martin Gräßlin wrote:
> > given kglobalaccel + kinit use KCrash, too.
> 
> isn't kglobalaccel only in the runtime part? If yes we can certainly make 
> the complete runtime optional (which would turn it to tier1).
> 
> Christoph Cullmann wrote:
> Yes, that is true, its only the "daemon".
> Btw., I see that there are backends for mac + windows for that daemon, 
> but I am not that sure it is that useful, as you have no dbus around to talk 
> with it, in the normal case.

> but I am not that sure it is that useful, as you have no dbus around to talk 
> with it, in the normal case.

I think they don't compile anyway.


- Martin


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


On Oct. 20, 2015, 4 p.m., Christoph Cullmann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125725/
> ---
> 
> (Updated Oct. 20, 2015, 4 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kservice
> 
> 
> Description
> ---
> 
> kservice depends on KCrash only for kbuildsycoca.
> make this optional and link only against it, if around. Move check to 
> kbuildsyscoca file.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 4c0f269 
>   src/kbuildsycoca/CMakeLists.txt 19bdc84 
>   src/kbuildsycoca/kbuildsycoca_main.cpp 03619cc 
> 
> Diff: https://git.reviewboard.kde.org/r/125725/diff/
> 
> 
> Testing
> ---
> 
> Seems to compile fine without it.
> 
> 
> Thanks,
> 
> Christoph Cullmann
> 
>

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

Review Request 125725: Make KCrash optional for kservice

2015-10-20 Thread Christoph Cullmann 

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

Review request for KDE Frameworks and David Faure.


Summary (updated)
-

Make KCrash optional for kservice


Repository: kservice


Description (updated)
---

kservice depends on KCrash only for kbuildsycoca.
make this optional and link only against it, if around. Move check to 
kbuildsyscoca file.


Diffs (updated)
-

  CMakeLists.txt 4c0f269 
  src/kbuildsycoca/CMakeLists.txt 19bdc84 
  src/kbuildsycoca/kbuildsycoca_main.cpp 03619cc 

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


Testing (updated)
---

Seems to compile fine without it.


Thanks,

Christoph Cullmann

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