Bug 15031

Summary: ANT <copy> with <fileset> does not spot bad symlinks
Product: Ant Reporter: JL <john.lonergan>
Component: CoreAssignee: Ant Notifications List <notifications>
Status: REOPENED ---    
Severity: normal CC: gus.heck
Priority: P3    
Version: 1.5   
Target Milestone: ---   
Hardware: Sun   
OS: other   

Description JL 2002-12-03 18:39:28 UTC
BELOW IS AN ANT CFG FILE THAT DESCRIBES THE PROBLEM

<!--
Ant 1.5 has a bug/limitation that can mask problems during make.
The <copy> operation does not spot or report files that are bad symlinks
it just ignores them.


to set up test case ....
>mkdir /tmp/src_with_some_bad_links
>cd /tmp/src_with_some_bad_links

>cat hello > realfile
>ln -s realfile goodlink
>ln -s nonexistantfile badlink

copy this file to /tmp and then run ant ...
>ant -buildfile /tmp/bad.xml

/tmp/target ends up with ...
  goodfile
  realfile

There is no mention of the fact that there is a bad symlink.
Ant does not report it and it does not return any indication there was a 
problem.

If however I use an explicit file name 
(eg <copy file="/tmp/src_with_some_bad_links/badlink" todir="/tmp/target"/>) 
then I do get an error (a nasty java error)

But this is not what I want to do - nor is it the sort of error message I 
expect (cos it looks like ant has dumped its pants).

-->




<project name="AntBug" default="default" basedir=".">

    <target name="default">

        <copy todir="/tmp/target">
            <fileset dir="/tmp/src_with_some_bad_links"/>
        </copy>

    </target>

</project>
Comment 1 Conor MacNeill 2003-07-23 13:21:23 UTC
I'm not sure what we can do here. Java does not support symlinks. The methods
java.io.File.getAbsolutePath() and getCanonicalPath() return the same value for
the broken link - so the trick of detecting links from the difference between
these two is not available. Also, according to java.io.File the file does not exist.

BTW, for me Ant did not drop its pants on the explicit copy of the broken link.
I get
/tmp/bad.xml:12: Warning: Could not find file
/tmp/src_with_some_bad_links/badlink to copy.

Seems reasonable behaviour given the limitations above.

I'm going to mark this as WONTFIX meaning CANTFIX. Sorry.
Comment 2 JL 2003-07-23 14:53:16 UTC
BUT YOU CANFIX !


You say "Also, according to java.io.File the file does not exist." - thats the 
key though isn't it!
Any file that exists in the directory but for which java.io.File.exists() 
return false is almost certainly a dead symlink.
We don't actually need to know its a symlink all we are trying to trap is the 
error where ant cannot copy the given file for some reason. I would argue that 
ant should also report an error for files that DO exist but however do not have 
read access.


The program below traps these cases easily.

ls -l /tmp/src_with_some_bad_links/

	lrwxrwxrwx   1 badlink -> nonexistantfile
	lrwxrwxrwx   1 goodlink -> realfile
	-rw-r--r--   1 realfile
	----------   1 unreadable

... we get ....

/tmp/src_with_some_bad_links: is a directory
/tmp/src_with_some_bad_links/realfile: exists and is readable
/tmp/src_with_some_bad_links/realfile: exists and is readable
/tmp/src_with_some_bad_links/badlink:exists in the parent directory, but does 
not actually exist - this is a dead symlink - ANT SHOULD REPORT THIS ERROR
/tmp/src_with_some_bad_links/unreadable: exists but is not readable ANT SHOULD 
REPORT THIS ERROR



Given this proof I would say it is entirely unreasonable that ant ignore such 
build errors.



==================================================
import java.io.File;

class test {
  public static  void main(String [] args ) {
    test("/tmp//src_with_some_bad_links");
  }

  static void test(String name) {
    test(new java.io.File(name));
  }

  static void test(File f) {

    try {
      String filename = f.getCanonicalPath();
      try {
        if (f.isDirectory()) {
          System.err.println(filename+": is a directory");
          File[] files = f.listFiles();

          for (int iFile=0; iFile < files.length; iFile++) {
            test(files[iFile]);
          }
        }
        else {

          if ( f.exists() ) {
            if ( f.canRead() ) System.err.println(filename+": exists and is 
readable");
            else System.err.println(filename+": exists but is not readable ANT 
SHOULD REPORT THIS ERROR");
          }
          else {
            System.err.println(filename+":exists in the parent directory, but 
does not actually exist - this is a dead symlink - ANT SHOULD REPORT THIS 
ERROR");
          }
        }
      }
      catch(Exception e) {
        System.err.println(filename + ":exception " + e.getMessage());
      }
    }
    catch(Exception e) {
      System.err.println(e.getMessage());
    }
  }
}


Comment 3 Gus Heck 2003-07-23 18:03:42 UTC
I think there is some possibility that JL may have a way to fix this, but more
consideration is needed. How does it react to things like named pipes and other
file system wierdos? Are these results true on all *nix's? What about across
different filesystems that may be mounted. And what about Cygwin?

JL, you might be interested to read bug 1550 for which I found a way to infer
the existance of Symlinks for the followsymlinks atribute. You will need to
answer the same types of questions before you can say that your method really
identifies broken symlinks only.

Even if it doesn't identify them specifically, it is possible that the other
things identified could be worth reporting with a warning, or even failing the
build.
Comment 4 JL 2003-07-23 22:48:38 UTC
Lets say the symlink issue is a red herring.

The underlying problem is that Ant is not detecting unreadable or nonexistant 
files consistently and so my builds are apparently suceeding where infact the 
is a problem in our package and fies are missing - this is bad however you look 
at it.

I have already demonstrated that ant can be tightened up so that it reports an 
error in the case where the directory says the file exists but io.File reports 
it as notexistant

Ant does correctly detect that it can't copy a regular file that is 
permissioned as unreadble but it doesn't detect the case I mention above.

Lets focus on detection of files that cannot be read (for whatever reason) and 
not let symlink detection sidetrack us.

Anyway looking at CopyDir.java I see that scanDir does this 
(srcFile.lastModified() > destFile.lastModified())) 

According to the JDK API docs ... "lastModified() Returns: ... 0L if the file 
does not exist or if an I/O error occurs". 
So for my dead link lastModified() is returning 0 (to indicate an error) and 
the lack of error detection code tricks the method into  thinking a copy is not 
required. 
To test this I set the overwrite="yes" attribute. Hey presto ant now dies when 
it encounters the dead sym link. 
So the code should actually be detecting the error value zero returned from 
lastModified() and in the case of the source file it should treat this as an 
error.

So right now ant behaves inconsistently. 
-If you explicitely tell ant to copy a dead link then it detects this and 
failed.
-If you are copying a dir and set overwrite='yes' then it detects dead links 
within dirs.
-If you are copying direct and -don-t  set overwrite="yes" then ant fails to 
detect the unreadable dead link.

So perhaps the focus of this discussion should be on making ant behave 
consistently with respect to unreadable and nonexistant files - ie abort.

Thx John
Comment 5 JL 2003-07-23 23:05:02 UTC
Surely we don't need to do much more than  call File.exists() and if it returns 
false then abort with an error ?

Doing this explicitely is much cleared than trying to infer a problem by 
looking out for zero returned from lastModified(). 

I still think the tag needs to error check the return value from lastModified() 
but I think the code should explicitely check File.exists() and File.readable()
Comment 6 Gus Heck 2003-07-30 15:43:34 UTC
Well, the situation you describe does seem to be somewhat inconsistant. Your
comments suggest that overwrite could be used as a workaround, but that is
admittedly inefficient at best, and in some cases might not be satisfactory. I'm
sold that there is something to be fixed, but not being a commiter I can't
promise anything. In your place, I would create a patch and attach it. Then we
can hope Connor or another commiter agrees, and commits your patch.
Comment 7 Stefan Bodewig 2005-03-14 11:28:52 UTC
Coming back to a really old report.

I completely agree that Ant is inconsistent here, even though it has come to behave
better in single-file copy here, it will report that the file doesn't exist (what
Conor notes below).

But this inconsistency goes further than just <copy>, we don't deal with those
unreadable/non-existant files in other tasks either.

As for the copy case, I'm afraid we are getting struck by backwards compatibility
again.  Touching the current beahvior would lead to breaking builds if we made
copy fail on bad symlinks.  All we could do was issuing a warning, IMHO.
Comment 8 johnlon 2005-08-09 14:45:58 UTC
Re
> Touching the current beahavior would lead to breaking builds if we made
> copy fail on bad symlinks.  All we could do was issuing a warning, IMHO.

Causing builds to break might be acceptable if you consider that those builds 
may actually be broken anyway, or at least on borrowed time, because of these 
unreported problems with the build. 

I think its far better to abort and force the user to fix the problem by way 
of removing the dead link or by coding round it in build.xml.



Comment 9 johnlon 2005-08-09 15:00:04 UTC
An earlier posting suggested that ...

    <copy overwrite="yes" 

is a workaround (though an obscure and v.poor one) in that forcing the copy 
causes the task to abort on dead symlinks.

However, since the original posting something may have changes in the Java 
code. This 'workaound' doesn't work - ie no abort happens.

So Ant still does not report this dead link problem and now there's no way to 
cause ant to fallover either.