Bug 34941

Summary: FTP uptodate calculations wrong
Product: Ant Reporter: Steve Cohen <scohen>
Component: Optional TasksAssignee: Ant Notifications List <notifications>
Status: RESOLVED FIXED    
Severity: normal CC: stevel
Priority: P2    
Version: 1.6.3   
Target Milestone: 1.6.4   
Hardware: All   
OS: other   

Description Steve Cohen 2005-05-17 02:12:09 UTC
Given the following task call

<ftp action = "get" 
       ...
       preservelastmodified="true"
       newer="true">
   <fileset.../>
</ftp>

If this is executed a second time, all files included in the fileset will be
retrieved regardless of whether or not they have changed on the server in the
intervening time interval.

This is because the underlying uptodate logic is returning false if the source
and destination timestamps are equal.  Logically, it should be true.  Logically,
any file where the local file's timestamp is greater than or equal to the remote
file's timestamp is up to date.  Any file whose local timestamp is less than its
remote file's timestamp is not.  But that's not the current implementation.

A similar issue exists on put.

The fix is easy to implement and I will do it, but I want to give the community
a chance to comment before implementing.  There may be a use case for the
existing functionality that I am not understanding, or there may be an important
backward compatibility issue that I am missing.
Comment 1 Steve Loughran 2005-05-19 01:13:32 UTC
Is this also the cause of a problem I'm seeing, namely that ftp put of a set of
files to a unix localhost is always updating, even when they havent changed?
Everything works properly when uploading to a local DOS box?

source: 
core/dist> ls -al *.sha1
-rw-r--r--  1 slo users 41 2005-05-18 23:45 diary-core-0.1alpha-src.tar.gz.sha1
-rw-r--r--  1 slo users 41 2005-05-18 23:45 diary-core-0.1alpha-src.zip.sha1
-rw-r--r--  1 slo users 41 2005-05-18 23:45 diary-core-0.1alpha.tar.gz.sha1
-rw-r--r--  1 slo users 41 2005-05-18 23:45 diary-core-0.1alpha.zip.sha1

dest:
temp/upload> ls -al *.sha1
-rw-r--r--  1 slo users 41 2005-05-19 00:07 diary-core-0.1alpha-src.tar.gz.sha1
-rw-r--r--  1 slo users 41 2005-05-19 00:07 diary-core-0.1alpha-src.zip.sha1
-rw-r--r--  1 slo users 41 2005-05-19 00:07 diary-core-0.1alpha.tar.gz.sha1
-rw-r--r--  1 slo users 41 2005-05-19 00:07 diary-core-0.1alpha.zip.sha1

      [ftp] Opening FTP connection to k2
      [ftp] connected
      [ftp] logging in to FTP server
      [ftp] login succeeded
      [ftp] changing the remote directory
      [ftp] sending files
      [ftp] checking date for diary-core-0.1alpha-src.tar.gz
      [ftp] transferring
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha-src.tar.gz
      [ftp] File
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha-src.tar.gz
copied to k2
      [ftp] checking date for diary-core-0.1alpha-src.tar.gz.sha1
      [ftp] transferring
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha-src.tar.gz.sha1
      [ftp] File
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha-src.tar.gz.sha1
copied to k2
      [ftp] checking date for diary-core-0.1alpha-src.zip
      [ftp] transferring
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha-src.zip
      [ftp] File
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha-src.zip
copied to k2
      [ftp] checking date for diary-core-0.1alpha-src.zip.sha1
      [ftp] transferring
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha-src.zip.sha1
      [ftp] File
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha-src.zip.sha1
copied to k2
      [ftp] checking date for diary-core-0.1alpha.tar.gz
      [ftp] transferring
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha.tar.gz
      [ftp] File
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha.tar.gz
copied to k2
      [ftp] checking date for diary-core-0.1alpha.tar.gz.sha1
      [ftp] transferring
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha.tar.gz.sha1
      [ftp] File
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha.tar.gz.sha1
copied to k2
      [ftp] checking date for diary-core-0.1alpha.zip
      [ftp] transferring
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha.zip
      [ftp] File
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha.zip copied to k2
      [ftp] checking date for diary-core-0.1alpha.zip.sha1
      [ftp] transferring
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha.zip.sha1
      [ftp] File
/home/slo/Projects/AntBook/app2/diary/core/dist/diary-core-0.1alpha.zip.sha1
copied to k2
      [ftp] 8 files sent
      [ftp] disconnecting



Comment 2 Steve Cohen 2005-05-19 02:51:15 UTC
It would seem you problem would not be fixed by the change I want to make.  That
change is simply to change this:

       if (this.action == SEND_FILES) {
           return remoteTimestamp + timeDiffMillis > localTimestamp;
       } else {
           return localTimestamp > remoteTimestamp + timeDiffMillis;
       }

to this:

       if (this.action == SEND_FILES) {
           return remoteTimestamp + timeDiffMillis >= localTimestamp;
       } else {
           return localTimestamp >= remoteTimestamp + timeDiffMillis;
       }

It would fix the problem of equal timestamps updating, but not the problem I see
here which is where the source is older than the destination but still updates.

However, to truly understand what is going on, can you supply the actual ant
target where this is happening with all the <ftp> attributes you are using?
Comment 3 Steve Cohen 2005-05-19 03:04:22 UTC
Wait a minute, Steve.  Now I REALLY want to see your <ftp> call parameters.  Are
you using the new defaultDateFormatConfig attribute?  Does an FTP listing from
these directories look exactly like these ls listings?  

Because, if it does, we are looking at one of these newfangled UNIX ftp servers
with all-numeric date formats.  This is something new, something that
commons-net 1.4.0 addresses but 1.2.2 does not.  1.2.2 (and the default
configuration in 1.4.0) assumed that all unix ftp servers listings looked like this:

-rw-r--r--  1 slo users 41 May 18 23:45 diary-core-0.1alpha-src.tar.gz.sha1

You must use the defaultDateFormatConfig="yyyy-MM-dd HH:mm" attribute to handle
the numeric format you are showing if that's the way your FTP server does it. 
Actually, this makes a lot of sense.  If your server FTP listings look like that
the default parser will fail to parse them, so your destination will show NO
files existing and hence everything will get copied.  
Comment 4 Steve Cohen 2005-05-19 10:02:20 UTC
Change outlined above was made on HEAD.  Files with equal timestamps are now
considered up to date.
Comment 5 Steve Loughran 2005-05-19 10:55:39 UTC
Server is Suse 9.2pro with vsftpd; It's at home (and turned off) so I cant get a
listing now. The target is 

  <ftp server="${unix.ftp.server}" 
    userid="${unix.ftp.user}"
    password="${unix.ftp.password}"
    verbose="true"
    action="put"
    depends="true"
    systemTypeKey="UNIX"
    preservelastmodified="true"
    remotedir="${unix.ftp.dir}">
      <fileset refid="ftp.upload.fileset"/>
    </ftp>
Comment 6 Steve Cohen 2005-05-20 01:39:03 UTC
Also marking version to 1.6.4 
All right, Steve.  I would say that you have a bug.  The problem is NOT

1) the same as this bug  (this bug concerns the case of equal timestamps, not
cases where an older file is copied over a newer one, which is what you have).

2) caused by an all-numeric date format on the new server, which would have been
a problem that 1.4.0 could have fixed, with the right parameters.

The only thing that's the least bit fishy in your config is that 
preservelastmodified="true"  .  You might want to try it without that since the
docs say it only works for gets.  But it shouldn't cause puts to break.  That
would still be a bug.

So I would recommend logging this as a new bug.

I've also marked it as target milestone 1.6.4 since I put it in their at Antoine
's urging.

Comment 7 Steve Cohen 2005-05-22 00:15:35 UTC
I previously stated that I think that your problem results from a new bug.  I no
longer think so.  It's not a bug.

First of all the preserveLastModified is documented as only apply to gets, not
puts, as I previously stated, but that is irrelevant.

More importantly, you must consider timestamp resolution differences between
your local machine and the ftp server.  Even if they are on the same box,
commons-net-ftp, since it must work through directory listing formats, is
restricted to what is displayed by an FTP listing, typically, HH:mm.  Whereas
your local file system more than likely provides second resolution.  Therefore,
even if serverTimeZoneConfig is specified, the timediffmillis attribute must
also be specified, not to account for the time zone differences but to account
for the granularity difference.  A value of 60000 must be set here.

I'm not sure I'm happy with this situation, but with the fix I've checked in the
task is now performing as currently documented.  The timediffmillis attribute is
becoming more of a messy "slush fund" as it handles both time zone differences
and timestamp granularity issues.  Using time zones is much better than using
timediffmillis to handle this (since you can let java "do the math", and more
importantly, because it understands daylight savings time, which timediffmillis
does not.)  The only good use left for timediffmillis is to handle granularity
issues.  I think we should think about deprecating timediffmillis in favor of an
attribute that takes some predefined granularity constants such as
"MINUTE_GRANULARITY", etc. which the task can translate into milliseconds.