You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ant.apache.org by ja...@apache.org on 2017/12/10 08:44:01 UTC
ant git commit: BZ-58683 Honour the overwrite=false for existing
symlinks. Plus, use Java 7 java.nio.file.Files support for symbolinks,
in symlink task
Repository: ant
Updated Branches:
refs/heads/master 0483b9fb2 -> cefdbd398
BZ-58683 Honour the overwrite=false for existing symlinks. Plus, use Java 7 java.nio.file.Files support for symbolinks, in symlink task
Project: http://git-wip-us.apache.org/repos/asf/ant/repo
Commit: http://git-wip-us.apache.org/repos/asf/ant/commit/cefdbd39
Tree: http://git-wip-us.apache.org/repos/asf/ant/tree/cefdbd39
Diff: http://git-wip-us.apache.org/repos/asf/ant/diff/cefdbd39
Branch: refs/heads/master
Commit: cefdbd398d8e310b218f9e2ca6f0b7fb13eddbb9
Parents: 0483b9f
Author: Jaikiran Pai <ja...@gmail.com>
Authored: Sat Dec 9 13:02:56 2017 +0530
Committer: Jaikiran Pai <ja...@gmail.com>
Committed: Sun Dec 10 14:12:25 2017 +0530
----------------------------------------------------------------------
WHATSNEW | 23 +-
manual/Tasks/symlink.html | 22 +-
.../taskdefs/optional/unix/symlink.xml | 14 +-
.../ant/taskdefs/optional/unix/Symlink.java | 251 ++++++++++---------
.../ant/taskdefs/optional/unix/SymlinkTest.java | 24 ++
5 files changed, 196 insertions(+), 138 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/ant/blob/cefdbd39/WHATSNEW
----------------------------------------------------------------------
diff --git a/WHATSNEW b/WHATSNEW
index ce969f4..52387d1 100644
--- a/WHATSNEW
+++ b/WHATSNEW
@@ -17,15 +17,20 @@ Fixed bugs:
* bootstrapping Ant on Windows failed
Bugzilla Report 61027
- * Bugzilla Reports 59648 and 43271 - Fixed the issue where the
- SCP based tasks would try to change the permissions on the
- parent directory of a transferred file, instead of changing it
- on the transferred file itself.
-
- * Bugzilla Report 60644 - Fixed the issue where the source file
- being copied could end up being corrupted if the target of the
- copy happened to be the same source file (symlinked back
- to itself).
+ * Fixed the issue where the SCP based tasks would try to change
+ the permissions on the parent directory of a transferred file,
+ instead of changing it on the transferred file itself.
+ Bugzilla Reports 59648 and 43271
+
+ * Fixed the issue where the source file being copied could end
+ up being corrupted if the target of the copy happened to be
+ the same source file (symlinked back to itself).
+ Bugzilla Report 60644
+
+ * Fixed the issue where symlink creation with "overwrite=false",
+ on existing symlink whose target was a directory, would end
+ up creating a new symlink under the target directory.
+ Bugzilla Report 58683
Other changes:
--------------
http://git-wip-us.apache.org/repos/asf/ant/blob/cefdbd39/manual/Tasks/symlink.html
----------------------------------------------------------------------
diff --git a/manual/Tasks/symlink.html b/manual/Tasks/symlink.html
index df42abd..331bd79 100644
--- a/manual/Tasks/symlink.html
+++ b/manual/Tasks/symlink.html
@@ -26,10 +26,10 @@
<h2><a name="symlink">Symlink</a></h2>
<h3>Description</h3>
-<p> Manages symbolic links on Unix based platforms. Can be used to
-make an individual link, delete a link, create multiple links from properties files,
-or create properties files describing links in the specified directories.
-Existing links are not overwritten by default.
+<p> Manages symbolic links on platforms where Java supports symbolic links.
+Can be used to make an individual link, delete a link, create multiple links
+from properties files, or create properties files describing links in the
+specified directories. Existing links are not overwritten by default.
<p><a href="../Types/fileset.html">FileSet</a>s are used to select a
set of links to record, or a set of property files to create links from. </p>
@@ -127,18 +127,8 @@ set of links to record, or a set of property files to create links from. </p>
variable to an absolute path, which will remove the /some/working/directory portion
of the above path and allow ant to find the correct commandline execution script.
- <p><strong>LIMITATIONS:</strong> Because Java has no direct support for
- handling symlinks this task divines them by comparing canonical and
- absolute paths. On non-unix systems this may cause false positives.
- Furthermore, any operating system on which the command
- <code>ln -s <linkname> <resourcename></code> is not a valid
- command on the command line will not be able to use action="single" or
- action="recreate". Action="record" and action=delete should still work. Finally,
- the lack of support for symlinks in Java means that all links are recorded as
- links to the <strong>canonical</strong> resource name. Therefore the link:
- <code>link --> subdir/dir/../foo.bar</code> will be recorded as
- <code>link=subdir/foo.bar</code> and restored as
- <code>link --> subdir/foo.bar</code></p>
+ <p><strong>NOTE:</strong> Starting Ant version 1.10.2, this task relies on the symbolic
+ link support introduced in Java 7 through the java.nio.file.Files APIs</p>
http://git-wip-us.apache.org/repos/asf/ant/blob/cefdbd39/src/etc/testcases/taskdefs/optional/unix/symlink.xml
----------------------------------------------------------------------
diff --git a/src/etc/testcases/taskdefs/optional/unix/symlink.xml b/src/etc/testcases/taskdefs/optional/unix/symlink.xml
index f039a62..134f29f 100644
--- a/src/etc/testcases/taskdefs/optional/unix/symlink.xml
+++ b/src/etc/testcases/taskdefs/optional/unix/symlink.xml
@@ -302,7 +302,7 @@
<sleep seconds="${delay}"/> <!-- make sure OS has time to do the execs -->
- <symlink action="recreate">
+ <symlink action="recreate" overwrite="true">
<fileset dir="${output}/symtest1" includes="**/recorded.links"/>
</symlink>
@@ -350,5 +350,17 @@
<delete file="${output}/file2"/>
</target>
+ <target name="test-overwrite-link" depends="setUp">
+ <mkdir dir="${output}/test-overwrite"/>
+ <mkdir dir="${output}/test-overwrite/dir1"/>
+ <property name="test.overwrite.link.target.dir" location="${output}/test-overwrite/dir1"/>
+ <!-- Create a symlink to the dir, this should work fine -->
+ <symlink link="${output}/test-overwrite/symlinked1" resource="${test.overwrite.link.target.dir}"/>
+ <!-- Create a symlink at the previously created symlink path with overwrite = false.
+ This *shouldn't* create a new link (within the target resource). See https://bz.apache.org/bugzilla/show_bug.cgi?id=58683 -->
+ <symlink link="${output}/test-overwrite/symlinked1" resource="${test.overwrite.link.target.dir}"/>
+
+
+ </target>
</project>
http://git-wip-us.apache.org/repos/asf/ant/blob/cefdbd39/src/main/org/apache/tools/ant/taskdefs/optional/unix/Symlink.java
----------------------------------------------------------------------
diff --git a/src/main/org/apache/tools/ant/taskdefs/optional/unix/Symlink.java b/src/main/org/apache/tools/ant/taskdefs/optional/unix/Symlink.java
index 7317c04..52d8968 100644
--- a/src/main/org/apache/tools/ant/taskdefs/optional/unix/Symlink.java
+++ b/src/main/org/apache/tools/ant/taskdefs/optional/unix/Symlink.java
@@ -36,9 +36,11 @@ import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.PrintStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
import java.util.ArrayList;
import java.util.HashMap;
-import java.nio.file.Files;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -51,11 +53,8 @@ import org.apache.tools.ant.DirectoryScanner;
import org.apache.tools.ant.Project;
import org.apache.tools.ant.dispatch.DispatchTask;
import org.apache.tools.ant.dispatch.DispatchUtils;
-import org.apache.tools.ant.taskdefs.Execute;
import org.apache.tools.ant.taskdefs.LogOutputStream;
import org.apache.tools.ant.types.FileSet;
-import org.apache.tools.ant.util.FileUtils;
-import org.apache.tools.ant.util.SymbolicLinkUtils;
/**
* Creates, Deletes, Records and Restores Symlinks.
@@ -100,24 +99,10 @@ import org.apache.tools.ant.util.SymbolicLinkUtils;
* <symlink action="delete" link="${dir.top}/foo"/>
* </pre>
*
- * <p><strong>LIMITATIONS:</strong> Because Java has no direct support for
- * handling symlinks this task divines them by comparing canonical and
- * absolute paths. On non-unix systems this may cause false positives.
- * Furthermore, any operating system on which the command
- * <code>ln -s link resource</code> is not a valid command on the command line
- * will not be able to use action="delete", action="single"
- * or action="recreate", but action="record" should still
- * work. Finally, the lack of support for symlinks in Java means that all links
- * are recorded as links to the <strong>canonical</strong> resource name.
- * Therefore the link: <code>link --> subdir/dir/../foo.bar</code> will be
- * recorded as <code>link=subdir/foo.bar</code> and restored as
- * <code>link --> subdir/foo.bar</code>.</p>
- *
+ * <p><strong>Note:</strong> Starting Ant version 1.10.2, this task relies on the symbolic link support
+ * introduced in Java 7 through the {@link Files} APIs</code>.
*/
public class Symlink extends DispatchTask {
- private static final FileUtils FILE_UTILS = FileUtils.getFileUtils();
- private static final SymbolicLinkUtils SYMLINK_UTILS =
- SymbolicLinkUtils.getSymbolicLinkUtils();
private String resource;
private String link;
@@ -187,12 +172,18 @@ public class Symlink extends DispatchTask {
handleError("Must define the link name for symlink!");
return;
}
+ final Path linkPath = Paths.get(link);
+ if (!Files.isSymbolicLink(linkPath)) {
+ log("Skipping deletion of " + linkPath + " since it's not a symlink", Project.MSG_VERBOSE);
+ // just ignore and silently return (this is consistent
+ // with the current, 1.9.x versions, of Ant)
+ return;
+
+ }
log("Removing symlink: " + link);
- SYMLINK_UTILS.deleteSymbolicLink(FILE_UTILS
- .resolveFile(new File("."), link),
- this);
+ deleteSymLink(linkPath);
} catch (IOException ioe) {
- handleError(ioe.toString());
+ handleError(ioe.getMessage());
} finally {
setDefaults();
}
@@ -207,26 +198,31 @@ public class Symlink extends DispatchTask {
try {
if (fileSets.isEmpty()) {
handleError(
- "File set identifying link file(s) required for action recreate");
+ "File set identifying link file(s) required for action recreate");
return;
}
- Properties links = loadLinks(fileSets);
-
- for (String lnk : links.stringPropertyNames()) {
- String res = links.getProperty(lnk);
- // handle the case where lnk points to a directory (bug 25181)
+ final Properties links = loadLinks(fileSets);
+ for (final String lnk : links.stringPropertyNames()) {
+ final String res = links.getProperty(lnk);
try {
- File test = new File(lnk);
- if (!SYMLINK_UTILS.isSymbolicLink(lnk)) {
- doLink(res, lnk);
- } else if (!test.getCanonicalPath().equals(
- new File(res).getCanonicalPath())) {
- SYMLINK_UTILS.deleteSymbolicLink(test, this);
- doLink(res, lnk);
- } // else lnk exists, do nothing
- } catch (IOException ioe) {
- handleError("IO exception while creating link");
+ if (Files.isSymbolicLink(Paths.get(lnk)) &&
+ new File(lnk).getCanonicalPath().equals(new File(res).getCanonicalPath())) {
+ // it's already a symlink and the symlink target is the same
+ // as the target noted in the properties file. So there's no
+ // need to recreate it
+ continue;
+ }
+ } catch (IOException e) {
+ final String errMessage = "Failed to check if path " + lnk + " is a symbolic link, linking to " + res;
+ if (failonerror) {
+ throw new BuildException(errMessage, e);
+ }
+ // log and continue
+ log(errMessage, Project.MSG_INFO);
+ continue;
}
+ // create the link
+ this.doLink(res, lnk);
}
} finally {
setDefaults();
@@ -361,56 +357,45 @@ public class Symlink extends DispatchTask {
/**
* Delete a symlink (without deleting the associated resource).
+ * <p>
+ * <p>This is a convenience method that simply invokes {@link #deleteSymlink(File)}
*
- * <p>This is a convenience method that simply invokes
- * <code>deleteSymlink(java.io.File)</code>.
- *
- * @param path A string containing the path of the symlink to delete.
+ * @param path A string containing the path of the symlink to delete.
+ * @throws IOException If the deletion attempt fails
*
- * @throws IOException If calls to <code>File.rename</code>
- * or <code>File.delete</code> fail.
- * @deprecated use
- * org.apache.tools.ant.util.SymbolicLinkUtils#deleteSymbolicLink
- * instead
+ * @deprecated use {@link Files#delete(Path)} instead
*/
@Deprecated
- public static void deleteSymlink(String path)
- throws IOException {
- SYMLINK_UTILS.deleteSymbolicLink(new File(path), null);
+ public static void deleteSymlink(final String path)
+ throws IOException {
+ deleteSymlink(Paths.get(path).toFile());
}
/**
* Delete a symlink (without deleting the associated resource).
- *
- * <p>This is a utility method that removes a unix symlink without removing
+ * <p>
+ * <p>This is a utility method that removes a symlink without removing
* the resource that the symlink points to. If it is accidentally invoked
- * on a real file, the real file will not be harmed.</p>
- *
- * <p>This method works by
- * getting the canonical path of the link, using the canonical path to
- * rename the resource (breaking the link) and then deleting the link.
- * The resource is then returned to its original name inside a finally
- * block to ensure that the resource is unharmed even in the event of
- * an exception.</p>
+ * on a real file, the real file will not be harmed and instead this method
+ * returns silently.</p>
+ * <p>
*
- * <p>Since Ant 1.8.0 this method will try to delete the File object if
- * it reports it wouldn't exist (as symlinks pointing nowhere usually do).
- * Prior version would throw a FileNotFoundException in that case.</p>
+ * <p>Since Ant 1.10.2 this method relies on the {@link Files#isSymbolicLink(Path)}
+ * and {@link Files#delete(Path)} to check and delete the symlink
+ * </p>
*
- * @param linkfil A <code>File</code> object of the symlink to delete.
+ * @param linkfil A <code>File</code> object of the symlink to delete. Cannot be null.
+ * @throws IOException If the attempt to delete runs into exception
*
- * @throws IOException If calls to <code>File.rename</code>,
- * <code>File.delete</code> or
- * <code>File.getCanonicalPath</code>
- * fail.
- * @deprecated use
- * org.apache.tools.ant.util.SymbolicLinkUtils#deleteSymbolicLink
- * instead
+ * @deprecated use {@link Files#delete(Path)} instead
*/
@Deprecated
- public static void deleteSymlink(File linkfil)
- throws IOException {
- SYMLINK_UTILS.deleteSymbolicLink(linkfil, null);
+ public static void deleteSymlink(final File linkfil)
+ throws IOException {
+ if (!Files.isSymbolicLink(linkfil.toPath())) {
+ return;
+ }
+ deleteSymLink(linkfil.toPath());
}
/**
@@ -448,36 +433,59 @@ public class Symlink extends DispatchTask {
/**
* Conduct the actual construction of a link.
*
- * <p>The link is constructed by calling <code>Execute.runCommand</code>.</p>
- *
- * @param res The path of the resource we are linking to.
- * @param lnk The name of the link we wish to make.
+ * @param res The path of the resource we are linking to.
+ * @param lnk The name of the link we wish to make.
* @throws BuildException when things go wrong
*/
private void doLink(String res, String lnk) throws BuildException {
- File linkfil = new File(lnk);
- String options = "-s";
- if (overwrite) {
- options += "f";
- if (linkfil.exists()) {
- try {
- SYMLINK_UTILS.deleteSymbolicLink(linkfil, this);
- } catch (FileNotFoundException fnfe) {
- log("Symlink disappeared before it was deleted: " + lnk);
- } catch (IOException ioe) {
- log("Unable to overwrite preexisting link or file: " + lnk,
- ioe, Project.MSG_INFO);
+ final Path link = Paths.get(lnk);
+ final Path target = Paths.get(res);
+ final boolean alreadyExists = Files.exists(link);
+ if (!alreadyExists) {
+ // if the path (at which the link is expected to be created) isn't already present
+ // then we just go ahead and attempt to symlink
+ try {
+ Files.createSymbolicLink(link, target);
+ } catch (IOException e) {
+ if (failonerror) {
+ throw new BuildException("Failed to create symlink " + lnk + " to target " + res, e);
}
+ log("Unable to create symlink " + lnk + " to target " + res, e, Project.MSG_INFO);
+ }
+ return;
+ }
+ // file already exists, see if we are allowed to overwrite
+ if (!overwrite) {
+ log("Skipping symlink creation, since file at " + lnk + " already exists and overwrite is set to false", Project.MSG_INFO);
+ return;
+ }
+ // we have been asked to overwrite, so we now do the necessary steps
+
+ // initiate a deletion *only* if the path is a symlink, else we fail with error
+ if (!Files.isSymbolicLink(link)) {
+ final String errMessage = "Cannot overwrite " + lnk + " since the path already exists and isn't a symlink";
+ if (failonerror) {
+ throw new BuildException(errMessage);
}
+ // log and return
+ log(errMessage, Project.MSG_INFO);
+ return;
}
+ // it's a symlink, so we delete the *link* first
try {
- Execute.runCommand(this, "ln", options, res, lnk);
- } catch (BuildException failedToExecute) {
+ deleteSymLink(link);
+ } catch (IOException e) {
+ // our deletion attempt failed, just log it and try to create the symlink
+ // anyway, since we have been asked to overwrite
+ log("Failed to delete existing symlink at " + lnk, e, Project.MSG_DEBUG);
+ }
+ try {
+ Files.createSymbolicLink(link, target);
+ } catch (IOException e) {
if (failonerror) {
- throw failedToExecute;
+ throw new BuildException("Failed to create symlink " + lnk + " to target " + res, e);
}
- //log at the info level, and keep going.
- log(failedToExecute.getMessage(), failedToExecute, Project.MSG_INFO);
+ log("Unable to create symlink " + lnk + " to target " + res, e, Project.MSG_INFO);
}
}
@@ -488,30 +496,33 @@ public class Symlink extends DispatchTask {
* "record". This means that filesets are interpreted
* as the directories in which links may be found.</p>
*
- * @param fileSets The filesets specified by the user.
- * @return A HashSet of <code>File</code> objects containing the
- * links (with canonical parent directories).
+ * @param fileSets The filesets specified by the user.
+ * @return A Set of <code>File</code> objects containing the
+ * links (with canonical parent directories).
*/
private Set<File> findLinks(List<FileSet> fileSets) {
- Set<File> result = new HashSet<>();
+ final Set<File> result = new HashSet<>();
for (FileSet fs : fileSets) {
DirectoryScanner ds = fs.getDirectoryScanner(getProject());
File dir = fs.getDir(getProject());
Stream.of(ds.getIncludedFiles(), ds.getIncludedDirectories())
- .flatMap(Stream::of).forEach(path -> {
- try {
- File f = new File(dir, path);
- File pf = f.getParentFile();
- String name = f.getName();
- if (SYMLINK_UTILS.isSymbolicLink(pf, name)) {
- result.add(new File(pf.getCanonicalFile(), name));
+ .flatMap(Stream::of).forEach(path -> {
+ try {
+ final File f = new File(dir, path);
+ final File pf = f.getParentFile();
+ final String name = f.getName();
+ // we use the canonical path of the parent dir in which the (potential)
+ // link resides
+ final File parentDirCanonicalizedFile = new File(pf.getCanonicalPath(), name);
+ if (Files.isSymbolicLink(parentDirCanonicalizedFile.toPath())) {
+ result.add(parentDirCanonicalizedFile);
+ }
+ } catch (IOException e) {
+ handleError("IOException: " + path + " omitted");
}
- } catch (IOException e) {
- handleError("IOException: " + path + " omitted");
- }
- });
+ });
}
return result;
}
@@ -568,4 +579,20 @@ public class Symlink extends DispatchTask {
}
return finalList;
}
+
+ private static void deleteSymLink(final Path path) throws IOException {
+ // Implementation note: We intentionally use java.io.File#delete() instead of
+ // java.nio.file.Files#delete(Path) since it turns out that the latter doesn't
+ // update/clear the "canonical file paths cache" maintained by the JRE FileSystemProvider.
+ // Not clearing/updating that cache results in this deleted (and later recreated) symlink
+ // to point to a wrong/outdated target for a few seconds (30 seconds is the time the JRE
+ // maintains the cache entries for). All this is implementation detail of the JRE and
+ // probably a bug, but given that it affects our tests (SymlinkTest#testRecreate consistently fails
+ // on MacOS/Unix) as well as the Symlink task, it makes sense to use this API instead of
+ // the Files#delete(Path) API
+ final boolean deleted = path.toFile().delete();
+ if (!deleted) {
+ throw new IOException("Could not delete symlink at " + path);
+ }
+ }
}
http://git-wip-us.apache.org/repos/asf/ant/blob/cefdbd39/src/tests/junit/org/apache/tools/ant/taskdefs/optional/unix/SymlinkTest.java
----------------------------------------------------------------------
diff --git a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/unix/SymlinkTest.java b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/unix/SymlinkTest.java
index b5f4bbf..f85bb14 100644
--- a/src/tests/junit/org/apache/tools/ant/taskdefs/optional/unix/SymlinkTest.java
+++ b/src/tests/junit/org/apache/tools/ant/taskdefs/optional/unix/SymlinkTest.java
@@ -35,6 +35,7 @@ import org.apache.tools.ant.taskdefs.condition.Os;
import org.apache.tools.ant.Project;
import org.apache.tools.ant.util.SymbolicLinkUtils;
import org.junit.After;
+import org.junit.Assert;
import org.junit.Assume;
import org.junit.Before;
import org.junit.Rule;
@@ -46,6 +47,9 @@ import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
/**
* Test cases for the Symlink task. Link creation, link deletion, recording
@@ -287,6 +291,26 @@ public class SymlinkTest {
}
+ /**
+ * Tests that when {@code symlink} task is used to create a symbolic link and {@code overwrite} option
+ * is {@code false}, then any existing symbolic link at the {@code link} location (whose target is a directory)
+ * doesn't end up create a new symbolic link within the target directory.
+ *
+ *
+ * @throws Exception
+ * @see <a href="https://bz.apache.org/bugzilla/show_bug.cgi?id=58683">BZ-58683</a> for more details
+ */
+ @Test
+ public void testOverwriteExistingLink() throws Exception {
+ buildRule.executeTarget("test-overwrite-link");
+ final Project p = buildRule.getProject();
+ final String linkTargetResource = p.getProperty("test.overwrite.link.target.dir");
+ Assert.assertNotNull("Property test.overwrite.link.target.dir is not set", linkTargetResource);
+ final Path targetResourcePath = Paths.get(linkTargetResource);
+ Assert.assertTrue(targetResourcePath + " is not a directory", Files.isDirectory(targetResourcePath));
+ Assert.assertEquals(targetResourcePath + " directory was expected to be empty", 0, Files.list(targetResourcePath).count());
+ }
+
@After
public void tearDown() {
if (buildRule.getProject() != null) {