D17545: Do not stat move/copy job if the destination file system does not support writing

2019-01-19 Thread Shubham
shubham abandoned this revision. shubham added a comment. Abandoned in favor of D17632 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17545 To: shubham, #frameworks, dfaure Cc: davidedmundson, ngraham, broulik, kde-frameworks-devel,

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-16 Thread David Faure
dfaure added a comment. My proposal is at https://phabricator.kde.org/D17632 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17545 To: shubham, #frameworks, dfaure Cc: davidedmundson, ngraham, broulik, kde-frameworks-devel, michaelh, bruns

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-16 Thread David Faure
dfaure added a comment. In fact this is the wrong location for this code. The checks on the dest dir are in CopyJobPrivate::slotResultStating. I'm working on a different fix. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17545 To: shubham, #frameworks, dfaure Cc:

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-16 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > jobtest.cpp:1004 > + > +QSignalSpy spy(job, ::error); > +job->setUiDelegate(nullptr); At runtime, Qt says: QSignalSpy: Not a valid signal: '' Indeed this is the getter, not a signal. `result()` is the signal, but you don't really need

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-16 Thread Shubham
shubham added a comment. Did anyone tried it? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17545 To: shubham, #frameworks, dfaure Cc: davidedmundson, ngraham, broulik, kde-frameworks-devel, michaelh, bruns

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-14 Thread Shubham
shubham marked an inline comment as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17545 To: shubham, #frameworks, dfaure Cc: davidedmundson, ngraham, broulik, kde-frameworks-devel, michaelh, bruns

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-14 Thread Shubham
shubham updated this revision to Diff 47549. shubham added a comment. Unit test REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17545?vs=47503=47549 REVISION DETAIL https://phabricator.kde.org/D17545 AFFECTED FILES autotests/jobtest.cpp

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-13 Thread Shubham
shubham added a comment. @dfaure I'm not so familiar writing unit tests. This is one I have made by doing some modifications. would you like to give me some help? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17545 To: shubham, #frameworks, dfaure Cc: davidedmundson,

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-13 Thread Shubham
shubham added a comment. void JobTest::moveDirectoryToInaccessibleFilesystem() { #ifdef Q_OS_WIN QSKIP("Skipping unaccessible folder test on Windows, cannot remove all permissions from a folder"); #endif // Given a directory that cannot be moved to destination

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-13 Thread David Faure
dfaure added a comment. The logic is the same in all cases, the mode doesn't really make a difference, so I'm ok with picking one -- let's say Move, so we can also check that nothing disappeared. File or folder also makes no difference, so I'm ok with just one of these. The one thing

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-13 Thread Shubham
shubham marked an inline comment as done. shubham added a comment. @dfaure do you need tests for all three modes ie. copy,move and link And individually for files and folders REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17545 To: shubham, #frameworks, dfaure Cc:

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-13 Thread David Faure
dfaure added inline comments. INLINE COMMENTS > shubham wrote in copyjob.cpp:868-872 > emitresult() emits the corresponding signal and then commits suicide, hence > no such chance possible. Still, don't call statNextSrc() then. REPOSITORY R241 KIO REVISION DETAIL

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-13 Thread Shubham
shubham marked an inline comment as done. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17545 To: shubham, #frameworks, dfaure Cc: davidedmundson, ngraham, broulik, kde-frameworks-devel, michaelh, bruns

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-13 Thread Shubham
shubham added inline comments. INLINE COMMENTS > davidedmundson wrote in copyjob.cpp:868-872 > emitting a result and then continuing seems questionable, you'll end up with > the job emitting another result later emitresult() emits the corresponding signal and then commits suicide, hence no

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-13 Thread David Edmundson
davidedmundson added a comment. > And there should be a unittest for it. This still needs doing INLINE COMMENTS > copyjob.cpp:867 > +q->setError(ERR_CANNOT_WRITE); > +q->setErrorText(m_currentDestURL.toDisplayString()); > +q->emitResult();

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-13 Thread Shubham
shubham updated this revision to Diff 47503. shubham marked 3 inline comments as done. shubham added a comment. Done above requested changes. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17545?vs=47465=47503 REVISION DETAIL

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-13 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > copyjob.cpp:862 > +// if the destination file system doesn't support writing, do not > stat > +QFileInfo

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-12 Thread Shubham
shubham added a subscriber: ngraham. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17545 To: shubham, #frameworks, dfaure Cc: ngraham, broulik, kde-frameworks-devel, michaelh, bruns

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-12 Thread Shubham
shubham added a reviewer: dfaure. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D17545 To: shubham, #frameworks, dfaure Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-12 Thread Shubham
shubham updated this revision to Diff 47465. REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D17545?vs=47462=47465 REVISION DETAIL https://phabricator.kde.org/D17545 AFFECTED FILES src/core/copyjob.cpp To: shubham, #frameworks Cc: broulik,

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-12 Thread Kai Uwe Broulik
broulik added a comment. The bug report is about checking whether it's writable, you're checking the protocol here, and `file` will always "support writing". This patch could still make sense, though. INLINE COMMENTS > copyjob.cpp:864 > +QPointer that = q; > +emit

D17545: Do not stat move/copy job if the destination file system does not support writing

2018-12-12 Thread Shubham
shubham created this revision. shubham added a reviewer: Frameworks. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. shubham requested review of this revision. REVISION SUMMARY BUG: 141564 REPOSITORY R241 KIO REVISION DETAIL