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

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Stefan Bodewig
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, <[hidden email]> 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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

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

Gintautas Grigelionis
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 <[hidden email]>:

> 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, <[hidden email]> 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: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

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

Stefan Bodewig
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

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

Gintautas Grigelionis
2018-06-04 8:26 GMT+02:00 Stefan Bodewig <[hidden email]>:

> 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
Reply | Threaded
Open this post in threaded view
|

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

Gintautas Grigelionis
2018-06-04 8:26 GMT+02:00 Stefan Bodewig <[hidden email]>:

> 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
Reply | Threaded
Open this post in threaded view
|

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

Stefan Bodewig
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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

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

Gintautas Grigelionis
2018-06-06 10:50 GMT+02:00 Stefan Bodewig <[hidden email]>:

> 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
Reply | Threaded
Open this post in threaded view
|

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

Stefan Bodewig
On 2018-06-06, Gintautas Grigelionis wrote:

> 2018-06-06 10:50 GMT+02:00 Stefan Bodewig <[hidden email]>:

>> 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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

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

Gintautas Grigelionis
2018-06-06 14:31 GMT+02:00 Stefan Bodewig <[hidden email]>:

> 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>
Reply | Threaded
Open this post in threaded view
|

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

Stefan Bodewig
On 2018-06-06, Gintautas Grigelionis wrote:

> 2018-06-06 14:31 GMT+02:00 Stefan Bodewig <[hidden email]>:

>> 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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

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

Gintautas Grigelionis
2018-06-16 15:42 GMT+02:00 Stefan Bodewig <[hidden email]>:

> On 2018-06-06, Gintautas Grigelionis wrote:
>
> > 2018-06-06 14:31 GMT+02:00 Stefan Bodewig <[hidden email]>:
>
> >> 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
Reply | Threaded
Open this post in threaded view
|

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

Stefan Bodewig
On 2018-06-16, Gintautas Grigelionis wrote:

> 2018-06-16 15:42 GMT+02:00 Stefan Bodewig <[hidden email]>:

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

>>> 2018-06-06 14:31 GMT+02:00 Stefan Bodewig <[hidden email]>:

>>>> 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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

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

Gintautas Grigelionis
Den lör 16 juni 2018 kl 17:29 skrev Stefan Bodewig <[hidden email]>:

> On 2018-06-16, Gintautas Grigelionis wrote:
>
> > 2018-06-16 15:42 GMT+02:00 Stefan Bodewig <[hidden email]>:
>
> >> On 2018-06-06, Gintautas Grigelionis wrote:
>
> >>> 2018-06-06 14:31 GMT+02:00 Stefan Bodewig <[hidden email]>:
>
> >>>> 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
Reply | Threaded
Open this post in threaded view
|

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

Gintautas Grigelionis
In reply to this post by Gintautas Grigelionis
More symlinks related issues (see [5] ... below)

Gintas

On Wed, 6 Jun 2018 at 21:30, Gintautas Grigelionis <[hidden email]>
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
>