D21367: kioexec: change the scary debug messages for delayed deletion
This revision was automatically updated to reflect the committed changes. Closed by commit R241:cb5a3ac8ff22: kioexec: change the scary debug messages for delayed deletion (authored by Lekensteyn). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21367?vs=58714=59022 REVISION DETAIL https://phabricator.kde.org/D21367 AFFECTED FILES src/kioexec/main.cpp To: Lekensteyn, elvisangelaccio, dfaure Cc: ngraham, kde-frameworks-devel, michaelh, bruns
D21367: kioexec: change the scary debug messages for delayed deletion
dfaure accepted this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D21367 To: Lekensteyn, elvisangelaccio, dfaure Cc: ngraham, kde-frameworks-devel, michaelh, bruns
D21367: kioexec: change the scary debug messages for delayed deletion
elvisangelaccio accepted this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D21367 To: Lekensteyn, elvisangelaccio, dfaure Cc: ngraham, kde-frameworks-devel, michaelh, bruns
D21367: kioexec: change the scary debug messages for delayed deletion
Lekensteyn updated this revision to Diff 58714. Lekensteyn set the repository for this revision to R241 KIO. Lekensteyn added a comment. Using camelCase now. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21367?vs=58640=58714 REVISION DETAIL https://phabricator.kde.org/D21367 AFFECTED FILES src/kioexec/main.cpp To: Lekensteyn, elvisangelaccio, dfaure Cc: ngraham, kde-frameworks-devel, michaelh, bruns
D21367: kioexec: change the scary debug messages for delayed deletion
elvisangelaccio added inline comments. INLINE COMMENTS > main.cpp:262 > // it will have time to start up and read the file before it > gets deleted. #130709. > -qDebug() << "sleeping..."; > -QThread::currentThread()->sleep(180); // 3 mn > +const int sleep_secs = 180; > +qDebug() << "sleeping for" << sleep_secs << "seconds before > deleting file..."; Please use camelCase REVISION DETAIL https://phabricator.kde.org/D21367 To: Lekensteyn, elvisangelaccio, dfaure Cc: ngraham, kde-frameworks-devel, michaelh, bruns
D21367: kioexec: change the scary debug messages for delayed deletion
Lekensteyn updated this revision to Diff 58640. Lekensteyn marked an inline comment as done. Lekensteyn edited the test plan for this revision. Lekensteyn added a comment. Addressed review comment by adding a constant for the interval. Changed minutes -> seconds in the debug message. CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D21367?vs=58549=58640 REVISION DETAIL https://phabricator.kde.org/D21367 AFFECTED FILES src/kioexec/main.cpp To: Lekensteyn, elvisangelaccio, dfaure Cc: ngraham, kde-frameworks-devel, michaelh, bruns
D21367: kioexec: change the scary debug messages for delayed deletion
elvisangelaccio added inline comments. INLINE COMMENTS > main.cpp:261-263 > // it will have time to start up and read the file before it > gets deleted. #130709. > -qDebug() << "sleeping..."; > +qDebug() << "sleeping for 3 minutes before deleting file..."; > QThread::currentThread()->sleep(180); // 3 mn While at it, maybe it could be worth to put 180 in some variable `foo` and then use `foo/60` instead of hardcoding the number of minutes in the debug message. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D21367 To: Lekensteyn, elvisangelaccio, dfaure Cc: ngraham, kde-frameworks-devel, michaelh, bruns
D21367: kioexec: change the scary debug messages for delayed deletion
dfaure accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D21367 To: Lekensteyn, elvisangelaccio, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D21367: kioexec: change the scary debug messages for delayed deletion
Lekensteyn created this revision. Lekensteyn added reviewers: elvisangelaccio, dfaure. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. Lekensteyn requested review of this revision. REVISION SUMMARY After resuming my laptop, I found this scary message in journalctl: org.kde.Spectacle[123]: about to delete "/home/user/Pictures" containing "Screenshot_png" Apparently this message is due to me closing Kolourpaint and Spectacle right before system sleep which scheduled a delayed deletion. Make the debug messages more obvious to avoid any doubt. TEST PLAN Assume /tmp to be non-empty, then `touch /tmp/x.png` and `kioexec --tempfiles ls /tmp/x.png`. Expected output: EXEC "/usr/bin/ls /tmp/x.png" /tmp/x.png EXEC done sleeping for 3 minutes before deleting file... Three minutes have passed, deleting "/tmp/x.png" Doing something similar, but with `/tmp/z/x.txt` in a directory with no other files: ... Three minutes have passed, deleting "/tmp/z/some.txt" Removed empty parent directory "/tmp/z" Discussion: perhaps the sleep should only occur if the problematic loffice/Amarok case was detected, namely a process that quits quickly (e.g. <3 seconds?). However since I did not ran into any issue so far, I'll limit it to this error message change. Deleting the parent directory is also questionable, but again it did not actually trigger any issue so far. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D21367 AFFECTED FILES src/kioexec/main.cpp To: Lekensteyn, elvisangelaccio, dfaure Cc: kde-frameworks-devel, michaelh, ngraham, bruns
