This forum uses cookies. Click X button to hide this message. What is stored? / Privacy
Total Commander Forum Index Total Commander
Forum - Public Discussion and Support
 
 FAQFAQ   SearchSearch   MemberlistMemberlist   UsergroupsUsergroups   RegisterRegister 
 ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

Synchronize Dirs - file properties not synchronized properly

 
Post new topic   Reply to topic    Total Commander Forum Index -> TC9.1x bug reports (English) Printable version
View previous topic :: View next topic  
Author Message
MarcinW
Power Member
Power Member


Joined: 23 Jan 2012
Posts: 763
Location: Poland

PostPosted: Mon Nov 20, 2017 5:08 pm    Post subject: Synchronize Dirs - file properties not synchronized properly Reply with quote

Steps to reproduce (TC 9.12 RC4):

1a) In left panel, create empty file: "c:\test1\file.txt"
1b) Set read-only attribute of this file and clear all other attributes
1c) Set date and time of this file to, let's say, 2017-01-01 1:00:00

2a) In right panel, create empty file: "c:\test2\file.txt"
2b) Set archive attribute of this file and clear all other attributes
2c) Set date and time of this file to the current date and time

3) Launch "Commands" -> "Synchronize Dirs..."
4) Ensure that "ignore date" option is unchecked and press "Compare" button - a line with information about both "file.txt" files will be displayed
5) Right-click this line and choose option "Copy file properties <- (right to left)..."
6) A "Copy file properties <-" dialog appears: check "Timestamp (date+time)" option and "File attributes" option and press OK

In left panel, attributes of the "c:\test1\file.txt" file were properly changed from read-only to archive, but date and time were NOT changed and are still 2017-01-01 1:00:00

Regards
Back to top
View user's profile Send private message Send e-mail
ghisler(Author)
Site Admin
Site Admin


Joined: 04 Feb 2003
Posts: 35473
Location: Switzerland

PostPosted: Tue Nov 21, 2017 10:48 am    Post subject: Reply with quote

This happens because Windows doesn't allow to modify files with read only attribute - not even the time stamp. I will try to fix it.
_________________
Author of Total Commander
http://www.ghisler.com
Back to top
View user's profile Send private message Send e-mail Visit poster's website
MarcinW
Power Member
Power Member


Joined: 23 Jan 2012
Posts: 763
Location: Poland

PostPosted: Mon May 07, 2018 7:44 am    Post subject: Reply with quote

Original problem has been fixed, but there is still a problem with the current implementation (TC 9.20 beta 1). Try this:

1a) In left panel, create empty file: "c:\test1\file.txt"
1b) Clear all the attributes of the file (i.e. RAHS)

2a) In right panel, create empty file: "c:\test2\file.txt"
2b) Change access rights of the file - for example grant all access rights to everyone
2c) Clear all the attributes of the file (i.e. RAHS)

3) Launch "Commands" -> "Synchronize Dirs..."
4) Press "Compare" button - a line with information about both "file.txt" files will be displayed
5) Right-click this line and choose option "Copy file properties <- (right to left)..."
6) A "Copy file properties <-" dialog appears: check "NTFS permissions" option and press OK

In left panel, an ARCHIVE attribute of the "c:\test1\file.txt" file has been set.

It seems that modifying NTFS permissions makes an ARCHIVE attribute set.

Regards
Back to top
View user's profile Send private message Send e-mail
Dalai
Power Member
Power Member


Joined: 28 Jan 2005
Posts: 6000
Location: Meiningen (Südthüringen)

PostPosted: Mon May 07, 2018 8:10 am    Post subject: Reply with quote

MarcinW wrote:
It seems that modifying NTFS permissions makes an ARCHIVE attribute set.

Yes, it does, and it makes sense because the file needs to be archived (backed up) again, at least in case all permissions are part of the backup.

Regards
Dalai
_________________
#101164 Personal licence
Athlon X4 880K, 16 GiB RAM, Gigabyte F2A88X-D3HP, Win7 x64

Plugins: Services2, Startups
Back to top
View user's profile Send private message Send e-mail
MarcinW
Power Member
Power Member


Joined: 23 Jan 2012
Posts: 763
Location: Poland

PostPosted: Mon May 07, 2018 11:09 am    Post subject: Reply with quote

You are right - so TC should remove an ARCHIVE attribute when it's not desired.
Back to top
View user's profile Send private message Send e-mail
ghisler(Author)
Site Admin
Site Admin


Joined: 04 Feb 2003
Posts: 35473
Location: Switzerland

PostPosted: Fri May 11, 2018 2:43 am    Post subject: Reply with quote

This should be fixed in TC 9.20 beta 2, please test it!
_________________
Author of Total Commander
http://www.ghisler.com
Back to top
View user's profile Send private message Send e-mail Visit poster's website
MarcinW
Power Member
Power Member


Joined: 23 Jan 2012
Posts: 763
Location: Poland

PostPosted: Fri May 11, 2018 5:12 am    Post subject: Reply with quote

I need a bit more time to test this thoroughly.
Back to top
View user's profile Send private message Send e-mail
MarcinW
Power Member
Power Member


Joined: 23 Jan 2012
Posts: 763
Location: Poland

PostPosted: Thu May 17, 2018 9:31 am    Post subject: Reply with quote

Still not good.

Try this (TC 9.20 beta 3):

1a) In left panel, create empty file: "c:\test1\file.txt"
1b) Set Read Only, Hidden and System attributes
1c) Clear all access rights for the file - remove all access rights for everyone (this will also set Archive attribute)

2) In right panel, create empty file: "c:\test2\file.txt"

Now synchronize attributes and NTFS access rights from left to right.


Instead of having the right file with Read Only, Hidden, System and Archive attributes set, we get a file with only Archive attribute set. Not good. It's worse than in TC 9.12.


General algorithm idea should be:
1) set date/time and attributes (may fail if old NTFS permissions deny this operation)
2) change NTFS permissions (this will set an Archive attribute)
3) set date/time and attributes again, if failed in step 1 or if the Archive attribute needs to be cleared (may fail if new NTFS permissions deny this operation)

When using this idea - except the case, when both old and new NTFS permissions deny any operations - the worst case failure is when we have an Archive attribute set (but it shouldn't be set) - not when we have Read Only, Hidden and System attributes cleared (when they should be set).


I created a draft of the algorithm - pay attention to "for Retry:=False to True do" loop:
Code:
{Notes:
- setting date/time requires opening the file (SetFileTime),
  so Read-Only attribute must NOT be set
- setting NTFS access rights makes Archive attribute set
}
procedure SyncAlgorithmDraft(
  SyncDateTime, SyncAttr, SyncAccessRights : Boolean;
  const SrcFileName, DestFileName : string;
  const SrcDateTime : TFileTime;
  const SrcAttr : Integer; const DestAttr : Integer);
var
  SyncDateTimeERR, SyncAttrERR, SyncAccessRightsERR : Boolean;
  NeededDestDateTime : TFileTime;
  NeededDestAttr : Integer;
  NeededDestAccessRights : record end; // place some needed data format here
  Retry : Boolean;
begin
  NeededDestDateTime:=SrcDateTime;
  if SyncAttr then
    NeededDestAttr:=SrcAttr
  else
    NeededDestAttr:=DestAttr;

  for Retry:=False to True do
  begin
    SyncDateTimeERR:=False;
    SyncAttrERR:=False;
    SyncAccessRightsERR:=False;

    {==========================================================================}
    {STEP 1: Synchronize date/time}
    if SyncDateTime then
    if not SetFileTimeWrapper(DestFileName,NeededDestDateTime) then
    begin
      if DestAttr and FILE_ATTRIBUTE_READONLY <> 0 then
      if FileSetAttr(DestFileName,NeededDestAttr and not FILE_ATTRIBUTE_READONLY) = ERROR_SUCCESS then
      begin
        {If we don't need Read-Only attribute set, we are ready with
         attributes; if we need it set - we must force setting it}
        SyncAttr:=NeededDestAttr and FILE_ATTRIBUTE_READONLY <> 0;

        if not SetFileTimeWrapper(DestFileName,NeededDestDateTime) then
          SyncDateTimeERR:=True;
      end;
    end;

    {==========================================================================}
    {STEP 2: Synchronize attributes}
    if SyncAttr then
    if FileSetAttr(DestFileName,NeededDestAttr) <> ERROR_SUCCESS then
      SyncAttrERR:=True;

    if not SyncAccessRights then
      Break; {We are ready}
    if Retry then
      Break; {We already tried with new access rights}

    {==========================================================================}
    {STEP 3: Synchronize access rights}
    if not GetFileAccessRightsWrapper(SrcFileName,NeededDestAccessRights) then
    begin
      SyncAccessRightsERR:=True;
      Break; {Access rights have NOT been changed, so retrying with new access
              rights makes no sense}
    end;
    if not SetFileAccessRightsWrapper(DestFileName,NeededDestAccessRights) then
    begin
      SyncAccessRightsERR:=True;
      Break; {Access rights have NOT been changed, so retrying with new access
              rights makes no sense}
    end;

    {==========================================================================}
    {STEP 4: Handle retrying}

    {In case of earlier error, schedule setting date/time again, since the
     destination file has new access rights now}
    SyncDateTime:=SyncDateTimeERR;

    {In case of earlier error, schedule setting attributes again, since the
     destination file has new access rights now}
    {Setting access rights makes Archive attribute set, so schedule clearing
     it, if it's not desired}
    SyncAttr:=SyncAttrERR or (NeededDestAttr and FILE_ATTRIBUTE_ARCHIVE = 0);
  end;

  if SyncDateTimeERR or SyncAttrERR or SyncAccessRightsERR then
    ; // Show some message here, like "Some of the file properties could not be set properly"
end;
Back to top
View user's profile Send private message Send e-mail
ghisler(Author)
Site Admin
Site Admin


Joined: 04 Feb 2003
Posts: 35473
Location: Switzerland

PostPosted: Fri May 18, 2018 4:19 am    Post subject: Reply with quote

Sorry, but this wouldn't work either - the new access rights may prevent the setting of attributes too, e.g. when copying other user's files. It may be tricky to set access rights to allow setting file attributes. Therefore I will keep it as it is now.
_________________
Author of Total Commander
http://www.ghisler.com
Back to top
View user's profile Send private message Send e-mail Visit poster's website
MarcinW
Power Member
Power Member


Joined: 23 Jan 2012
Posts: 763
Location: Poland

PostPosted: Fri May 18, 2018 9:16 am    Post subject: Reply with quote

ghisler(Author) wrote:
the new access rights may prevent the setting of attributes too, e.g. when copying other user's files

Of course you are right. Maybe I wasn't clear enough.

I was talking here only about cases when:
1) old access rights don't, but new access rights do allow setting attributes and date/time,
2) old access rights do, but new access rights don't allow setting attributes and date/time.


Current TC 9.12 behavior is:
TC sets attributes before setting new access rights, so it fails in case 1) and succeeds in case 2)

Current TC 9.20 beta 3 behavior is:
TC sets attributes after setting new access rights, so it succeeds in case 1) and fails in case 2)

What I'm trying to say, is that TC should set attributes and and date/time before and also after changing access rights, so it will handle both case 1) and case 2).


Instead of placing the same piece of code (setting date/time and attributes) twice in the SyncAlgorithmDraft above, I just used a "for Retry:=False to True do" loop. Please load this piece of code into Delphi, so it will be formatted and colored well, comments will be more visible - then everything should become clear. The code sets attributes and date/time for the second time only if necessary - to avoid useless disk operations. It also handles the fact, that setting access rights makes an Archive attribute set.

Regards
Back to top
View user's profile Send private message Send e-mail
ghisler(Author)
Site Admin
Site Admin


Joined: 04 Feb 2003
Posts: 35473
Location: Switzerland

PostPosted: Mon May 21, 2018 1:20 am    Post subject: Reply with quote

Well, this would have other unwanted results, e.g. when the first change attribute call succeeds and the second fails, then the files will have that additional "archive" attribute.
_________________
Author of Total Commander
http://www.ghisler.com
Back to top
View user's profile Send private message Send e-mail Visit poster's website
MarcinW
Power Member
Power Member


Joined: 23 Jan 2012
Posts: 763
Location: Poland

PostPosted: Tue May 22, 2018 4:33 am    Post subject: Reply with quote

ghisler(Author) wrote:
when the first change attribute call succeeds and the second fails, then the files will have that additional "archive" attribute.

Unfortunately this is true. But it's a worst case failure of this algorithm. It seems to be better than the current implementation, when you can end up having a file with lacking Read Only, Hidden and/or System attributes...

After all, having an Archive attribute set (when not needed), may only lead to making a backup of the file (when not really needed).

By trying to set date/time and attributes twice, TC can do everything that is possible to do.

And, of course, TC can still inform the user, that the attributes have not been set properly...

Regards
Back to top
View user's profile Send private message Send e-mail
ghisler(Author)
Site Admin
Site Admin


Joined: 04 Feb 2003
Posts: 35473
Location: Switzerland

PostPosted: Tue May 22, 2018 8:29 am    Post subject: Reply with quote

I think a good compomise is calling SetFileAttributes before and after setting the permissions. This will allow to set them if at least one of the two permissions allows to set the attributes. Nothing changes for setting the time.
_________________
Author of Total Commander
http://www.ghisler.com
Back to top
View user's profile Send private message Send e-mail Visit poster's website
MarcinW
Power Member
Power Member


Joined: 23 Jan 2012
Posts: 763
Location: Poland

PostPosted: Tue May 22, 2018 5:06 pm    Post subject: Reply with quote

ghisler(Author) wrote:
Nothing changes for setting the time.


But setting time is similar to setting attributes - we must have access rights to open the file. So - if I didn't miss something - it may currently fail in a similar way as setting attributes?
Back to top
View user's profile Send private message Send e-mail
ghisler(Author)
Site Admin
Site Admin


Joined: 04 Feb 2003
Posts: 35473
Location: Switzerland

PostPosted: Wed May 23, 2018 9:01 am    Post subject: Reply with quote

Sorry, for now I will not change it. This way it will at least not be worse than in TC 9.12.
_________________
Author of Total Commander
http://www.ghisler.com
Back to top
View user's profile Send private message Send e-mail Visit poster's website
Display posts from previous:   
Post new topic   Reply to topic    Total Commander Forum Index -> TC9.1x bug reports (English) All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Imprint/Impressum: This site is maintained by Ghisler Software GmbH
Privacy Policy | Datenschutzerklärung | Politique de Confidentialité

Using phpBB © phpBB Group