D28902: CopyJob: when stat'ing the dest, use StatBasic.

2020-05-15 Thread Méven Car 
meven added a comment.


  Fix at D29767 

REPOSITORY
  R241 KIO

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

To: dfaure, meven
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns

D28902: CopyJob: when stat'ing the dest, use StatBasic.

2020-05-14 Thread Méven Car 
meven added a comment.


  Fix and test coming

REPOSITORY
  R241 KIO

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

To: dfaure, meven
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns

D28902: CopyJob: when stat'ing the dest, use StatBasic.

2020-05-14 Thread Méven Car 
meven added a comment.


  In D28902#671260 , @ngraham wrote:
  
  > @dfaure git bisect says this caused 
https://bugs.kde.org/show_bug.cgi?id=421213.
  >
  > After fixing, maybe we should get a test for that use case?
  
  
  I see the issue, because we don't resolve the symlink here, we don't figure 
out dest is a folder and hence copyjob acts as if moving file to dest file, 
instead of to dest dir.

REPOSITORY
  R241 KIO

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

To: dfaure, meven
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns

D28902: CopyJob: when stat'ing the dest, use StatBasic.

2020-05-14 Thread Nathaniel Graham 
ngraham added a comment.


  @dfaure git bisect says this caused 
https://bugs.kde.org/show_bug.cgi?id=421213.
  
  After fixing, maybe we should get a test for that use case?

REPOSITORY
  R241 KIO

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

To: dfaure, meven
Cc: ngraham, kde-frameworks-devel, LeGast00n, cblack, michaelh, bruns

D28902: CopyJob: when stat'ing the dest, use StatBasic.

2020-04-18 Thread Méven Car 
meven added a comment.


  D28947  for the StatAcl fix

REPOSITORY
  R241 KIO

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

To: dfaure, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28902: CopyJob: when stat'ing the dest, use StatBasic.

2020-04-18 Thread David Faure 
dfaure added a comment.


  In D28902#650160 , @meven wrote:
  
  > So the `settings:/` , `remote:/` should return UDS_LOCAL_PATH for 
KIO::StatBasic, but file:/ should not .
  
  
  That's for sure, it's redundant for kio_file.
  
  > The code here uses UDS_LOCAL_PATH when present only and does not require it
  
  Of course. Remote ioslaves can't even set it ;)
  I have adjusted the comment to avoid sounding like it's required.

REPOSITORY
  R241 KIO

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

To: dfaure, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28902: CopyJob: when stat'ing the dest, use StatBasic.

2020-04-18 Thread David Faure 
dfaure updated this revision to Diff 80462.
dfaure added a comment.


  update comment

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28902?vs=80338=80462

BRANCH
  2020_use_StatDetails

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

AFFECTED FILES
  src/core/copyjob.cpp

To: dfaure, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28902: CopyJob: when stat'ing the dest, use StatBasic.

2020-04-18 Thread David Faure 
dfaure closed this revision.

REPOSITORY
  R241 KIO

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

To: dfaure, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28902: CopyJob: when stat'ing the dest, use StatBasic.

2020-04-17 Thread Méven Car 
meven added a comment.


  I believe that's just fine but the comment could be updated :
  From udsentry.h
  
/// A local file path if the ioslave display files sitting
/// on the local filesystem (but in another hierarchy, e.g. settings:/ or 
remote:/)
UDS_LOCAL_PATH
  
  So the `settings:/` , `remote:/` should return UDS_LOCAL_PATH for 
KIO::StatBasic, but file:/ should not .
  The code here uses UDS_LOCAL_PATH when present only and does not require it.

REPOSITORY
  R241 KIO

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

To: dfaure, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28902: CopyJob: when stat'ing the dest, use StatBasic.

2020-04-17 Thread Méven Car 
meven accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R241 KIO

BRANCH
  master

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

To: dfaure, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D28902: CopyJob: when stat'ing the dest, use StatBasic.

2020-04-16 Thread David Faure 
dfaure created this revision.
dfaure added a reviewer: meven.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
dfaure requested review of this revision.

REVISION SUMMARY
  However the StatDetail enum doesn't specify which enum value will get us
  UDS_LOCAL_PATH (because kio_file doesn't have that). kio_desktop does,
  but doesn't honour StatDetail yet. One approach is to consider it part
  of Basic, the other is to add a new enum value for it.

TEST PLAN
  jobtest and testtrash still pass

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  src/core/copyjob.cpp

To: dfaure, meven
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns