ant-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jaikiran Pai <jai.forums2...@gmail.com>
Subject Re: Potential breaking change to symlink task [ was Re: Jenkins build became unstable: Ant-Build-Matrix-master-Linux » JDK 1.8 (latest) #977]
Date Tue, 12 Dec 2017 05:32:19 GMT
I went back and read up both these bugzilla reports again and I now 
realize that supporting one doesn't necessarily mean not supporting the 
other. BZ-43426 is about letting existing file be overwritten 
(irrespective of the type of the file) if the overwrite flag is true. 
BZ-58683 is about not overwriting any kind of file, if overwrite flag is 
false (before the changes I introduced, it was in fact overwriting files 
even when overwrite was false, a side-effect of the ln -s usage).

So I have now pushed a fix[1] on top of my previous changes, which 
should accommodate both these use cases (and continue to use Java 7 APIs 
for symlinking). No change has been done to the existing test and it now 
passes. Do let me know, if this is good enough to have BZ-58683 fixed 
and yet maintain backward compatibility with existing released versions 
of Ant. If you do see any issues with this set of changes, I am willing 
to undo them and go back to the original state that was before I started 
these changes.

[1] 
https://github.com/apache/ant/commit/b3c7d5dc451960986a94d24785a2c1d24b0b0d6a

-Jaikiran


On 10/12/17 5:15 PM, Stefan Bodewig wrote:
> [@Steve I'm trying to drag you in as you've been the one who brought in
> the change that gets contested by
> https://bz.apache.org/bugzilla/show_bug.cgi?id=58683 and maybe you
> recall the details better than we do.]
>
> On 2017-12-10, Jaikiran Pai wrote:
>
>> On 10/12/17 3:09 PM, Stefan Bodewig wrote:
>>> On 2017-12-10, Jaikiran Pai wrote:
>>>> I'll investigate why this is failing (local tests pass for me) and fix it.
>>> Target testCreateOverFile in the antunit test explicitly tries to
>>> replace a file with a link, doing exactly what the bugzilla report says
>>> is a bug. So the behavior seems to have been intentional.
>>> We now need to figure out whether the bug report was genuine (and list
>>> the change as breaking) or we want to revert part of your fix, change
>>> the documentation and change the bugzilla issue's resolution to a
>>> WONTFIX.
>> The symlink task documentation[1] states this for the "overwrite" attribute:
>> <quote>
>> Overwrite existing links or not.
>> </quote>
>> It does explicitly mention that it's meant for overwriting links. The
>> test here, like you note, tries to overwrite a non-symlink file and
>> IMO it should error out, like it does with this change. After all, the
>> entire symlink task is meant to just deal with symlinks and
>> overwriting non-symlinks doesn't seem right. Having said that, I do
>> agree that this is a change in behaviour and we should list this as
>> such, if we do decide to let this change stay.
>> The reason why that existing test was introduced in first place and
>> the feature to let the symlink task overwrite a non-symlink file goes
>> back to this issue
>> https://bz.apache.org/bugzilla/show_bug.cgi?id=43426. The rationale in
>> that description seems to be that the Unix ln -sf apparently allows
>> overwriting non-symlink file with a symlink. Given that context, I'm
>> not really sure how we should proceed. Should we make this breaking
>> change or should we let the prior behaviour continue?
>> I'm in favour of this breaking change, but I won't mind switching back
>> to the prior behaviour if you and others think that's a better thing
>> to do.
> Thank you for digging into the history, Jaikiran.
>
> 43426 is explicitly listed as a fixed bug (in 1.8.0) so I'm leaning
> towards keeping and documenting the behavior of allowing <symlink> to
> replace regular files.
>
> Stefan
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@ant.apache.org
> For additional commands, e-mail: dev-help@ant.apache.org
>


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


Mime
View raw message