You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ant.apache.org by gi...@apache.org on 2018/05/23 04:39:49 UTC

[2/2] ant git commit: Bz 22370: followlinks attribute

Bz 22370: followlinks attribute

Project: http://git-wip-us.apache.org/repos/asf/ant/repo
Commit: http://git-wip-us.apache.org/repos/asf/ant/commit/35a84fea
Tree: http://git-wip-us.apache.org/repos/asf/ant/tree/35a84fea
Diff: http://git-wip-us.apache.org/repos/asf/ant/diff/35a84fea

Branch: refs/heads/master
Commit: 35a84fea1a9f73e54de0207a04c12f764ac8821c
Parents: c9e0c50
Author: Gintas Grigelionis <gi...@apache.org>
Authored: Wed May 23 06:38:49 2018 +0200
Committer: Gintas Grigelionis <gi...@apache.org>
Committed: Wed May 23 06:38:49 2018 +0200

----------------------------------------------------------------------
 manual/Types/selectors.html                     | 57 +++++++++++------
 .../ant/types/selectors/OwnedBySelector.java    | 17 ++++-
 .../ant/types/selectors/PosixGroupSelector.java | 14 +++-
 .../selectors/PosixPermissionsSelector.java     | 16 ++++-
 .../types/selectors/OwnedBySelectorTest.java    | 45 +++++++++++--
 .../types/selectors/PosixGroupSelectorTest.java | 33 +++++++++-
 .../selectors/PosixPermissionsSelectorTest.java | 67 ++++++++++++++++++--
 7 files changed, 209 insertions(+), 40 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/ant/blob/35a84fea/manual/Types/selectors.html
----------------------------------------------------------------------
diff --git a/manual/Types/selectors.html b/manual/Types/selectors.html
index 955a1b2..e3289af 100644
--- a/manual/Types/selectors.html
+++ b/manual/Types/selectors.html
@@ -86,7 +86,7 @@
         by a given user.</li>
       <li><a href="#posixGroup"><code>&lt;posixGroup&gt;</code>&mdash;Select
         files if they have a given POSIX group.</li>
-      <li><a href="#posixPermissions"><code>&lt;posixPermissions&gt;</code>&mdash;Select
+      <li><a href="#posixPermissions"><code>&lt;posixPermissions&gt;</code></a>&mdash;Select
         files if they have given POSIX permissions.</li>
     </ul>
 
@@ -925,6 +925,11 @@
         <td>Username of the expected owner</td>
         <td>Yes</td>
       </tr>
+        <tr>
+          <td>followlinks</td>
+          <td>Must the selector follow symbolic links?</td>
+          <td>No; defaults to <q>false</q> (was <q>true</q> before Ant 1.10.4)</td>
+        </tr>
     </table>
 
     <h4 id="posixGroup">PosixGroup Selector</h4>
@@ -937,16 +942,21 @@
     <p><em>Since Ant 1.10.4</em></p>
 
     <table class="attr">
-        <tr>
-            <th scope="col">Attribute</th>
-            <th scope="col">Description</th>
-            <th scope="col">Required</th>
-        </tr>
-        <tr>
-            <td>group</td>
-            <td>POSIX group name</td>
-            <td>Yes</td>
-        </tr>
+      <tr>
+        <th scope="col">Attribute</th>
+        <th scope="col">Description</th>
+        <th scope="col">Required</th>
+      </tr>
+      <tr>
+        <td>group</td>
+        <td>POSIX group name</td>
+        <td>Yes</td>
+      </tr>
+      <tr>
+        <td>followlinks</td>
+        <td>Must the selector follow symbolic links?</td>
+        <td>No; defaults to <q>false</q></td>
+      </tr>
     </table>
 
     <h4 id="posixPermissions">PosixPermissions Selector</h4>
@@ -959,16 +969,21 @@
     <p><em>Since Ant 1.10.4</em></p>
 
     <table class="attr">
-        <tr>
-            <th scope="col">Attribute</th>
-            <th scope="col">Description</th>
-            <th scope="col">Required</th>
-        </tr>
-        <tr>
-            <td>permissions</td>
-            <td>POSIX permissions in string (<q>rwxrwxrwx</q>) or octal (<q>777</q>) format</td>
-            <td>Yes</td>
-        </tr>
+      <tr>
+        <th scope="col">Attribute</th>
+        <th scope="col">Description</th>
+        <th scope="col">Required</th>
+      </tr>
+      <tr>
+        <td>permissions</td>
+        <td>POSIX permissions in string (<q>rwxrwxrwx</q>) or octal (<q>777</q>) format</td>
+        <td>Yes</td>
+      </tr>
+      <tr>
+        <td>followlinks</td>
+        <td>Must the selector follow symbolic links?</td>
+        <td>No; defaults to <q>false</q></td>
+      </tr>
     </table>
 
     <h4 id="scriptselector">Script Selector</h4>

http://git-wip-us.apache.org/repos/asf/ant/blob/35a84fea/src/main/org/apache/tools/ant/types/selectors/OwnedBySelector.java
----------------------------------------------------------------------
diff --git a/src/main/org/apache/tools/ant/types/selectors/OwnedBySelector.java b/src/main/org/apache/tools/ant/types/selectors/OwnedBySelector.java
index d37fd2f..cb21de4 100644
--- a/src/main/org/apache/tools/ant/types/selectors/OwnedBySelector.java
+++ b/src/main/org/apache/tools/ant/types/selectors/OwnedBySelector.java
@@ -21,9 +21,11 @@ package org.apache.tools.ant.types.selectors;
 import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
+import java.nio.file.LinkOption;
 import java.nio.file.attribute.UserPrincipal;
 
 import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.PropertyHelper;
 
 /**
  * A selector that selects files based on their owner.
@@ -40,14 +42,24 @@ public class OwnedBySelector implements FileSelector {
 
     private String owner;
 
+    private boolean followLinks = false;
+
     /**
-     * Sets the User-Name to look for.
+     * Sets the user name to look for.
      * @param owner the user name
      */
     public void setOwner(String owner) {
         this.owner = owner;
     }
 
+    /**
+     * Sets the "follow links" flag.
+     * @param followLinks the user name
+     */
+    public void setFollowLinks(String followLinks) {
+        this.followLinks = PropertyHelper.toBoolean(followLinks);
+    }
+
     @Override
     public boolean isSelected(File basedir, String filename, File file) {
         if (owner == null) {
@@ -55,7 +67,8 @@ public class OwnedBySelector implements FileSelector {
         }
         if (file != null) {
             try {
-                UserPrincipal user = Files.getOwner(file.toPath());
+                UserPrincipal user = followLinks ? Files.getOwner(file.toPath())
+                        : Files.getOwner(file.toPath(), LinkOption.NOFOLLOW_LINKS);
                 return user != null && owner.equals(user.getName());
             } catch (UnsupportedOperationException | IOException ex) {
                 // => not the expected owner

http://git-wip-us.apache.org/repos/asf/ant/blob/35a84fea/src/main/org/apache/tools/ant/types/selectors/PosixGroupSelector.java
----------------------------------------------------------------------
diff --git a/src/main/org/apache/tools/ant/types/selectors/PosixGroupSelector.java b/src/main/org/apache/tools/ant/types/selectors/PosixGroupSelector.java
index 9c74985..a72d0fd 100644
--- a/src/main/org/apache/tools/ant/types/selectors/PosixGroupSelector.java
+++ b/src/main/org/apache/tools/ant/types/selectors/PosixGroupSelector.java
@@ -19,6 +19,7 @@
 package org.apache.tools.ant.types.selectors;
 
 import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.PropertyHelper;
 
 import java.io.File;
 import java.io.IOException;
@@ -41,6 +42,8 @@ public class PosixGroupSelector implements FileSelector {
 
     private String group;
 
+    private boolean followLinks = false;
+
     /**
      * Sets the group name to look for.
      * @param group the group name
@@ -49,13 +52,22 @@ public class PosixGroupSelector implements FileSelector {
         this.group = group;
     }
 
+    /**
+     * Sets the "follow links" flag.
+     * @param followLinks the user name
+     */
+    public void setFollowLinks(String followLinks) {
+        this.followLinks = PropertyHelper.toBoolean(followLinks);
+    }
+
     @Override
     public boolean isSelected(File basedir, String filename, File file) {
         if (group == null) {
             throw new BuildException("the group attribute is required");
         }
         try {
-            GroupPrincipal actualGroup = Files.readAttributes(file.toPath(),
+            GroupPrincipal actualGroup = followLinks ? Files.readAttributes(file.toPath(),
+                    PosixFileAttributes.class).group() : Files.readAttributes(file.toPath(),
                     PosixFileAttributes.class, LinkOption.NOFOLLOW_LINKS).group();
             return actualGroup != null && actualGroup.getName().equals(group);
         } catch (IOException e) {

http://git-wip-us.apache.org/repos/asf/ant/blob/35a84fea/src/main/org/apache/tools/ant/types/selectors/PosixPermissionsSelector.java
----------------------------------------------------------------------
diff --git a/src/main/org/apache/tools/ant/types/selectors/PosixPermissionsSelector.java b/src/main/org/apache/tools/ant/types/selectors/PosixPermissionsSelector.java
index 8cd60ab..fd6cef7 100644
--- a/src/main/org/apache/tools/ant/types/selectors/PosixPermissionsSelector.java
+++ b/src/main/org/apache/tools/ant/types/selectors/PosixPermissionsSelector.java
@@ -19,6 +19,7 @@
 package org.apache.tools.ant.types.selectors;
 
 import org.apache.tools.ant.BuildException;
+import org.apache.tools.ant.PropertyHelper;
 import org.apache.tools.ant.util.PermissionUtils;
 
 import java.io.File;
@@ -40,6 +41,8 @@ public class PosixPermissionsSelector implements FileSelector {
 
     private String permissions;
 
+    private boolean followLinks = false;
+
     /**
      * Sets the permissions to look for.
      * @param permissions the permissions string (rwxrwxrwx or octal)
@@ -59,14 +62,23 @@ public class PosixPermissionsSelector implements FileSelector {
         }
     }
 
+    /**
+     * Sets the "follow links" flag.
+     * @param followLinks the user name
+     */
+    public void setFollowLinks(String followLinks) {
+        this.followLinks = PropertyHelper.toBoolean(followLinks);
+    }
+
     @Override
     public boolean isSelected(File basedir, String filename, File file) {
         if (permissions == null) {
             throw new BuildException("the permissions attribute is required");
         }
         try {
-            return PosixFilePermissions.toString(
-                    Files.getPosixFilePermissions(file.toPath(), LinkOption.NOFOLLOW_LINKS))
+            return PosixFilePermissions.toString(followLinks
+                    ? Files.getPosixFilePermissions(file.toPath())
+                    : Files.getPosixFilePermissions(file.toPath(), LinkOption.NOFOLLOW_LINKS))
                     .equals(permissions);
         } catch (IOException e) {
             // => not the expected permissions

http://git-wip-us.apache.org/repos/asf/ant/blob/35a84fea/src/tests/junit/org/apache/tools/ant/types/selectors/OwnedBySelectorTest.java
----------------------------------------------------------------------
diff --git a/src/tests/junit/org/apache/tools/ant/types/selectors/OwnedBySelectorTest.java b/src/tests/junit/org/apache/tools/ant/types/selectors/OwnedBySelectorTest.java
index d73626f..e8619e4 100644
--- a/src/tests/junit/org/apache/tools/ant/types/selectors/OwnedBySelectorTest.java
+++ b/src/tests/junit/org/apache/tools/ant/types/selectors/OwnedBySelectorTest.java
@@ -19,14 +19,19 @@
 package org.apache.tools.ant.types.selectors;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeFalse;
 
 import java.io.File;
+import java.io.IOException;
 import java.nio.file.Files;
+import java.nio.file.LinkOption;
+import java.nio.file.Path;
 import java.nio.file.attribute.UserPrincipal;
 
 import org.apache.tools.ant.taskdefs.condition.Os;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -36,18 +41,46 @@ public class OwnedBySelectorTest {
     @Rule
     public TemporaryFolder folder = new TemporaryFolder();
 
-    @Test
-    public void ownedByIsTrueForSelf() throws Exception {
+    private final File TEST_FILE = new File("/etc/passwd");
+
+    private final String SELF = System.getProperty("user.name");
+
+    private final String ROOT = "root";
+
+    private OwnedBySelector s;
+
+    @Before
+    public void setUp() {
         // at least on Jenkins the file is owned by "BUILTIN\Administrators"
         assumeFalse(Os.isFamily("windows"));
-        String self = System.getProperty("user.name");
+
+        s = new OwnedBySelector();
+    }
+
+    @Test
+    public void ownedByIsTrueForSelf() throws Exception {
         File file = folder.newFile("f.txt");
         UserPrincipal user = Files.getOwner(file.toPath());
-        assertEquals(self, user.getName());
+        assertEquals(SELF, user.getName());
 
-        OwnedBySelector s = new OwnedBySelector();
-        s.setOwner(self);
+        s.setOwner(SELF);
         assertTrue(s.isSelected(null, null, file));
     }
 
+    @Test
+    public void ownedByFollowSymlinks() throws IOException {
+        File target = new File(folder.getRoot(), "link");
+        Path symbolicLink = Files.createSymbolicLink(target.toPath(), TEST_FILE.toPath());
+
+        UserPrincipal root = Files.getOwner(symbolicLink);
+        assertEquals(ROOT, root.getName());
+
+        UserPrincipal user = Files.getOwner(symbolicLink, LinkOption.NOFOLLOW_LINKS);
+        assertEquals(SELF, user.getName());
+
+        s.setOwner(SELF);
+        assertTrue(s.isSelected(null, null, symbolicLink.toFile()));
+        s.setFollowLinks("yes");
+        assertFalse(s.isSelected(null, null, symbolicLink.toFile()));
+    }
 }

http://git-wip-us.apache.org/repos/asf/ant/blob/35a84fea/src/tests/junit/org/apache/tools/ant/types/selectors/PosixGroupSelectorTest.java
----------------------------------------------------------------------
diff --git a/src/tests/junit/org/apache/tools/ant/types/selectors/PosixGroupSelectorTest.java b/src/tests/junit/org/apache/tools/ant/types/selectors/PosixGroupSelectorTest.java
index 8d4e28f..2cb0a21 100644
--- a/src/tests/junit/org/apache/tools/ant/types/selectors/PosixGroupSelectorTest.java
+++ b/src/tests/junit/org/apache/tools/ant/types/selectors/PosixGroupSelectorTest.java
@@ -9,10 +9,14 @@ import org.junit.rules.TemporaryFolder;
 import java.io.File;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
+import java.nio.file.Path;
 import java.nio.file.attribute.GroupPrincipal;
+import java.nio.file.attribute.PosixFileAttributes;
 import java.util.Map;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeNoException;
 import static org.junit.Assume.assumeTrue;
@@ -24,13 +28,15 @@ public class PosixGroupSelectorTest {
 
     private final String GROUP_GETTER = "getGid";
 
+    private final File TEST_FILE = new File("/etc/passwd");
+
     private Class<?> jaasProviderClass;
 
     private PosixGroupSelector s;
 
     @Before
     public void setUp() {
-        assumeTrue(Os.isFamily("unix"));
+        assumeTrue("Not POSIX", Os.isFamily("unix"));
         String osName = System.getProperty("os.name", "unknown").toLowerCase();
         String jaasProviderClassName = osName.contains("sunos")
            ? "com.sun.security.auth.module.SolarisSystem"
@@ -46,7 +52,7 @@ public class PosixGroupSelectorTest {
     }
 
     @Test
-    public void PosixGroupIsTrueForSelf() throws Exception {
+    public void posixGroupIsTrueForSelf() throws Exception {
         long gid = (long) jaasProviderClass.getMethod(GROUP_GETTER)
                 .invoke(jaasProviderClass.newInstance());
 
@@ -61,4 +67,27 @@ public class PosixGroupSelectorTest {
         assertTrue(s.isSelected(null, null, file));
     }
 
+    @Test
+    public void posixGroupFollowSymlinks() throws Exception {
+        long gid = (long) jaasProviderClass.getMethod(GROUP_GETTER)
+                .invoke(jaasProviderClass.newInstance());
+
+        File target = new File(folder.getRoot(), "link");
+        Path symbolicLink = Files.createSymbolicLink(target.toPath(), TEST_FILE.toPath());
+        Map<String, Object> linkAttributes = Files.readAttributes(target.toPath(),
+                "unix:group,gid", LinkOption.NOFOLLOW_LINKS);
+        long linkGid = (int) linkAttributes.get("gid");
+        assertEquals("Different GIDs", gid, linkGid);
+
+        GroupPrincipal targetGroup = Files.readAttributes(target.toPath(),
+                PosixFileAttributes.class).group();
+        GroupPrincipal linkGroup = (GroupPrincipal) linkAttributes.get("group");
+        assertNotEquals("Same group name", linkGroup.getName(),
+                targetGroup.getName());
+
+        s.setGroup(linkGroup.getName());
+        assertTrue(s.isSelected(null, null, symbolicLink.toFile()));
+        s.setFollowLinks("yes");
+        assertFalse(s.isSelected(null, null, symbolicLink.toFile()));
+    }
 }

http://git-wip-us.apache.org/repos/asf/ant/blob/35a84fea/src/tests/junit/org/apache/tools/ant/types/selectors/PosixPermissionsSelectorTest.java
----------------------------------------------------------------------
diff --git a/src/tests/junit/org/apache/tools/ant/types/selectors/PosixPermissionsSelectorTest.java b/src/tests/junit/org/apache/tools/ant/types/selectors/PosixPermissionsSelectorTest.java
index c72b61ae..1dde9c4 100644
--- a/src/tests/junit/org/apache/tools/ant/types/selectors/PosixPermissionsSelectorTest.java
+++ b/src/tests/junit/org/apache/tools/ant/types/selectors/PosixPermissionsSelectorTest.java
@@ -10,9 +10,16 @@ import org.junit.rules.TemporaryFolder;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.attribute.PosixFilePermission;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashSet;
+import java.util.Set;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assume.assumeTrue;
 
@@ -35,7 +42,7 @@ public class PosixPermissionsSelectorTest {
 
         @Before
         public void setUp() {
-            assumeTrue("no POSIX", Os.isFamily("unix"));
+            assumeTrue("Not POSIX", Os.isFamily("unix"));
             s = new PosixPermissionsSelector();
         }
 
@@ -54,9 +61,9 @@ public class PosixPermissionsSelectorTest {
         public TemporaryFolder folder = new TemporaryFolder();
 
         // requires JUnit 4.12
-        @Parameterized.Parameters(name = "legal argument: |{0}|")
+        @Parameterized.Parameters(name = "legal argument (self): |{0}|")
         public static Collection<String> data() {
-            return Arrays.asList("755", "rwxr-xr-x");
+            return Arrays.asList("750", "rwxr-x---");
         }
 
         @Parameterized.Parameter
@@ -64,14 +71,62 @@ public class PosixPermissionsSelectorTest {
 
         @Before
         public void setUp() {
-            assumeTrue("No POSIX", Os.isFamily("unix"));
+            assumeTrue("Not POSIX", Os.isFamily("unix"));
             s = new PosixPermissionsSelector();
         }
 
         @Test
-        public void PosixPermissionsIsTrueForSelf() throws Exception {
+        public void test() throws Exception {
+            // do not depend on default umask
+            File subFolder = folder.newFolder();
+            Set<PosixFilePermission> permissions = new HashSet<>();
+            permissions.add(PosixFilePermission.OWNER_READ);
+            permissions.add(PosixFilePermission.OWNER_WRITE);
+            permissions.add(PosixFilePermission.OWNER_EXECUTE);
+            permissions.add(PosixFilePermission.GROUP_READ);
+            permissions.add(PosixFilePermission.GROUP_EXECUTE);
+            Files.setPosixFilePermissions(subFolder.toPath(), permissions);
+
+            s.setPermissions(argument);
+            assertTrue(s.isSelected(null, null, subFolder));
+        }
+    }
+
+    @RunWith(Parameterized.class)
+    public static class LegalSymbolicLinkArgumentTest {
+
+        private final File TEST_FILE = new File("/etc/passwd");
+
+        private PosixPermissionsSelector s;
+
+        @Rule
+        public TemporaryFolder folder = new TemporaryFolder();
+
+        // requires JUnit 4.12
+        @Parameterized.Parameters(name = "legal argument (link): |{0}|")
+        public static Collection<String> data() {
+            return Arrays.asList("644", "rw-r--r--");
+        }
+
+        @Parameterized.Parameter
+        public String argument;
+
+        @Before
+        public void setUp() {
+            assumeTrue("Not POSIX", Os.isFamily("unix"));
+            s = new PosixPermissionsSelector();
+        }
+
+        @Test
+        public void test() throws Exception {
+            // symlinks have execute bit set by default
+            File target = new File(folder.getRoot(), "link");
+            Path symbolicLink = Files.createSymbolicLink(target.toPath(), TEST_FILE.toPath());
+
             s.setPermissions(argument);
-            assertTrue(s.isSelected(null, null, folder.newFolder()));
+            assertFalse(s.isSelected(null, null, symbolicLink.toFile()));
+            s.setFollowLinks("yes");
+            assertTrue(s.isSelected(null, null, symbolicLink.toFile()));
         }
     }
 


Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Gintautas Grigelionis <g....@gmail.com>.
More symlinks related issues (see [5] ... below)

Gintas

On Wed, 6 Jun 2018 at 21:30, Gintautas Grigelionis <g....@gmail.com>
wrote:

>
> There are other issues, like support for symlinks in archives (JRE does
> not support them in
> zip files [1], rather one -- like Gradle [2] -- must use Commons
> Compress). Actually, there are a couple
> of old Bugzilla issues addressing symlinks [3, 4] :-).
>
> [1] https://bugs.openjdk.java.net/browse/JDK-8184973
> [2] https://github.com/paleozogt/symzip-plugin
>
[3] https://bz.apache.org/bugzilla/show_bug.cgi?id=14320
>
[4] https://bz.apache.org/bugzilla/show_bug.cgi?id=15031
> [5] https://bz.apache.org/bugzilla/show_bug.cgi?id=15244
>
[6] https://bz.apache.org/bugzilla/show_bug.cgi?id=19252
>
[7] https://bz.apache.org/bugzilla/show_bug.cgi?id=40059
> [8] https://bz.apache.org/bugzilla/show_bug.cgi?id=56700
>

Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Gintautas Grigelionis <g....@gmail.com>.
Den lör 16 juni 2018 kl 17:29 skrev Stefan Bodewig <bo...@apache.org>:

> On 2018-06-16, Gintautas Grigelionis wrote:
>
> > 2018-06-16 15:42 GMT+02:00 Stefan Bodewig <bo...@apache.org>:
>
> >> On 2018-06-06, Gintautas Grigelionis wrote:
>
> >>> 2018-06-06 14:31 GMT+02:00 Stefan Bodewig <bo...@apache.org>:
>
> >>>> Would the symlink be included in DirectoryScanner's "included files"
> or
> >>>> "included directories"? What will <copy> do to a symlink that is
> >>>> included but not followed.
>
> >>> "Files or directories" dichotomy might be a straitjacket, but symlinks
> >> can
> >>> be fitted into it depending on what their target is.
>
> >> DirectoryScanner's interface provides you with files and directories and
> >> as it stands these File objects may actually by symlinks and the
> >> implicit expectation is that whoever uses them follows the links and in
> >> the end works on the target.
>
> > If things work as you believe, then it's simple -- FileSets just pass
> > the symlinks to consumers which may or may not check whether File
> > objects are symlinks. In the former case, they might use the new
> > semantics, in the latter case nothing's changed.
>
> In that case - the later - the followsymlinks="false" and
> skipsymlinks="false" would behave the same as followsymlinks="true" for
> those who do not check whether a file is a symlink, correct?
>

Correct.

In case of followsymlinks="false" and skipsymlinks="false" I expect
FileSets or
DirSets to contain symlinks as such; but well-behaved symlink-unaware tasks
would follow the symlinks effectively behaving as if followsymlinks="true"
(the default) with the old semantics.


> >>> I wonder if we can have an interim solution when selectors could use
> >>> proper followsymlinks semantics, but behaviour of DirectoryScanner is
> >>> unchanged?
>
> >> You may give it a try, I'm not sure exactly what you mean.
>
> > I attached a test case to one of my previous e-mails to illustrate
> > what I mean.
>
> The mailing list is configured to not allow attachments.
>

I just included in my reply on 6/6 at 21.30 without describing what it was
:-(
See [1] below.

> One part of it would be symlink support in archives (zip and tar).
>
> To which extent?
>
> When creating archives you may need to decide whether you want to use a
> relative or an absolute path to the target (I must admit I have no idea
> whether nio allows us to know how the target has been specified as
> opposed to just what the target is). When extracting and trying to
> re-create symlinks you may also need to watch out for path traversal
> problems.
>

To the extent where tarfilesets and zipfilesets can make use of symlinks in
the same way as filesets would do.
I am aware of extra complexity with path traversals that may cause loops
etc.

What is your time-frame for this? I've been thinking about cutting Ant
> releases soonish, but this is something for a different thread.
>

This is for the future, I'm quite content with what I could do with
selectors so far
(unless I missed something). I'm looking forward to spend time on symlinks
in other parts of Ant later this summer.

Gintas

[1] http://marc.info/?l=ant-dev&m=152833304425275&w=2

Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Stefan Bodewig <bo...@apache.org>.
On 2018-06-16, Gintautas Grigelionis wrote:

> 2018-06-16 15:42 GMT+02:00 Stefan Bodewig <bo...@apache.org>:

>> On 2018-06-06, Gintautas Grigelionis wrote:

>>> 2018-06-06 14:31 GMT+02:00 Stefan Bodewig <bo...@apache.org>:

>>>> I guess here our API breaks down as we only ever deal with files or
>>>> directories (outside of the symlink task).

>>> FileSet documentation should be more explicit about the matter, in
>>> particular explaining what "followsymlinks" really means.

>> You are right. In a Java world where you couldn't really do anything
>> with symlinks it has probably been clear implicitly.

> I would argue that things are less clear implicitly since Java 7, we just
> seemed to ignore it.

Right.

>>>> Would the symlink be included in DirectoryScanner's "included files" or
>>>> "included directories"? What will <copy> do to a symlink that is
>>>> included but not followed.

>>> "Files or directories" dichotomy might be a straitjacket, but symlinks
>> can
>>> be fitted into it depending on what their target is.

>> DirectoryScanner's interface provides you with files and directories and
>> as it stands these File objects may actually by symlinks and the
>> implicit expectation is that whoever uses them follows the links and in
>> the end works on the target.

> If things work as you believe, then it's simple -- FileSets just pass
> the symlinks to consumers which may or may not check whether File
> objects are symlinks. In the former case, they might use the new
> semantics, in the latter case nothing's changed.

In that case - the later - the followsymlinks="false" and
skipsymlinks="false" would behave the same as followsymlinks="true" for
those who do not check whether a file is a symlink, correct?

>>> I wonder if we can have an interim solution when selectors could use
>>> proper followsymlinks semantics, but behaviour of DirectoryScanner is
>>> unchanged?

>> You may give it a try, I'm not sure exactly what you mean.

> I attached a test case to one of my previous e-mails to illustrate
> what I mean.

The mailing list is configured to not allow attachments.

> Hmm, I've got an impression that Commons Compress was more capable;
> if Ant can handle zip and tar files with symlinks, then a big part of the
> job is done.

Commons Compress and Ant are the same in that you can create an archive
that contains entries that look like symlinks to tools that know how to
handle them. And at least in the case of tar and Commons Compress they
will tell you a certain entry is a symlink (for zip you need to detect
that yourself by looking at the flags).

In that sense we can create archives with symlinks in them and we will
be able to detect an entry inisde of an archive is a symlink. I'd have
to check whether we need to backport anything from Commons Compress in
order to make the little symlink support that is there is also available
to Ant.

>>> So, what's the plan?

>> It's your itch, what is your plan? :-)

> One part of it would be symlink support in archives (zip and tar).

To which extent?

When creating archives you may need to decide whether you want to use a
relative or an absolute path to the target (I must admit I have no idea
whether nio allows us to know how the target has been specified as
opposed to just what the target is). When extracting and trying to
re-create symlinks you may also need to watch out for path traversal
problems.

What is your time-frame for this? I've been thinking about cutting Ant
releases soonish, but this is soemthing for a different threat.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Gintautas Grigelionis <g....@gmail.com>.
2018-06-16 15:42 GMT+02:00 Stefan Bodewig <bo...@apache.org>:

> On 2018-06-06, Gintautas Grigelionis wrote:
>
> > 2018-06-06 14:31 GMT+02:00 Stefan Bodewig <bo...@apache.org>:
>
> >> I guess here our API breaks down as we only ever deal with files or
> >> directories (outside of the symlink task).
>
> > FileSet documentation should be more explicit about the matter, in
> > particular explaining what "followsymlinks" really means.
>
> You are right. In a Java world where you couldn't really do anything
> with symlinks it has probably been clear implicitly.
>

I would argue that things are less clear implicitly since Java 7, we just
seemed to ignore it.
Too bad change of Java ownership left things unfinished, as in archive
support.


> >> Would the symlink be included in DirectoryScanner's "included files" or
> >> "included directories"? What will <copy> do to a symlink that is
> >> included but not followed.
>
> > "Files or directories" dichotomy might be a straitjacket, but symlinks
> can
> > be fitted into it depending on what their target is.
>
> DirectoryScanner's interface provides you with files and directories and
> as it stands these File objects may actually by symlinks and the
> implicit expectation is that whoever uses them follows the links and in
> the end works on the target.
>

If things work as you believe, then it's simple -- FileSets just pass the
symlinks to consumers
which may or may not check whether File objects are symlinks. In the former
case, they might
use the new semantics, in the latter case nothing's changed.


> We could add new array of symlinks that are not supposed to be followed
> but most tasks would simply ignore them (all tasks that we don't change
> ourselves).
>
> > Dangling symlinks should go into notFollowedSymlinks.  Regarding
> > <copy>, included but not followed symlink must be left as is (eg
> > directories are not filtered either).
>
> Probably. There will not be a interpretation that will work for all
> tasks, it will be on a task by task basis. As we can only control the
> tasks that are inside of Ant's codebase, this means we must not change
> the interpretation of the files and directories returned by
> DirectoryScanner at all.
>

That is fine as long the tasks follow symlinks in a FileSet and no
additional structures for keeping them is needed.
If there are tasks that might choke on/skip a File which is a symlink, or,
as is the case with shared libraries on U*X,
add multiple copies of the same file to an archive by following symlinks,
then there's more work to do.


> > I wonder if we can have an interim solution when selectors could use
> > proper followsymlinks semantics, but behaviour of DirectoryScanner is
> > unchanged?
>
> You may give it a try, I'm not sure exactly what you mean.
>

I attached a test case to one of my previous e-mails to illustrate what I
mean.


> >>> Consequently, I expect
> >>> followsymlinks="false" skipsymlinks="false" to behave as
> >>> followsymlinks="false";
>
> >> which would be the same as skipsymlinks="true", right?
>
> > No, under the new proposal that would include the symlinks as symlinks,
> not
> > files or directories.
>
> Where would DirectoryScanner put those included symlinks?
>

Either treat them as files/directories, or put in a separate structure, as
discussed above.

> in the meantime, would it make sense to document what "followsymlinks"
> > means in FileSet and what "followsymlinks" means in selectors that
> > permit the attribute?
>
> We must document that, I'd say :-)
>

That's done.


> > There are other issues, like support for symlinks in archives (JRE does
> not
> > support them in
> > zip files [1], rather one -- like Gradle [2] -- must use Commons
> Compress).
> > Actually, there are a couple
> > of old Bugzilla issues addressing symlinks [3, 4] :-).
>
> I know Commons Compress pretty well ;-) and it doesn't really support
> symlinks in archives either. Given that Ant's zip package is the parent
> of Compress' zip support we should be able to do whatever Commons
> Compress can do here as well. There is a good reason why we don't use
> java.util.zip at all.
>

Hmm, I've got an impression that Commons Compress was more capable;
if Ant can handle zip and tar files with symlinks, then a big part of the
job is done.


> > So, what's the plan?
>
> It's your itch, what is your plan? :-)
>

One part of it would be symlink support in archives (zip and tar).
Another part would be investigating of what DirectoryScanner could do and
what is
expected of FileSet/DirSet, then deciding whether it is possible to fit the
symlinks in there.

Gintas

Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Stefan Bodewig <bo...@apache.org>.
On 2018-06-06, Gintautas Grigelionis wrote:

> 2018-06-06 14:31 GMT+02:00 Stefan Bodewig <bo...@apache.org>:

>> I guess here our API breaks down as we only ever deal with files or
>> directories (outside of the symlink task).

> FileSet documentation should be more explicit about the matter, in
> particular explaining what "followsymlinks" really means.

You are right. In a Java world where you couldn't really do anything
with symlinks it has probably been clear implicitly.

>> Would the symlink be included in DirectoryScanner's "included files" or
>> "included directories"? What will <copy> do to a symlink that is
>> included but not followed.

> "Files or directories" dichotomy might be a straitjacket, but symlinks can
> be fitted into it depending on what their target is.

DirectoryScanner's interface provides you with files and directories and
as it stands these File objects may actually by symlinks and the
implicit expectation is that whoever uses them follows the links and in
the end works on the target.

We could add new array of symlinks that are not supposed to be followed
but most tasks would simply ignore them (all tasks that we don't change
ourselves).

> Dangling symlinks should go into notFollowedSymlinks.  Regarding
> <copy>, included but not followed symlink must be left as is (eg
> directories are not filtered either).

Probably. There will not be a interpretation that will work for all
tasks, it will be on a task by task basis. As we can only control the
tasks that are inside of Ant's codebase, this means we must not change
the interpretation of the files and directories returned by
DirectoryScanner at all.

> I wonder if we can have an interim solution when selectors could use
> proper followsymlinks semantics, but behaviour of DirectoryScanner is
> unchanged?

You may give it a try, I'm not sure exactly what you mean.

>>> Consequently, I expect
>>> followsymlinks="false" skipsymlinks="false" to behave as
>>> followsymlinks="false";

>> which would be the same as skipsymlinks="true", right?

> No, under the new proposal that would include the symlinks as symlinks, not
> files or directories.

Where would DirectoryScanner put those included symlinks?

> in the meantime, would it make sense to document what "followsymlinks"
> means in FileSet and what "followsymlinks" means in selectors that
> permit the attribute?

We must document that, I'd say :-)

> There are other issues, like support for symlinks in archives (JRE does not
> support them in
> zip files [1], rather one -- like Gradle [2] -- must use Commons Compress).
> Actually, there are a couple
> of old Bugzilla issues addressing symlinks [3, 4] :-).

I know Commons Compress pretty well ;-) and it doesn't really support
symlinks in archives either. Given that Ant's zip package is the parent
of Compress' zip support we should be able to do whatever Commons
Compress can do here as well. There is a good reason why we don't use
java.util.zip at all.

> So, what's the plan?

It's your itch, what is your plan? :-)

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Gintautas Grigelionis <g....@gmail.com>.
2018-06-06 14:31 GMT+02:00 Stefan Bodewig <bo...@apache.org>:

> I guess here our API breaks down as we only ever deal with files or
> directories (outside of the symlink task).
>

FileSet documentation should be more explicit about the matter, in
particular explaining what "followsymlinks" really means.

Would the symlink be included in DirectoryScanner's "included files" or
> "included directories"? What will <copy> do to a symlink that is
> included but not followed.
>

"Files or directories" dichotomy might be a straitjacket, but symlinks can
be fitted into it depending on what their target is.
Dangling symlinks should go into notFollowedSymlinks.
Regarding <copy>, included but not followed symlink must be left as is (eg
directories are not filtered either).

>
> The current semantics of fileset's followsymlinks is deeply rooted in
> "we don't know how to deal with links and can only handle files and
> directories". I'm afraid a bunch of tasks will not do what you expect if
> DirectoryScanner suddenly returned File instances that are not real
> files/directories. Either they'd simply follow the link or break down -
> I assume in most cases it will be the former.
>

True; I wonder if we can have an interim solution when selectors could use
proper followsymlinks semantics,
but behaviour of DirectoryScanner is unchanged?


> > Consequently, I expect
> > followsymlinks="false" skipsymlinks="false" to behave as
> > followsymlinks="false";
>
> which would be the same as skipsymlinks="true", right?
>

No, under the new proposal that would include the symlinks as symlinks, not
files or directories.


> > whereas followsymlinks="true" skipsymlinks="true" is ambiguous: if
> > followsymlinks takes precedence, skipsymlinks is redundant; if
> > skipsymlinks takes precedence, then followsymlinks is ignored.
>
> So we'd need to decide what takes precedence and document it properly.
>
> As I said above, I don't think Ant's tasks are going to treat
> non-followed symlinks the way you'd expect them to. We could collect
> them separately and add a new getIncludedSymlinks method to
> DirectoryScanner so each task could do what it is supposed to do for
> not-followed links, but then we'll start documenting whether a given
> task X handles those links at all and if so what it does.
>

That's true; in the meantime, would it make sense to document what
"followsymlinks" means
in FileSet and what "followsymlinks" means in selectors that permit the
attribute?

There are other issues, like support for symlinks in archives (JRE does not
support them in
zip files [1], rather one -- like Gradle [2] -- must use Commons Compress).
Actually, there are a couple
of old Bugzilla issues addressing symlinks [3, 4] :-).

So, what's the plan?

Gintas

[1] https://bugs.openjdk.java.net/browse/JDK-8184973
[2] https://github.com/paleozogt/symzip-plugin
[3] https://bz.apache.org/bugzilla/show_bug.cgi?id=14320
[4] https://bz.apache.org/bugzilla/show_bug.cgi?id=15244

<project>
  <property name="work.dir" value="${java.io.tmpdir}/work"/>
  <delete dir="${work.dir}"/>
  <mkdir dir="${work.dir}"/>
  <touch file="${work.dir}/test.txt"/>
  <symlink link="${work.dir}/link.txt" resource="${work.dir}/test.txt"/>
  <symlink link="${work.dir}/passwd.txt" resource="/etc/passwd"/>

  <property name="current.work.dir" value="${user.dir}/work"/>
  <mkdir dir="${current.work.dir}"/>
  <touch file="${current.work.dir}/test.txt"/>
  <symlink link="${work.dir}/work" resource="${current.work.dir}"/>

  <fileset dir="${work.dir}" id="fileset"/>
  <echo>followsymlinks=true: ${toString:fileset}</echo>
  <fileset dir="${work.dir}" id="fileset-symlinks" followsymlinks="false"/>
  <echo>followsymlinks=false: ${toString:fileset-symlinks}</echo>
  <fileset dir="${work.dir}" id="fileset-is-symlink">
    <symlink/>
  </fileset>
  <echo>symlink followsymlinks=true: ${toString:fileset-is-symlink}</echo>
  <fileset dir="${work.dir}" id="fileset-symlinks-is-symlink"
followsymlinks="false">
    <symlink/>
  </fileset>
  <echo>symlink followsymlinks=false:
${toString:fileset-symlinks-is-symlink}</echo>

  <fileset dir="${work.dir}" id="fileset-owned-by-user">
    <ownedby owner="${user.name}"/>
  </fileset>
  <echo>ownedby ${user.name}: ${toString:fileset-owned-by-user}</echo>
  <fileset dir="${work.dir}" id="fileset-symlinks-owned-by-user">
    <ownedby owner="${user.name}" followsymlinks="false"/>
  </fileset>
  <echo>followsymlinks=false ownedby ${user.name}:
${toString:fileset-symlinks-owned-by-user}</echo>

  <property name="wheel.group" value="wheel"/>
  <fileset dir="${work.dir}" id="fileset-owned-by-group">
    <posixgroup group="${wheel.group}"/>
  </fileset>
  <echo>group ${wheel.group}: ${toString:fileset-owned-by-group}</echo>
  <fileset dir="${work.dir}" id="fileset-symlinks-owned-by-group">
    <posixgroup group="${wheel.group}" followsymlinks="false"/>
  </fileset>
  <echo>followsymlinks=false group ${wheel.group}:
${toString:fileset-symlinks-owned-by-group}</echo>

  <property name="file.permissions" value="644"/>
  <fileset dir="${work.dir}" id="fileset-with-permissions">
    <posixpermissions permissions="${file.permissions}"/>
  </fileset>
  <echo>permissions ${file.permissions}:
${toString:fileset-with-permissions}</echo>
  <fileset dir="${work.dir}" id="fileset-symlinks-with-permissions">
    <posixpermissions permissions="${file.permissions}"
followsymlinks="false"/>
  </fileset>
  <echo>followsymlinks=false permissions ${file.permissions}:
${toString:fileset-symlinks-with-permissions}</echo>
</project>

Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Stefan Bodewig <bo...@apache.org>.
On 2018-06-06, Gintautas Grigelionis wrote:

> 2018-06-06 10:50 GMT+02:00 Stefan Bodewig <bo...@apache.org>:

>> On 2018-06-05, Gintautas Grigelionis wrote:

>>> Stefan is right -- "followsymlinks" for the fileset should have been
>> called
>>> "skipsymlinks" or something like that.

>> I'm afraid I don't follow you here, what did you expect followsymlinks
>> going by the name? What would the "new semantics of followsymlinks" you
>> talk about be?

>> How would the combinations

>> followsymlinks="true" skipsymlinks="true"
>> followsymlinks="false" skipsymlinks="false"

>> behave?

> I expect the following:
> followsymlinks="false" should select symbolic links (rather than omitting
> them which seems to be its current behaviour);
> followsymlinks="true" should select whatever the links point at to (unless
> there is a loop).

I guess here our API breaks down as we only ever deal with files or
directories (outside of the symlink task).

Would the symlink be included in DirectoryScanner's "included files" or
"included directories"? What will <copy> do to a symlink that is
included but not followed.

The current semantics of fileset's followsymlinks is deeply rooted in
"we don't know how to deal with links and can only handle files and
directories". I'm afraid a bunch of tasks will not do what you expect if
DirectoryScanner suddenly returned File instances that are not real
files/directories. Either they'd simply follow the link or break down -
I assume in most cases it will be the former.

> Consequently, I expect
> followsymlinks="false" skipsymlinks="false" to behave as
> followsymlinks="false";

which would be the same as skipsymlinks="true", right?

> whereas followsymlinks="true" skipsymlinks="true" is ambiguous: if
> followsymlinks takes precedence, skipsymlinks is redundant; if
> skipsymlinks takes precedence, then followsymlinks is ignored.

So we'd need to decide what takes precedence and document it properly.

As I said above, I don't think Ant's tasks are going to treat
non-followed symlinks the way you'd expect them to. We could collect
them separately and add a new getIncludedSymlinks method to
DirectoryScanner so each task could do what it is supposed to do for
not-followed links, but then we'll start documenting whether a given
task X handles those links at all and if so what it does.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Gintautas Grigelionis <g....@gmail.com>.
2018-06-06 10:50 GMT+02:00 Stefan Bodewig <bo...@apache.org>:

> On 2018-06-05, Gintautas Grigelionis wrote:
>
> > Stefan is right -- "followsymlinks" for the fileset should have been
> called
> > "skipsymlinks" or something like that.
>
> I'm afraid I don't follow you here, what did you expect followsymlinks
> going by the name? What would the "new semantics of followsymlinks" you
> talk about be?
>
> How would the combinations
>
> followsymlinks="true" skipsymlinks="true"
> followsymlinks="false" skipsymlinks="false"
>
> behave?
>

I expect the following:
followsymlinks="false" should select symbolic links (rather than omitting
them which seems to be its current behaviour);
followsymlinks="true" should select whatever the links point at to (unless
there is a loop).

Consequently, I expect
followsymlinks="false" skipsymlinks="false" to behave as
followsymlinks="false"; whereas
followsymlinks="true" skipsymlinks="true" is ambiguous: if followsymlinks
takes precedence, skipsymlinks is redundant;
if skipsymlinks takes precedence, then followsymlinks is ignored.

Gintas

Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Stefan Bodewig <bo...@apache.org>.
On 2018-06-05, Gintautas Grigelionis wrote:

> Stefan is right -- "followsymlinks" for the fileset should have been called
> "skipsymlinks" or something like that.

I'm afraid I don't follow you here, what did you expect followsymlinks
going by the name? What would the "new semantics of followsymlinks" you
talk about be?

How would the combinations

followsymlinks="true" skipsymlinks="true"
followsymlinks="false" skipsymlinks="false"

behave?

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Gintautas Grigelionis <g....@gmail.com>.
2018-06-04 8:26 GMT+02:00 Stefan Bodewig <bo...@apache.org>:

> What happens when I do something like this
>
> <fileset dir="..." followsymlinks="false">
>   <ownedby owner="me" followsymlinks="true"/>
> </fileset>
>
> I believe - but haven't tested it - that for symlinks <ownedby> is never
> going to be invoked at all as DirectoryScanner will skip the symlink so
> the attribute's value on ownedby is irrelevant. If I'm correct we should
> probably document it somewhere.
>
> Of course the same is true for the existing <symlink> selector, so this
> is a more general task.


Stefan is right -- "followsymlinks" for the fileset should have been called
"skipsymlinks" or something like that.

What's worse, the way things are now, there is a risk for confusion.
I'd like add "skipsymlinks" and change "followsymlinks" for the fileset so
that
a fileset behaves as follows:

none of the attributes set (default):

skipsymlinks=false, followsymlinks=true (for consistency -- breaks BWC);

one ore both attributes set:

followsymlinks=true, skipsymlinks not set => warn, followsymlinks=false and
skipsymlinks=false for BWC;
followsymlinks=false, skipsymlinks not set => warn and skipsymlinks=true
for BWC;

skipsymlinks=false, followsymlinks set => new semantics for followsymlinks;
skipsymlinks=false, followsymlinks not set => new semantics,
followsymlinks=true for consistency;
skipsymlinks=true => followsymlinks not set -- ditto, else a warning about
useless attribute?

What do you think?

Gintas

Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Gintautas Grigelionis <g....@gmail.com>.
2018-06-04 8:26 GMT+02:00 Stefan Bodewig <bo...@apache.org>:

> What happens when I do something like this
>
> <fileset dir="..." followsymlinks="false">
>   <ownedby owner="me" followsymlinks="true"/>
> </fileset>
>
> I believe - but haven't tested it - that for symlinks <ownedby> is never
> going to be invoked at all as DirectoryScanner will skip the symlink so
> the attribute's value on ownedby is irrelevant. If I'm correct we should
> probably document it somewhere.
>
> Of course the same is true for the existing <symlink> selector, so this
> is a more general task.


I'll try to create a test case and cover as many combinations as possible,
including selector containers. My expectation is that followsymlinks="false"
only makes sense for a selector when followsymlinks="false" for the fileset.
I also expect that <symlink> only works in that case.

If my expectation turns out to be correct, I will document that as a general
footnote for all selectors that may or may not deal with symlinks depending
on
what DirectoryScanner finds in the first place.
It would be of interest to glean the value of followsymlinks of the fileset
and issue a warning when a selector is rendered less useful.

Gintas

Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Stefan Bodewig <bo...@apache.org>.
On 2018-06-01, Gintautas Grigelionis wrote:

> thanks for reviewing this. I missed the fact that filesets had a similar
> attribute.
> Hope everything is consistent now.

What I meant with

>> you should probably check and document how the new followlinks attribute
>> interacts with fileset's followsymlinks attribute.

was more something like: What happens when I do something like this

<fileset dir="..." followsymlinks="false">
  <ownedby owner="me" followsymlinks="true"/>
</fileset>

I believe - but haven't tested it - that for symlinks <ownedby> is never
going to be invoked at all as DirectoryScanner will skip the symlink so
the attribute's value on ownedby is irrelevant. If I'm correct we should
probably document it somewhere.

Of course the same is true for the existing <symlink> selector, so this
is a more general task.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org


Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Gintautas Grigelionis <g....@gmail.com>.
Hi Stefan,

thanks for reviewing this. I missed the fact that filesets had a similar
attribute.
Hope everything is consistent now.

Gintas

2018-05-28 17:00 GMT+02:00 Stefan Bodewig <bo...@apache.org>:

> Hi Gintas
>
> you should probably check and document how the new followlinks attribute
> interacts with fileset's followsymlinks attribute.
>
> Please add something to WHATSNEW.
>
> Some additional notes inline.
>
> On 2018-05-23, <gi...@apache.org> wrote:
>
> > http://git-wip-us.apache.org/repos/asf/ant/blob/35a84fea/
> manual/Types/selectors.html
> > ----------------------------------------------------------------------
> > diff --git a/manual/Types/selectors.html b/manual/Types/selectors.html
> > index 955a1b2..e3289af 100644
> > --- a/manual/Types/selectors.html
> > +++ b/manual/Types/selectors.html
> > @@ -925,6 +925,11 @@
> >          <td>Username of the expected owner</td>
> >          <td>Yes</td>
> >        </tr>
> > +        <tr>
> > +          <td>followlinks</td>
> > +          <td>Must the selector follow symbolic links?</td>
> > +          <td>No; defaults to <q>false</q> (was <q>true</q> before Ant
> 1.10.4)</td>
> > +        </tr>
> >      </table>
>
> Why change the default?
>
> > http://git-wip-us.apache.org/repos/asf/ant/blob/35a84fea/
> src/main/org/apache/tools/ant/types/selectors/OwnedBySelector.java
>
> > +    /**
> > +     * Sets the "follow links" flag.
> > +     * @param followLinks the user name
> > +     */
> > +    public void setFollowLinks(String followLinks) {
> > +        this.followLinks = PropertyHelper.toBoolean(followLinks);
> > +    }
>
> public void setFollowLinks(boolean followLinks) {
>     this.followLinks = followLinks;
> }
>
> does the same and looks clearer to me. Same for the other attribute
> setters.
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>
>

Re: [2/2] ant git commit: Bz 22370: followlinks attribute

Posted by Stefan Bodewig <bo...@apache.org>.
Hi Gintas

you should probably check and document how the new followlinks attribute
interacts with fileset's followsymlinks attribute.

Please add something to WHATSNEW.

Some additional notes inline.

On 2018-05-23, <gi...@apache.org> wrote:

> http://git-wip-us.apache.org/repos/asf/ant/blob/35a84fea/manual/Types/selectors.html
> ----------------------------------------------------------------------
> diff --git a/manual/Types/selectors.html b/manual/Types/selectors.html
> index 955a1b2..e3289af 100644
> --- a/manual/Types/selectors.html
> +++ b/manual/Types/selectors.html
> @@ -925,6 +925,11 @@
>          <td>Username of the expected owner</td>
>          <td>Yes</td>
>        </tr>
> +        <tr>
> +          <td>followlinks</td>
> +          <td>Must the selector follow symbolic links?</td>
> +          <td>No; defaults to <q>false</q> (was <q>true</q> before Ant 1.10.4)</td>
> +        </tr>
>      </table>

Why change the default?

> http://git-wip-us.apache.org/repos/asf/ant/blob/35a84fea/src/main/org/apache/tools/ant/types/selectors/OwnedBySelector.java

> +    /**
> +     * Sets the "follow links" flag.
> +     * @param followLinks the user name
> +     */
> +    public void setFollowLinks(String followLinks) {
> +        this.followLinks = PropertyHelper.toBoolean(followLinks);
> +    }

public void setFollowLinks(boolean followLinks) {
    this.followLinks = followLinks;
}

does the same and looks clearer to me. Same for the other attribute
setters.

Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
For additional commands, e-mail: dev-help@ant.apache.org