D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham added a comment.


  pino: I mean the very same m_iconButton is common to all other places entries 
like root,home etc, we are just creating it depending on whether it is trash or 
not

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, broulik, #dolphin, #frameworks
Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns


D14360: Remove custom icon selection for trash

2018-07-25 Thread Pino Toscano
pino added a comment.


  In D14360#298208 , @shubham wrote:
  
  > pino: I don't why m_iconButton isn't needed(because it is being used by 
other entries also ), am I right?
  
  
  I don't understandwhat you mean, can you please rephrase it?
  
  The point is: if the icon must not be edited, then even creating the 
`KiconButton` for it is not useful, because it's an unused widget. Even more, 
Creating it and making it hidden still uses the resources needed to create it, 
and load the icon for it. So... just do not create it, taking care of handling 
in the dialog the case when `m_iconButton` is null.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, broulik, #dolphin, #frameworks
Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns


D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham added a comment.


  pino: I don't why m_iconButton isn't needed(because it is being used by other 
entries also ), am I right?

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, broulik, #dolphin, #frameworks
Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns


D14360: Remove custom icon selection for trash

2018-07-25 Thread Pino Toscano
pino added a comment.


  TBH creating a widget without adding it to a layout does not make much sense 
to me -- in the past I saw those situations as "floating" widgets usually 
anchored at the top-left corner of the parent widget.
  The best way here is to just not create `m_iconButton` at all, like I 
suggested in D14378: Remove custom icon selection for trash 
 too.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, broulik, #dolphin, #frameworks
Cc: pino, kde-frameworks-devel, michaelh, ngraham, bruns


D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham updated this revision to Diff 38468.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14360?vs=38452=38468

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

AFFECTED FILES
  src/filewidgets/kfileplaceeditdialog.cpp

To: shubham, ngraham, broulik, #dolphin, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


KDE CI: Frameworks purpose kf5-qt5 SUSEQt5.10 - Build # 91 - Fixed!

2018-07-25 Thread CI System
BUILD SUCCESS
 Build URL
https://build.kde.org/job/Frameworks%20purpose%20kf5-qt5%20SUSEQt5.10/91/
 Project:
Frameworks purpose kf5-qt5 SUSEQt5.10
 Date of build:
Thu, 26 Jul 2018 03:45:50 +
 Build duration:
9 min 10 sec and counting
   JUnit Tests
  Name: (root) Failed: 0 test(s), Passed: 3 test(s), Skipped: 0 test(s), Total: 3 test(s)
   Cobertura Report
  
   Project Coverage Summary
  
   Name
  PackagesFilesClassesLinesConditionalsCobertura Coverage Report22%
(5/23)26%
(14/53)26%
(14/53)19%
(413/2151)18%
(194/1096)Coverage Breakdown by Package
Name
   FilesClassesLinesConditionalsautotests100%
(2/2)100%
(2/2)97%
(120/124)58%
(56/96)src100%
(8/8)100%
(8/8)68%
(212/312)50%
(104/210)src.externalprocess0%
(0/2)0%
(0/2)0%
(0/136)0%
(0/88)src.fileitemactionplugin0%
(0/1)0%
(0/1)0%
(0/17)0%
(0/12)src.plugins.bluetooth0%
(0/1)0%
(0/1)0%
(0/34)0%
(0/14)src.plugins.email0%
(0/1)0%
(0/1)0%
(0/55)0%
(0/20)src.plugins.imgur0%
(0/2)0%
(0/2)0%
(0/186)0%
(0/69)src.plugins.kdeconnect0%
(0/1)0%
(0/1)0%
(0/32)0%
(0/12)src.plugins.ktp-sendfile0%
(0/1)0%
(0/1)0%
(0/28)0%
(0/12)src.plugins.nextcloud0%
(0/3)0%
(0/3)0%
(0/79)0%
(0/34)src.plugins.pastebin0%
(0/1)0%
(0/1)0%
(0/54)0%
(0/29)src.plugins.phabricator0%
(0/3)0%
(0/3)0%
(0/222)0%
(0/82)src.plugins.phabricator.quick0%
(0/5)0%
(0/5)0%
(0/99)0%
(0/62)src.plugins.phabricator.tests0%
(0/1)0%
(0/1)0%
(0/60)0%
(0/28)src.plugins.reviewboard0%
(0/3)0%
(0/3)0%
(0/233)0%
(0/76)src.plugins.reviewboard.quick0%
(0/7)0%
(0/7)0%
(0/153)0%
(0/92)src.plugins.saveas100%
(1/1)100%
(1/1)57%
(29/51)64%
(28/44)src.plugins.telegram0%
(0/1)0%
(0/1)0%
(0/45)0%

D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread David Faure
dfaure added a comment.


  In D14302#298099 , @thiago wrote:
  
  > In D14302#297515 , @dfaure wrote:
  >
  > > The idea of the old code was: if I can't get the lock, then someone else 
is already in the process of starting kdeinit, so I'll just wait for that to 
happen, by locking again, i.e. blocking on purpose, and then checking that the 
DBus name is up, i.e. the other process did manage to do it successfully.
  >
  >
  > It's Kind of silly to tryLock() then lock(). Why not just lock()? Was it 
the optimisation to avoid the D-Bus call? If so, a comment would be warranted.
  
  
  Yes: it would be kind of silly to check the dbus servicename, get the lock 
(because nobody else is doing this), and then check the dbus service name 
again, what chance did that have to suddenly pass?
  
  I think the new set of comments explains quite clearly the logic, now.
  
  > Anyway, one theory for why the lock file is still present: it's stale from 
a previous boot, but the PID is taken by some process in the current boot. This 
is fixed in Qt 5.11 by saving the boot ID (commit 
772863355a0cf57a49e93608790dfd17c8fd82da). So question to @jtamate , what Qt 
version is this? Can you paste here the contents of the lock file itself, as 
well as what process (if any) the PID in that file points to.
  
  Good point.

REPOSITORY
  R271 KDBusAddons

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

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns


D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Thiago Macieira
thiago added a comment.


  In D14302#297515 , @dfaure wrote:
  
  > The idea of the old code was: if I can't get the lock, then someone else is 
already in the process of starting kdeinit, so I'll just wait for that to 
happen, by locking again, i.e. blocking on purpose, and then checking that the 
DBus name is up, i.e. the other process did manage to do it successfully.
  
  
  It's Kind of silly to tryLock() then lock(). Why not just lock()? Was it the 
optimisation to avoid the D-Bus call? If so, a comment would be warranted.
  
  > I said from the start that it wasn't tryLock() that was blocking but 
lock(), good to see that this is now confirmed, however we're back to square 
one: why is lock never returning? Surely the other process which is executing 
this method is releasing the lock after the QProcess::execute, right?
  
  Yeah, I trusted the patch too. The frame for lock() is missing because it's a 
tail-call jump.
  
  Anyway, one theory for why the lock file is still present: it's stale from a 
previous boot, but the PID is taken by some process in the current boot. This 
is fixed in Qt 5.11 by saving the boot ID (commit 
772863355a0cf57a49e93608790dfd17c8fd82da). So question to @jtamate , what Qt 
version is this? Can you paste here the contents of the lock file itself, as 
well as what process (if any) the PID in that file points to.

REPOSITORY
  R271 KDBusAddons

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

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns


D10040: Add serial number and EISA ID to OutputDevice interface

2018-07-25 Thread David Edmundson
davidedmundson commandeered this revision.
davidedmundson added a reviewer: dvratil.

REPOSITORY
  R127 KWayland

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

To: davidedmundson, graesslin, sebas, #kwin, dvratil
Cc: kde-frameworks-devel, davidedmundson, plasma-devel, ragreen, Pitel, 
schernikov, michaelh, ZrenBot, ngraham, bruns, alexeymin, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, eliasp, sebas, apol, mart, hein


D14360: Remove custom icon selection for trash

2018-07-25 Thread Nathaniel Graham
ngraham requested changes to this revision.
ngraham added a reviewer: Frameworks.
ngraham added a comment.
This revision now requires changes to proceed.


  It's important to test your changes before submitting. :) This now causes 
Gwenview to segfault when you try to edit its Trash entry. The reason is 
because now that `m_iconButton` isn't added to the layout, the line beginning 
with `layout->labelForField()` crashes when it tries to run, since 
`m_iconButton isn't in a layout. That needs to be moved into the conditional as 
well.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, broulik, #dolphin, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham updated this revision to Diff 38452.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14360?vs=38405=38452

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

AFFECTED FILES
  src/filewidgets/kfileplaceeditdialog.cpp

To: shubham, ngraham, broulik, #dolphin
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D14360: Remove custom icon selection for trash

2018-07-25 Thread Nathaniel Graham
ngraham added a reviewer: Dolphin.
ngraham requested changes to this revision.
ngraham added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kfileplaceeditdialog.cpp:123
>  m_iconButton = new KIconButton(this);
> +m_iconButton->setVisible(url.scheme() != QLatin1String("trash"));
>  layout->addRow(i18n("Choose an :"), m_iconButton);
>  m_iconButton->setObjectName(QStringLiteral("icon button"));

Let's also avoid adding the button to the layout in this case, just like you're 
doing in D14378 .

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, broulik, #dolphin
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
This revision was automatically updated to reflect the committed changes.
Closed by commit R271:8097e1aa0d21: Dont block forever in 
ensureKdeinitRunning (authored by jtamate).

REPOSITORY
  R271 KDBusAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14302?vs=38403=38448

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

AFFECTED FILES
  src/kdeinitinterface.cpp

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns


D14360: Remove custom icon selection for trash

2018-07-25 Thread Nathaniel Graham
ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  This works from Gwenview (for example), but does not work from Dolphin, 
because sadly Dolphin has its own duplicate of this code. :( Could you submit 
another patch for Dolphin too? It's the same change, to a file by the same 
name. :(
  
  We really really really need to eliminate this duplication...

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D12820: Add KWayland virtual desktop protocol

2018-07-25 Thread Marco Martin
mart added a comment.


  RFC: add back some info for the layout? (in particular: rows and orientation, 
tought orientation is a bit concerning as is view-spacific)

REPOSITORY
  R127 KWayland

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

To: mart, #kwin, #plasma, graesslin, hein
Cc: davidedmundson, zzag, bshah, romangg, kde-frameworks-devel, michaelh, 
ngraham, bruns


D10343: Create containment on specified screen

2018-07-25 Thread Nathaniel Graham
ngraham added a reviewer: Plasma.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: hoffmannrobert, #plasma
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D13743: Migrate build system to use find_package in autotests/ki18n_install

2018-07-25 Thread Ralf Habacker
habacker added a comment.


  ping

REPOSITORY
  R249 KI18n

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

To: habacker, apol, ilic
Cc: bcooksley, ltoscano, kde-frameworks-devel, michaelh, ngraham, bruns


D10343: Create containment on specified screen

2018-07-25 Thread Robert Hoffmann
hoffmannrobert updated this revision to Diff 38410.
hoffmannrobert edited the test plan for this revision.
hoffmannrobert added a comment.
Restricted Application edited subscribers, added: kde-frameworks-devel; 
removed: Frameworks.


  - Remove unnecessary Containment::setLastScreen()

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10343?vs=26652=38410

BRANCH
  master

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

AFFECTED FILES
  src/plasma/corona.cpp
  src/plasma/corona.h
  src/plasma/private/corona_p.h

To: hoffmannrobert
Cc: kde-frameworks-devel, michaelh, ngraham, bruns, #frameworks


D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham updated this revision to Diff 38405.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14360?vs=38402=38405

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

AFFECTED FILES
  src/filewidgets/kfileplaceeditdialog.cpp

To: shubham, ngraham, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D14360: Remove custom icon selection for trash

2018-07-25 Thread Kai Uwe Broulik
broulik added a comment.


  If the URL of the places item is of scheme `trash` (Scheme is the part before 
the colon, e.g. `file`, `http`, …), then it should hide it, yes.

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham added a comment.


  broulik: you mean the visibility of button will depend on the type of 
KFilePlacesItem(if its trash , its hidden, else not)

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D14360: Remove custom icon selection for trash

2018-07-25 Thread Kai Uwe Broulik
broulik added a comment.


  Thanks a lot for your patch!
  
  I think it is cleaner if the button is setup but then hidden:
  
m_iconButton->setVisible(url.toString() != QLatin1String("trash:/"));
  
  Curiously, Dolphin overwrites the icon whenever `url.protocol() == "trash` 
whereas `KFilePlacesModel` only does it for `url.toString() == "trash:/"`. 
However you cannot actually add files inside Trash to Places in Dolphin (and 
why would you, you can from file dialog, though, but that is a bug) anyway.
  So perhaps can be simplified even to:
  
m_iconButton->setVisible(url.scheme() != QLatin1String("trash"));

REPOSITORY
  R241 KIO

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

To: shubham, ngraham, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D13782: RFC: Ignore NTFS hidden flag for root volume

2018-07-25 Thread Wolfgang Bauer
wbauer added a comment.


  In D13782#297639 , @wbauer wrote:
  
  > Using QDir::canonicalPath() (instead of QDir::cleanPath() ) would fix all 3 
cases:
  >
  >   const QString fullFilePath = QDir(path + QLatin1Char('/') + 
filename).canonicalPath();
  >
  
  
  Or rather, as the current directory is set to `path` in line#507, this would 
actually suffice:
  
const QString fullFilePath = QDir(filename).canonicalPath();

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, yurikoles, bruns
Cc: ngraham, oysteins, wbauer, kde-frameworks-devel, michaelh, bruns


D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread David Faure
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.


  That's reasonable. Still when you see this warning in the future, it would be 
useful to find out what kdeinit+klauncher are actually doing, if klauncher 
doesn't manage to register with DBus after 30 seconds...

REPOSITORY
  R271 KDBusAddons

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

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns


D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
jtamate updated this revision to Diff 38403.
jtamate edited the summary of this revision.
jtamate added a comment.


  Don't wait forever, if the lock is blocked for more than 30 seconds, write a 
warning and don't try to start kdeinit5 again.

REPOSITORY
  R271 KDBusAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14302?vs=38387=38403

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

AFFECTED FILES
  src/kdeinitinterface.cpp

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns


D14360: Remove custom icon selection for trash

2018-07-25 Thread Shubham
shubham created this revision.
shubham added reviewers: ngraham, broulik.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
shubham requested review of this revision.

REVISION SUMMARY
  BUG: 391200

TEST PLAN
  1. Right click trash on places panel
  2. Edit
  
  Result: No custom icon menu

REPOSITORY
  R241 KIO

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

AFFECTED FILES
  src/filewidgets/kfileplaceeditdialog.cpp

To: shubham, ngraham, broulik
Cc: kde-frameworks-devel, michaelh, ngraham, bruns


D13782: RFC: Ignore NTFS hidden flag for root volume

2018-07-25 Thread Wolfgang Bauer
wbauer added a comment.


  In D13782#297547 , @wbauer wrote:
  
  > With additional debug output I do see the '.' entry in the root dir still 
marked as hidden (as mentioned in the test plan), and also the '..' entry in 
(first-level) subdirs is hidden.
  
  
  I also found another problem though: symlinks to the mountpoint/NTFS root dir 
are (still) hidden as well (also if they are on different partitions).
  
  Using QDir::canonicalPath() (instead of QDir::cleanPath() ) would fix all 3 
cases:
  
const QString fullFilePath = QDir(path + QLatin1Char('/') + 
filename).canonicalPath();
  
  (that one isn't static...)

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, yurikoles, bruns
Cc: ngraham, oysteins, wbauer, kde-frameworks-devel, michaelh, bruns


D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread David Faure
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  The comments look good. The actual change, not so much ;)

INLINE COMMENTS

> kdeinitinterface.cpp:72
> +QProcess proc;
> +// started asynchronously, so even if kdeinit5 takes forever to start, 
> the lock will be released
> +proc.start(srv, args);

But that means the other processes coming into this method, will either succeed 
in  tryLock() and will run yet another kdeinit instance (your system is slow 
and you're bombarding it with kdeinit processes trying to start at the same 
time?),
or worse, tryLock() fails and lock() succeeds (because the first process 
released the lock too early here), and then isServiceRegistered fails (because 
we're trying too early), and again we start yet another kdeinit process that 
will fight it with the currently starting one.

If the bug is that kdeinit takes forever to start (not just a long time) and 
QProcess::execute never returns, then that's the actual bug. This proposed 
change is just a workaround which potentially makes things worse (10 kdeinit 
processes attempting to start at the same time).

BTW ~QProcess would kill kdeinit, what you meant was startDetached. But all of 
the above is the reasoning against startDetached, actually ;)

REPOSITORY
  R271 KDBusAddons

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

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns


D13782: RFC: Ignore NTFS hidden flag for root volume

2018-07-25 Thread Wolfgang Bauer
wbauer added a subscriber: ngraham.
wbauer added a comment.


  In D13782#296381 , @ngraham wrote:
  
  > @wbauer, does this work for you now?
  
  
  Yes, it does work as expected now here in all cases I tested, and I didn't 
notice problems.
  
  With additional debug output I do see the '.' entry in the root dir still 
marked as hidden (as mentioned in the test plan), and also the '..' entry in 
(first-level) subdirs is hidden.
  This could be fixed e.g. by using QDir::cleanPath(), i.e.:
  
const QString fullFilePath = QDir::cleanPath(path + QLatin1Char('/') + 
filename);
  
  But I'm not sure that is really necessary as those entries are not displayed 
at all anyway (even with hidden files enabled).

INLINE COMMENTS

> file_unix.cpp:575
> +if (ntfsHidden) {
> +entry.insert(KIO::UDSEntry::UDS_HIDDEN, 1);
> +}

This is entry.fastInsert(...) now as of KIO 5.48.0, see 
https://cgit.kde.org/kio.git/commit/?id=7048d259529fd37134e9bc1bc8d3edc3d4ac8ef4
Maybe the patch should be rebased...

REPOSITORY
  R241 KIO

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

To: broulik, dfaure, yurikoles, bruns
Cc: ngraham, oysteins, wbauer, kde-frameworks-devel, michaelh, bruns


D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
jtamate updated this revision to Diff 38387.
jtamate edited the summary of this revision.
jtamate added a comment.


  > I said from the start that it wasn't tryLock() that was blocking but 
lock(), good to see that this is now confirmed.
  
  I trusted blindly in gdb stacktrace, and also I didn't understood, again, the 
code.
  
  Added your description as comments.

REPOSITORY
  R271 KDBusAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14302?vs=38372=38387

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

AFFECTED FILES
  src/kdeinitinterface.cpp

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns


D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread David Faure
dfaure added a comment.


  The idea of the old code was: if I can't get the lock, then someone else is 
already in the process of starting kdeinit, so I'll just wait for that to 
happen, by locking again, i.e. blocking on purpose, and then checking that the 
DBus name is up, i.e. the other process did manage to do it successfully.
  
  I can't wrap my head around whatever the new code is trying to do instead.
  
  I said from the start that it wasn't tryLock() that was blocking but lock(), 
good to see that this is now confirmed, however we're back to square one: why 
is lock never returning? Surely the other process which is executing this 
method is releasing the lock after the QProcess::execute, right?

REPOSITORY
  R271 KDBusAddons

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

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns


D14302: Don't block forever in ensureKdeinitRunning

2018-07-25 Thread Jaime Torres Amate
jtamate updated this revision to Diff 38372.
jtamate edited the summary of this revision.
jtamate edited the test plan for this revision.
jtamate added a comment.


  It is my first time of a backtrace that doesn't reflect the true state of a 
program. But I've learned the lesson, in case of counter intuitive backtrace, 
take a look at the dissasembler.
  Thanks everybody, specially to @thiago.

REPOSITORY
  R271 KDBusAddons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D14302?vs=38264=38372

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

AFFECTED FILES
  src/kdeinitinterface.cpp

To: jtamate, dfaure, #frameworks, thiago
Cc: lvsouza, kde-frameworks-devel, michaelh, ngraham, bruns