D27873: sftp: do not show creation time when we do not know it

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R320:b13449a13b00: sftp: do not show creation time when we do 
not know it (authored by sitter).

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27873?vs=78352=78444

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

AFFECTED FILES
  sftp/kio_sftp.cpp

To: sitter, ngraham, bruns, meven
Cc: meven, bruns, kde-frameworks-devel, kfm-devel, nikolaik, pberestov, 
iasensio, fprice, LeGast00n, cblack, fbampaloukas, alexde, GB_2, Codezela, 
feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov

D27873: sftp: do not show creation time when we do not know it

[Stefan Brüns]

bruns accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R320 KIO Extras

BRANCH
  sftp-ctime

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

To: sitter, ngraham, bruns, meven
Cc: meven, bruns, kde-frameworks-devel, kfm-devel, nikolaik, pberestov, 
iasensio, fprice, LeGast00n, cblack, fbampaloukas, alexde, GB_2, Codezela, 
feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov

D27873: sftp: do not show creation time when we do not know it

[Stefan Brüns]

bruns added a comment.


  Looks ok now.
  
  Should be followed up by a corresponding change for 
ACCESS_TIME/MODIFICATION_TIME - both are optional as well.

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, bruns, meven
Cc: meven, bruns, kde-frameworks-devel, kfm-devel, nikolaik, pberestov, 
iasensio, fprice, LeGast00n, cblack, fbampaloukas, alexde, GB_2, Codezela, 
feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov

D27873: sftp: do not show creation time when we do not know it

[Méven Car]

meven accepted this revision.
meven added a comment.


  Ok to me @bruns ?

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, bruns, meven
Cc: meven, bruns, kde-frameworks-devel, kfm-devel, nikolaik, pberestov, 
iasensio, fprice, LeGast00n, cblack, fbampaloukas, alexde, GB_2, Codezela, 
feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov

D27873: sftp: do not show creation time when we do not know it

[Harald Sitter]

sitter updated this revision to Diff 78352.
sitter added a comment.


  .

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27873?vs=77080=78352

BRANCH
  sftp-ctime

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

AFFECTED FILES
  sftp/kio_sftp.cpp

To: sitter, ngraham, bruns
Cc: meven, bruns, kde-frameworks-devel, kfm-devel, nikolaik, pberestov, 
iasensio, fprice, LeGast00n, cblack, fbampaloukas, alexde, GB_2, Codezela, 
feverfew, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov

D27873: sftp: do not show creation time when we do not know it

[Méven Car]

meven added a comment.


  In D27873#624567 , @sitter wrote:
  
  > In D27873#623582 , @bruns wrote:
  >
  > > Please correct the comment in the code - it depends on the server SFTP 
implementation.
  >
  >
  > Birth time does not depend on the file system?
  
  
  I guess on both but ultimately on the SFTP implementation that can lack the 
implementation when the underlying filesystem has the birth time.

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, bruns
Cc: meven, bruns, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, 
LeGast00n, cblack, fbampaloukas, alexde, GB_2, Codezela, feverfew, michaelh, 
spoorun, navarromorales, firef, ngraham, andrebarros, emmanuelp, mikesomov

D27873: sftp: do not show creation time when we do not know it

[Harald Sitter]

sitter added a comment.


  ping

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, bruns
Cc: bruns, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, 
LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov

D27873: sftp: do not show creation time when we do not know it

[Harald Sitter]

sitter added a comment.


  In D27873#623582 , @bruns wrote:
  
  > Please correct the comment in the code - it depends on the server SFTP 
implementation.
  
  
  Birth time does not depend on the file system?

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, bruns
Cc: bruns, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, 
LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov

D27873: sftp: do not show creation time when we do not know it

[Stefan Brüns]

bruns requested changes to this revision.
bruns added a comment.
This revision now requires changes to proceed.


  Please correct the comment in the code - it depends on the server SFTP 
implementation.

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, bruns
Cc: bruns, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, 
LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov

D27873: sftp: do not show creation time when we do not know it

[Harald Sitter]

sitter updated this revision to Diff 77080.
sitter added a comment.


  use flaggy

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27873?vs=77039=77080

BRANCH
  sftp-ctime

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

AFFECTED FILES
  sftp/kio_sftp.cpp

To: sitter, ngraham, bruns
Cc: bruns, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, 
LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov

D27873: sftp: do not show creation time when we do not know it

[Stefan Brüns]

bruns added a comment.


  At least the openssh implementation apparently only supports atime/mtime:
  
https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/sftp-common.c?rev=1.31=text/x-cvsweb-markup

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, bruns
Cc: bruns, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, 
LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov

D27873: sftp: do not show creation time when we do not know it

[Stefan Brüns]

bruns requested changes to this revision.
bruns added a comment.
This revision now requires changes to proceed.


  Btrfs definitely does support birthtime

INLINE COMMENTS

> kio_sftp.cpp:496
>  entry.fastInsert(KIO::UDSEntry::UDS_MODIFICATION_TIME, sb->mtime);
> -entry.fastInsert(KIO::UDSEntry::UDS_CREATION_TIME, sb->createtime);
> +if (sb->createtime != 0) {
> +// Birth time may be unknown if the underlying file system 
> doesn't

You should check for `SSH_FILEXFER_ATTR_CREATETIME` in the flags field:
https://api.libssh.org/master/group__libssh__sftp.html#gabbfacf308d2116ae09b1ca4f901afe2d

REPOSITORY
  R320 KIO Extras

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

To: sitter, ngraham, bruns
Cc: bruns, kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, 
LeGast00n, cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, 
meven, michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, 
emmanuelp, mikesomov

D27873: sftp: do not show creation time when we do not know it

[Nathaniel Graham]

ngraham accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R320 KIO Extras

BRANCH
  sftp-ctime

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

To: sitter, ngraham
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, 
cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, 
michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, mikesomov

D27873: sftp: do not show creation time when we do not know it

[Harald Sitter]

sitter created this revision.
sitter added a reviewer: ngraham.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
sitter requested review of this revision.

REVISION SUMMARY
  creation time (aka birth time) is file system dependent, if the fs doesn't
  support it we seem to get createtime==0. skip over it when that is the
  case.
  
  BUG: 375305
  FIXED-IN: 20.04

TEST PLAN
  browsing a directory on btrfs over sftp doesn't show creation time as 1970

REPOSITORY
  R320 KIO Extras

BRANCH
  sftp-ctime

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

AFFECTED FILES
  sftp/kio_sftp.cpp

To: sitter, ngraham
Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice, LeGast00n, 
cblack, MrPepe, fbampaloukas, alexde, GB_2, Codezela, feverfew, meven, 
michaelh, spoorun, navarromorales, firef, ngraham, andrebarros, bruns, 
emmanuelp, mikesomov