Proposal: removeDirectoryRecursive should not follow symlinks

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
52 messages Options
123
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: removeDirectoryRecursive should not follow symlinks

David Feuer

This seems reasonable, but if we have a deprecation cycle, the old name should (temporarily) be a synonym for the new one, and the deprecation warning should indicate that code intended to work with older versions needs to be audited.

On Jan 6, 2015 2:40 PM, "Gabriel Gonzalez" <[hidden email]> wrote:
I think it's safer to remove the old function altogether (perhaps after one deprecation cycle) and provide a new one under a different name, rather than modify it in place.

Modifying it in place risks the behavior that others mentioned where your program is unsafe to compile against older library versions.  Yes, the user could explicitly enforce that by putting a lower bound on the library, but most users won't even realize that they need to do that.


On 1/6/15, 11:37 AM, Edward Kmett wrote:
I'm +1 for fixing this, in place, on the current function.

The specification we have here is doing a very very bad thing and needs to be fixed, not slavishly copied forward because someone sometime once made a mistake. 

The current behavior grievously violates the expectations of anyone who would be in a situation to go and reach for it and has any prior experience with any other such tool.

-Edward



On Tue, Jan 6, 2015 at 11:14 AM, Malcolm Wallace <[hidden email]> wrote:

On 6 Jan 2015, at 14:59, Bardur Arantsson wrote:

> On 2015-01-06 14:57, Mike Meyer wrote:
>> On Tue, Jan 6, 2015 at 7:48 AM, Johan Tibell <[hidden email]> wrote:
>>
>>> This is not a bugfix. A bug is failing to follow the functions
>>> specification, which *does* include following symlinks.
>>>
>>
>> It's a bug in the design, not the code.

> Because *nobody* wants to follow symlinks when doing "rm -rf". Even if
> they think they do, they *really* don't.

I agree 100%.  Even time I use this function, I worry briefly about whether it follows symlinks, then think to myself "no, no-one would be so stupid to implement that deliberately in a publically available API".  So it was a real shock to discover in this thread that I was wrong, and furthermore that the function is documented as doing the wrong thing.  We should fix both spec and implementation, as soon as possible.

Regards,
    Malcolm
_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries



_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries


_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries


_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: removeDirectoryRecursive should not follow symlinks

Johan Tibell-2
Who volunteers to fix the breakages in Cabal and add all the needed CPP?

On Tue, Jan 6, 2015 at 2:45 PM, David Feuer <[hidden email]> wrote:

This seems reasonable, but if we have a deprecation cycle, the old name should (temporarily) be a synonym for the new one, and the deprecation warning should indicate that code intended to work with older versions needs to be audited.

On Jan 6, 2015 2:40 PM, "Gabriel Gonzalez" <[hidden email]> wrote:
I think it's safer to remove the old function altogether (perhaps after one deprecation cycle) and provide a new one under a different name, rather than modify it in place.

Modifying it in place risks the behavior that others mentioned where your program is unsafe to compile against older library versions.  Yes, the user could explicitly enforce that by putting a lower bound on the library, but most users won't even realize that they need to do that.


On 1/6/15, 11:37 AM, Edward Kmett wrote:
I'm +1 for fixing this, in place, on the current function.

The specification we have here is doing a very very bad thing and needs to be fixed, not slavishly copied forward because someone sometime once made a mistake. 

The current behavior grievously violates the expectations of anyone who would be in a situation to go and reach for it and has any prior experience with any other such tool.

-Edward



On Tue, Jan 6, 2015 at 11:14 AM, Malcolm Wallace <[hidden email]> wrote:

On 6 Jan 2015, at 14:59, Bardur Arantsson wrote:

> On 2015-01-06 14:57, Mike Meyer wrote:
>> On Tue, Jan 6, 2015 at 7:48 AM, Johan Tibell <[hidden email]> wrote:
>>
>>> This is not a bugfix. A bug is failing to follow the functions
>>> specification, which *does* include following symlinks.
>>>
>>
>> It's a bug in the design, not the code.

> Because *nobody* wants to follow symlinks when doing "rm -rf". Even if
> they think they do, they *really* don't.

I agree 100%.  Even time I use this function, I worry briefly about whether it follows symlinks, then think to myself "no, no-one would be so stupid to implement that deliberately in a publically available API".  So it was a real shock to discover in this thread that I was wrong, and furthermore that the function is documented as doing the wrong thing.  We should fix both spec and implementation, as soon as possible.

Regards,
    Malcolm
_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries



_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries


_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries


_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries



_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: removeDirectoryRecursive should not follow symlinks

Erik Hesselink
Does cabal rely on this behavior?

Erik

On Tue, Jan 6, 2015 at 9:37 PM, Johan Tibell <[hidden email]> wrote:

> Who volunteers to fix the breakages in Cabal and add all the needed CPP?
>
> On Tue, Jan 6, 2015 at 2:45 PM, David Feuer <[hidden email]> wrote:
>>
>> This seems reasonable, but if we have a deprecation cycle, the old name
>> should (temporarily) be a synonym for the new one, and the deprecation
>> warning should indicate that code intended to work with older versions needs
>> to be audited.
>>
>> On Jan 6, 2015 2:40 PM, "Gabriel Gonzalez" <[hidden email]> wrote:
>>>
>>> I think it's safer to remove the old function altogether (perhaps after
>>> one deprecation cycle) and provide a new one under a different name, rather
>>> than modify it in place.
>>>
>>> Modifying it in place risks the behavior that others mentioned where your
>>> program is unsafe to compile against older library versions.  Yes, the user
>>> could explicitly enforce that by putting a lower bound on the library, but
>>> most users won't even realize that they need to do that.
>>>
>>>
>>> On 1/6/15, 11:37 AM, Edward Kmett wrote:
>>>
>>> I'm +1 for fixing this, in place, on the current function.
>>>
>>> The specification we have here is doing a very very bad thing and needs
>>> to be fixed, not slavishly copied forward because someone sometime once made
>>> a mistake.
>>>
>>> The current behavior grievously violates the expectations of anyone who
>>> would be in a situation to go and reach for it and has any prior experience
>>> with any other such tool.
>>>
>>> -Edward
>>>
>>>
>>>
>>> On Tue, Jan 6, 2015 at 11:14 AM, Malcolm Wallace <[hidden email]>
>>> wrote:
>>>>
>>>>
>>>> On 6 Jan 2015, at 14:59, Bardur Arantsson wrote:
>>>>
>>>> > On 2015-01-06 14:57, Mike Meyer wrote:
>>>> >> On Tue, Jan 6, 2015 at 7:48 AM, Johan Tibell <[hidden email]>
>>>> >> wrote:
>>>> >>
>>>> >>> This is not a bugfix. A bug is failing to follow the functions
>>>> >>> specification, which *does* include following symlinks.
>>>> >>>
>>>> >>
>>>> >> It's a bug in the design, not the code.
>>>>
>>>> > Because *nobody* wants to follow symlinks when doing "rm -rf". Even if
>>>> > they think they do, they *really* don't.
>>>>
>>>> I agree 100%.  Even time I use this function, I worry briefly about
>>>> whether it follows symlinks, then think to myself "no, no-one would be so
>>>> stupid to implement that deliberately in a publically available API".  So it
>>>> was a real shock to discover in this thread that I was wrong, and
>>>> furthermore that the function is documented as doing the wrong thing.  We
>>>> should fix both spec and implementation, as soon as possible.
>>>>
>>>> Regards,
>>>>     Malcolm
>>>> _______________________________________________
>>>> Libraries mailing list
>>>> [hidden email]
>>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Libraries mailing list
>>> [hidden email]
>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>>
>>>
>>> _______________________________________________
>>> Libraries mailing list
>>> [hidden email]
>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>
>> _______________________________________________
>> Libraries mailing list
>> [hidden email]
>> http://www.haskell.org/mailman/listinfo/libraries
>>
>
>
> _______________________________________________
> Libraries mailing list
> [hidden email]
> http://www.haskell.org/mailman/listinfo/libraries
>
_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: removeDirectoryRecursive should not follow symlinks

Edward Z. Yang
I did a quick check, and all references in Cabal implicitly assume there
will be no symlinks in the directory being deleted.

Edward

Excerpts from Erik Hesselink's message of 2015-01-06 12:39:22 -0800:

> Does cabal rely on this behavior?
>
> Erik
>
> On Tue, Jan 6, 2015 at 9:37 PM, Johan Tibell <[hidden email]> wrote:
> > Who volunteers to fix the breakages in Cabal and add all the needed CPP?
> >
> > On Tue, Jan 6, 2015 at 2:45 PM, David Feuer <[hidden email]> wrote:
> >>
> >> This seems reasonable, but if we have a deprecation cycle, the old name
> >> should (temporarily) be a synonym for the new one, and the deprecation
> >> warning should indicate that code intended to work with older versions needs
> >> to be audited.
> >>
> >> On Jan 6, 2015 2:40 PM, "Gabriel Gonzalez" <[hidden email]> wrote:
> >>>
> >>> I think it's safer to remove the old function altogether (perhaps after
> >>> one deprecation cycle) and provide a new one under a different name, rather
> >>> than modify it in place.
> >>>
> >>> Modifying it in place risks the behavior that others mentioned where your
> >>> program is unsafe to compile against older library versions.  Yes, the user
> >>> could explicitly enforce that by putting a lower bound on the library, but
> >>> most users won't even realize that they need to do that.
> >>>
> >>>
> >>> On 1/6/15, 11:37 AM, Edward Kmett wrote:
> >>>
> >>> I'm +1 for fixing this, in place, on the current function.
> >>>
> >>> The specification we have here is doing a very very bad thing and needs
> >>> to be fixed, not slavishly copied forward because someone sometime once made
> >>> a mistake.
> >>>
> >>> The current behavior grievously violates the expectations of anyone who
> >>> would be in a situation to go and reach for it and has any prior experience
> >>> with any other such tool.
> >>>
> >>> -Edward
> >>>
> >>>
> >>>
> >>> On Tue, Jan 6, 2015 at 11:14 AM, Malcolm Wallace <[hidden email]>
> >>> wrote:
> >>>>
> >>>>
> >>>> On 6 Jan 2015, at 14:59, Bardur Arantsson wrote:
> >>>>
> >>>> > On 2015-01-06 14:57, Mike Meyer wrote:
> >>>> >> On Tue, Jan 6, 2015 at 7:48 AM, Johan Tibell <[hidden email]>
> >>>> >> wrote:
> >>>> >>
> >>>> >>> This is not a bugfix. A bug is failing to follow the functions
> >>>> >>> specification, which *does* include following symlinks.
> >>>> >>>
> >>>> >>
> >>>> >> It's a bug in the design, not the code.
> >>>>
> >>>> > Because *nobody* wants to follow symlinks when doing "rm -rf". Even if
> >>>> > they think they do, they *really* don't.
> >>>>
> >>>> I agree 100%.  Even time I use this function, I worry briefly about
> >>>> whether it follows symlinks, then think to myself "no, no-one would be so
> >>>> stupid to implement that deliberately in a publically available API".  So it
> >>>> was a real shock to discover in this thread that I was wrong, and
> >>>> furthermore that the function is documented as doing the wrong thing.  We
> >>>> should fix both spec and implementation, as soon as possible.
> >>>>
> >>>> Regards,
> >>>>     Malcolm
> >>>> _______________________________________________
> >>>> Libraries mailing list
> >>>> [hidden email]
> >>>> http://www.haskell.org/mailman/listinfo/libraries
> >>>
> >>>
> >>>
> >>>
> >>> _______________________________________________
> >>> Libraries mailing list
> >>> [hidden email]
> >>> http://www.haskell.org/mailman/listinfo/libraries
> >>>
> >>>
> >>>
> >>> _______________________________________________
> >>> Libraries mailing list
> >>> [hidden email]
> >>> http://www.haskell.org/mailman/listinfo/libraries
> >>>
> >>
> >> _______________________________________________
> >> Libraries mailing list
> >> [hidden email]
> >> http://www.haskell.org/mailman/listinfo/libraries
> >>
> >
> >
> > _______________________________________________
> > Libraries mailing list
> > [hidden email]
> > http://www.haskell.org/mailman/listinfo/libraries
> >
_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: removeDirectoryRecursive should not follow symlinks

Eric Mertens
In reply to this post by Johan Tibell-2

There's no option which avoid needing to fix all packages on hackage, so each maintainer using this function will be responsible for fixing his packages.

If we fix it in place everyone needs to add CPP to avoid calling the broken version on old versions of directory and use an alternative implementation.

If we make a new function everyone needs to conditionally call the new function or use an alternative implementation.


On Tue, Jan 6, 2015, 12:38 PM Johan Tibell <[hidden email]> wrote:
Who volunteers to fix the breakages in Cabal and add all the needed CPP?

On Tue, Jan 6, 2015 at 2:45 PM, David Feuer <[hidden email]> wrote:

This seems reasonable, but if we have a deprecation cycle, the old name should (temporarily) be a synonym for the new one, and the deprecation warning should indicate that code intended to work with older versions needs to be audited.

On Jan 6, 2015 2:40 PM, "Gabriel Gonzalez" <[hidden email]> wrote:
I think it's safer to remove the old function altogether (perhaps after one deprecation cycle) and provide a new one under a different name, rather than modify it in place.

Modifying it in place risks the behavior that others mentioned where your program is unsafe to compile against older library versions.  Yes, the user could explicitly enforce that by putting a lower bound on the library, but most users won't even realize that they need to do that.


On 1/6/15, 11:37 AM, Edward Kmett wrote:
I'm +1 for fixing this, in place, on the current function.

The specification we have here is doing a very very bad thing and needs to be fixed, not slavishly copied forward because someone sometime once made a mistake. 

The current behavior grievously violates the expectations of anyone who would be in a situation to go and reach for it and has any prior experience with any other such tool.

-Edward



On Tue, Jan 6, 2015 at 11:14 AM, Malcolm Wallace <[hidden email]> wrote:

On 6 Jan 2015, at 14:59, Bardur Arantsson wrote:

> On 2015-01-06 14:57, Mike Meyer wrote:
>> On Tue, Jan 6, 2015 at 7:48 AM, Johan Tibell <[hidden email]> wrote:
>>
>>> This is not a bugfix. A bug is failing to follow the functions
>>> specification, which *does* include following symlinks.
>>>
>>
>> It's a bug in the design, not the code.

> Because *nobody* wants to follow symlinks when doing "rm -rf". Even if
> they think they do, they *really* don't.

I agree 100%.  Even time I use this function, I worry briefly about whether it follows symlinks, then think to myself "no, no-one would be so stupid to implement that deliberately in a publically available API".  So it was a real shock to discover in this thread that I was wrong, and furthermore that the function is documented as doing the wrong thing.  We should fix both spec and implementation, as soon as possible.

Regards,
    Malcolm
_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries



_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries


_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries


_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries


_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: removeDirectoryRecursive should not follow symlinks

David Feuer
Do we need a separate compatibility package for just this function?
Not many people will want to try to roll their own.

On Tue, Jan 6, 2015 at 6:30 PM, Eric Mertens <[hidden email]> wrote:
> There's no option which avoid needing to fix all packages on hackage, so
> each maintainer using this function will be responsible for fixing his
> packages.
>
> If we fix it in place everyone needs to add CPP to avoid calling the broken
> version on old versions of directory and use an alternative implementation.
>
> If we make a new function everyone needs to conditionally call the new
> function or use an alternative implementation.
_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: removeDirectoryRecursive should not follow symlinks

Johan Tibell-2
In reply to this post by Erik Hesselink

I don't think so but if we change the function signature or name as some suggested it all needs to be cpped still.

On Jan 6, 2015 9:39 PM, "Erik Hesselink" <[hidden email]> wrote:
Does cabal rely on this behavior?

Erik

On Tue, Jan 6, 2015 at 9:37 PM, Johan Tibell <[hidden email]> wrote:
> Who volunteers to fix the breakages in Cabal and add all the needed CPP?
>
> On Tue, Jan 6, 2015 at 2:45 PM, David Feuer <[hidden email]> wrote:
>>
>> This seems reasonable, but if we have a deprecation cycle, the old name
>> should (temporarily) be a synonym for the new one, and the deprecation
>> warning should indicate that code intended to work with older versions needs
>> to be audited.
>>
>> On Jan 6, 2015 2:40 PM, "Gabriel Gonzalez" <[hidden email]> wrote:
>>>
>>> I think it's safer to remove the old function altogether (perhaps after
>>> one deprecation cycle) and provide a new one under a different name, rather
>>> than modify it in place.
>>>
>>> Modifying it in place risks the behavior that others mentioned where your
>>> program is unsafe to compile against older library versions.  Yes, the user
>>> could explicitly enforce that by putting a lower bound on the library, but
>>> most users won't even realize that they need to do that.
>>>
>>>
>>> On 1/6/15, 11:37 AM, Edward Kmett wrote:
>>>
>>> I'm +1 for fixing this, in place, on the current function.
>>>
>>> The specification we have here is doing a very very bad thing and needs
>>> to be fixed, not slavishly copied forward because someone sometime once made
>>> a mistake.
>>>
>>> The current behavior grievously violates the expectations of anyone who
>>> would be in a situation to go and reach for it and has any prior experience
>>> with any other such tool.
>>>
>>> -Edward
>>>
>>>
>>>
>>> On Tue, Jan 6, 2015 at 11:14 AM, Malcolm Wallace <[hidden email]>
>>> wrote:
>>>>
>>>>
>>>> On 6 Jan 2015, at 14:59, Bardur Arantsson wrote:
>>>>
>>>> > On 2015-01-06 14:57, Mike Meyer wrote:
>>>> >> On Tue, Jan 6, 2015 at 7:48 AM, Johan Tibell <[hidden email]>
>>>> >> wrote:
>>>> >>
>>>> >>> This is not a bugfix. A bug is failing to follow the functions
>>>> >>> specification, which *does* include following symlinks.
>>>> >>>
>>>> >>
>>>> >> It's a bug in the design, not the code.
>>>>
>>>> > Because *nobody* wants to follow symlinks when doing "rm -rf". Even if
>>>> > they think they do, they *really* don't.
>>>>
>>>> I agree 100%.  Even time I use this function, I worry briefly about
>>>> whether it follows symlinks, then think to myself "no, no-one would be so
>>>> stupid to implement that deliberately in a publically available API".  So it
>>>> was a real shock to discover in this thread that I was wrong, and
>>>> furthermore that the function is documented as doing the wrong thing.  We
>>>> should fix both spec and implementation, as soon as possible.
>>>>
>>>> Regards,
>>>>     Malcolm
>>>> _______________________________________________
>>>> Libraries mailing list
>>>> [hidden email]
>>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Libraries mailing list
>>> [hidden email]
>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>>
>>>
>>> _______________________________________________
>>> Libraries mailing list
>>> [hidden email]
>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>
>> _______________________________________________
>> Libraries mailing list
>> [hidden email]
>> http://www.haskell.org/mailman/listinfo/libraries
>>
>
>
> _______________________________________________
> Libraries mailing list
> [hidden email]
> http://www.haskell.org/mailman/listinfo/libraries
>

_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: removeDirectoryRecursive should not follow symlinks

Antonio Nikishaev
In reply to this post by Edward Z. Yang
"Edward Z. Yang" <[hidden email]> writes:

> I propose to backwards incompatibly change the behavior of
> removeDirectoryRecursive to not follow symlinks.  We could optionally
> add a replacement function under a new name which does follow
> symlinks.

+1 for changing it immediately,
-1 for any deprecation period,
-1 for adding the old one under another name

Motivation: even if documented, the behaviour is severe bug with
security implications.  I don't think anyone in the right mind would
expect such a function to follow symlinks.



_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: removeDirectoryRecursive should not follow symlinks

Gabriel Gonzalez
In reply to this post by Johan Tibell-2
Actually, I might retract my recommendation to provided it under a different name.  I'm neutral on this now and would consider it okay to change in place.

My reasoning is that this is a bug fix, so we can reasonably expect users to put lower bounds on software in response to bug fixes like we do with other software.

On 1/7/15, 1:47 AM, Johan Tibell wrote:

I don't think so but if we change the function signature or name as some suggested it all needs to be cpped still.

On Jan 6, 2015 9:39 PM, "Erik Hesselink" <[hidden email]> wrote:
Does cabal rely on this behavior?

Erik

On Tue, Jan 6, 2015 at 9:37 PM, Johan Tibell <[hidden email]> wrote:
> Who volunteers to fix the breakages in Cabal and add all the needed CPP?
>
> On Tue, Jan 6, 2015 at 2:45 PM, David Feuer <[hidden email]> wrote:
>>
>> This seems reasonable, but if we have a deprecation cycle, the old name
>> should (temporarily) be a synonym for the new one, and the deprecation
>> warning should indicate that code intended to work with older versions needs
>> to be audited.
>>
>> On Jan 6, 2015 2:40 PM, "Gabriel Gonzalez" <[hidden email]> wrote:
>>>
>>> I think it's safer to remove the old function altogether (perhaps after
>>> one deprecation cycle) and provide a new one under a different name, rather
>>> than modify it in place.
>>>
>>> Modifying it in place risks the behavior that others mentioned where your
>>> program is unsafe to compile against older library versions.  Yes, the user
>>> could explicitly enforce that by putting a lower bound on the library, but
>>> most users won't even realize that they need to do that.
>>>
>>>
>>> On 1/6/15, 11:37 AM, Edward Kmett wrote:
>>>
>>> I'm +1 for fixing this, in place, on the current function.
>>>
>>> The specification we have here is doing a very very bad thing and needs
>>> to be fixed, not slavishly copied forward because someone sometime once made
>>> a mistake.
>>>
>>> The current behavior grievously violates the expectations of anyone who
>>> would be in a situation to go and reach for it and has any prior experience
>>> with any other such tool.
>>>
>>> -Edward
>>>
>>>
>>>
>>> On Tue, Jan 6, 2015 at 11:14 AM, Malcolm Wallace <[hidden email]>
>>> wrote:
>>>>
>>>>
>>>> On 6 Jan 2015, at 14:59, Bardur Arantsson wrote:
>>>>
>>>> > On 2015-01-06 14:57, Mike Meyer wrote:
>>>> >> On Tue, Jan 6, 2015 at 7:48 AM, Johan Tibell <[hidden email]>
>>>> >> wrote:
>>>> >>
>>>> >>> This is not a bugfix. A bug is failing to follow the functions
>>>> >>> specification, which *does* include following symlinks.
>>>> >>>
>>>> >>
>>>> >> It's a bug in the design, not the code.
>>>>
>>>> > Because *nobody* wants to follow symlinks when doing "rm -rf". Even if
>>>> > they think they do, they *really* don't.
>>>>
>>>> I agree 100%.  Even time I use this function, I worry briefly about
>>>> whether it follows symlinks, then think to myself "no, no-one would be so
>>>> stupid to implement that deliberately in a publically available API".  So it
>>>> was a real shock to discover in this thread that I was wrong, and
>>>> furthermore that the function is documented as doing the wrong thing.  We
>>>> should fix both spec and implementation, as soon as possible.
>>>>
>>>> Regards,
>>>>     Malcolm
>>>> _______________________________________________
>>>> Libraries mailing list
>>>> [hidden email]
>>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Libraries mailing list
>>> [hidden email]
>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>>
>>>
>>> _______________________________________________
>>> Libraries mailing list
>>> [hidden email]
>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>
>> _______________________________________________
>> Libraries mailing list
>> [hidden email]
>> http://www.haskell.org/mailman/listinfo/libraries
>>
>
>
> _______________________________________________
> Libraries mailing list
> [hidden email]
> http://www.haskell.org/mailman/listinfo/libraries
>


_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries


_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: removeDirectoryRecursive should not follow symlinks

Greg Weber
We all agree that we should do something about this. For 99% of use cases this is a bug fix.
So I would like to just leave it up to the implementer to decide what to do.


On Wed, Jan 7, 2015 at 9:49 AM, Gabriel Gonzalez <[hidden email]> wrote:
Actually, I might retract my recommendation to provided it under a different name.  I'm neutral on this now and would consider it okay to change in place.

My reasoning is that this is a bug fix, so we can reasonably expect users to put lower bounds on software in response to bug fixes like we do with other software.


On 1/7/15, 1:47 AM, Johan Tibell wrote:

I don't think so but if we change the function signature or name as some suggested it all needs to be cpped still.

On Jan 6, 2015 9:39 PM, "Erik Hesselink" <[hidden email]> wrote:
Does cabal rely on this behavior?

Erik

On Tue, Jan 6, 2015 at 9:37 PM, Johan Tibell <[hidden email]> wrote:
> Who volunteers to fix the breakages in Cabal and add all the needed CPP?
>
> On Tue, Jan 6, 2015 at 2:45 PM, David Feuer <[hidden email]> wrote:
>>
>> This seems reasonable, but if we have a deprecation cycle, the old name
>> should (temporarily) be a synonym for the new one, and the deprecation
>> warning should indicate that code intended to work with older versions needs
>> to be audited.
>>
>> On Jan 6, 2015 2:40 PM, "Gabriel Gonzalez" <[hidden email]> wrote:
>>>
>>> I think it's safer to remove the old function altogether (perhaps after
>>> one deprecation cycle) and provide a new one under a different name, rather
>>> than modify it in place.
>>>
>>> Modifying it in place risks the behavior that others mentioned where your
>>> program is unsafe to compile against older library versions.  Yes, the user
>>> could explicitly enforce that by putting a lower bound on the library, but
>>> most users won't even realize that they need to do that.
>>>
>>>
>>> On 1/6/15, 11:37 AM, Edward Kmett wrote:
>>>
>>> I'm +1 for fixing this, in place, on the current function.
>>>
>>> The specification we have here is doing a very very bad thing and needs
>>> to be fixed, not slavishly copied forward because someone sometime once made
>>> a mistake.
>>>
>>> The current behavior grievously violates the expectations of anyone who
>>> would be in a situation to go and reach for it and has any prior experience
>>> with any other such tool.
>>>
>>> -Edward
>>>
>>>
>>>
>>> On Tue, Jan 6, 2015 at 11:14 AM, Malcolm Wallace <[hidden email]>
>>> wrote:
>>>>
>>>>
>>>> On 6 Jan 2015, at 14:59, Bardur Arantsson wrote:
>>>>
>>>> > On 2015-01-06 14:57, Mike Meyer wrote:
>>>> >> On Tue, Jan 6, 2015 at 7:48 AM, Johan Tibell <[hidden email]>
>>>> >> wrote:
>>>> >>
>>>> >>> This is not a bugfix. A bug is failing to follow the functions
>>>> >>> specification, which *does* include following symlinks.
>>>> >>>
>>>> >>
>>>> >> It's a bug in the design, not the code.
>>>>
>>>> > Because *nobody* wants to follow symlinks when doing "rm -rf". Even if
>>>> > they think they do, they *really* don't.
>>>>
>>>> I agree 100%.  Even time I use this function, I worry briefly about
>>>> whether it follows symlinks, then think to myself "no, no-one would be so
>>>> stupid to implement that deliberately in a publically available API".  So it
>>>> was a real shock to discover in this thread that I was wrong, and
>>>> furthermore that the function is documented as doing the wrong thing.  We
>>>> should fix both spec and implementation, as soon as possible.
>>>>
>>>> Regards,
>>>>     Malcolm
>>>> _______________________________________________
>>>> Libraries mailing list
>>>> [hidden email]
>>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Libraries mailing list
>>> [hidden email]
>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>>
>>>
>>> _______________________________________________
>>> Libraries mailing list
>>> [hidden email]
>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>
>> _______________________________________________
>> Libraries mailing list
>> [hidden email]
>> http://www.haskell.org/mailman/listinfo/libraries
>>
>
>
> _______________________________________________
> Libraries mailing list
> [hidden email]
> http://www.haskell.org/mailman/listinfo/libraries
>


_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries


_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries



_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: removeDirectoryRecursive should not follow symlinks

Dan Burton
+1 as proposed.

There is no known concrete use case which takes advantage of the "follow symlinks" behavior. There are some known use cases where symlinks are simply expected not to be present as a precondition. Using this function as though it were rm -rf (a very natural thing to do, imo) has the potential for disastrous results.

For these reasons, I say treat it as a bug. No deprecation cycle. Existing code using the library doesn't change and becomes automatically safer just by upgrading the dependency.

-- Dan Burton

On Thu, Jan 8, 2015 at 4:06 PM, Greg Weber <[hidden email]> wrote:
We all agree that we should do something about this. For 99% of use cases this is a bug fix.
So I would like to just leave it up to the implementer to decide what to do.


On Wed, Jan 7, 2015 at 9:49 AM, Gabriel Gonzalez <[hidden email]> wrote:
Actually, I might retract my recommendation to provided it under a different name.  I'm neutral on this now and would consider it okay to change in place.

My reasoning is that this is a bug fix, so we can reasonably expect users to put lower bounds on software in response to bug fixes like we do with other software.


On 1/7/15, 1:47 AM, Johan Tibell wrote:

I don't think so but if we change the function signature or name as some suggested it all needs to be cpped still.

On Jan 6, 2015 9:39 PM, "Erik Hesselink" <[hidden email]> wrote:
Does cabal rely on this behavior?

Erik

On Tue, Jan 6, 2015 at 9:37 PM, Johan Tibell <[hidden email]> wrote:
> Who volunteers to fix the breakages in Cabal and add all the needed CPP?
>
> On Tue, Jan 6, 2015 at 2:45 PM, David Feuer <[hidden email]> wrote:
>>
>> This seems reasonable, but if we have a deprecation cycle, the old name
>> should (temporarily) be a synonym for the new one, and the deprecation
>> warning should indicate that code intended to work with older versions needs
>> to be audited.
>>
>> On Jan 6, 2015 2:40 PM, "Gabriel Gonzalez" <[hidden email]> wrote:
>>>
>>> I think it's safer to remove the old function altogether (perhaps after
>>> one deprecation cycle) and provide a new one under a different name, rather
>>> than modify it in place.
>>>
>>> Modifying it in place risks the behavior that others mentioned where your
>>> program is unsafe to compile against older library versions.  Yes, the user
>>> could explicitly enforce that by putting a lower bound on the library, but
>>> most users won't even realize that they need to do that.
>>>
>>>
>>> On 1/6/15, 11:37 AM, Edward Kmett wrote:
>>>
>>> I'm +1 for fixing this, in place, on the current function.
>>>
>>> The specification we have here is doing a very very bad thing and needs
>>> to be fixed, not slavishly copied forward because someone sometime once made
>>> a mistake.
>>>
>>> The current behavior grievously violates the expectations of anyone who
>>> would be in a situation to go and reach for it and has any prior experience
>>> with any other such tool.
>>>
>>> -Edward
>>>
>>>
>>>
>>> On Tue, Jan 6, 2015 at 11:14 AM, Malcolm Wallace <[hidden email]>
>>> wrote:
>>>>
>>>>
>>>> On 6 Jan 2015, at 14:59, Bardur Arantsson wrote:
>>>>
>>>> > On 2015-01-06 14:57, Mike Meyer wrote:
>>>> >> On Tue, Jan 6, 2015 at 7:48 AM, Johan Tibell <[hidden email]>
>>>> >> wrote:
>>>> >>
>>>> >>> This is not a bugfix. A bug is failing to follow the functions
>>>> >>> specification, which *does* include following symlinks.
>>>> >>>
>>>> >>
>>>> >> It's a bug in the design, not the code.
>>>>
>>>> > Because *nobody* wants to follow symlinks when doing "rm -rf". Even if
>>>> > they think they do, they *really* don't.
>>>>
>>>> I agree 100%.  Even time I use this function, I worry briefly about
>>>> whether it follows symlinks, then think to myself "no, no-one would be so
>>>> stupid to implement that deliberately in a publically available API".  So it
>>>> was a real shock to discover in this thread that I was wrong, and
>>>> furthermore that the function is documented as doing the wrong thing.  We
>>>> should fix both spec and implementation, as soon as possible.
>>>>
>>>> Regards,
>>>>     Malcolm
>>>> _______________________________________________
>>>> Libraries mailing list
>>>> [hidden email]
>>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> Libraries mailing list
>>> [hidden email]
>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>>
>>>
>>> _______________________________________________
>>> Libraries mailing list
>>> [hidden email]
>>> http://www.haskell.org/mailman/listinfo/libraries
>>>
>>
>> _______________________________________________
>> Libraries mailing list
>> [hidden email]
>> http://www.haskell.org/mailman/listinfo/libraries
>>
>
>
> _______________________________________________
> Libraries mailing list
> [hidden email]
> http://www.haskell.org/mailman/listinfo/libraries
>


_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries


_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries



_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries



_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries
Reply | Threaded
Open this post in threaded view
|

Re: Proposal: removeDirectoryRecursive should not follow symlinks

John Wiegley-2
>>>>> Dan Burton <[hidden email]> writes:

> For these reasons, I say treat it as a bug. No deprecation cycle. Existing
> code using the library doesn't change and becomes automatically safer just
> by upgrading the dependency.

+1 as proposed, and +1 on treating it as a bug.

John
_______________________________________________
Libraries mailing list
[hidden email]
http://www.haskell.org/mailman/listinfo/libraries
123