On Mon, Jan 5, 2015 at 5:33 PM, Greg Weber wrote:

How about being backwards-compatible friendly by adding a new function with the friendly behavior, adding a deprecation notice to the existing function, and putting the existing function under a new name that signifies the -rf? That would put me at a +1

To be honest, that it followed symlinks in the first place is arguably a severe bug and also violates people's expectations. I suspect most existing users would either be (a) applying it to something they created with a known safe structure, so the change is irrelevant, or (b) horrified at the unexpected lurking bug. Functions like this should not follow symlinks. -- brandon s allbery kf8nh sine nomine associates allbery.b@gmail.com ballbery@sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

Sure, I'm happy for someone to put this forward as a counterproposal. My reasoning for changing the meaning of the identifier: I'm pretty sure almost everyone using the current function doesn't actually want to follow symlinks. To actually verify this I'd have to do some Hackage spelunking. Edward Excerpts from Greg Weber's message of 2015-01-05 14:33:41 -0800:

How about being backwards-compatible friendly by adding a new function with the friendly behavior, adding a deprecation notice to the existing function, and putting the existing function under a new name that signifies the -rf? That would put me at a +1

On Mon, Jan 5, 2015 at 2:31 PM, David Feuer wrote:

...

+1. Following symlinks in such a potentially-destructive operation is fundamentally wrong.

On Mon, Jan 5, 2015 at 5:25 PM, Edward Z. Yang wrote:

...

Discussion period: one month

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.

This would bring its behavior inline with rm -rf.

I also opened a ticket here: https://ghc.haskell.org/trac/ghc/ticket/9266

Thanks, Edward _______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

---

Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

+1 to change implementation as proposed. On 05.01.2015 23:46, Bardur Arantsson wrote:

On 2015-01-05 23:25, Edward Z. Yang wrote:

...

Discussion period: one month

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.

This would bring its behavior inline with rm -rf.

I also opened a ticket here: https://ghc.haskell.org/trac/ghc/ticket/9266

Thanks, Edward

+1 as is.

-1 on adding a deprecation cycle unless someone can come up with a REALLY good reason for that.

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

-- Andreas Abel <>< Du bist der geliebte Mensch. Department of Computer Science and Engineering Chalmers and Gothenburg University, Sweden andreas.abel@gu.se http://www2.tcs.ifi.lmu.de/~abel/

On Mon, Jan 5, 2015 at 5:54 PM, Johan Tibell wrote:

Aside: can we look at what other languages with similar functions do?

You will find that essentially all other implementations do the right thing and not follow symlinks, because the other behavior is a severe bug. I really do not understand why anyone believes the current behavior is defensible. -- brandon s allbery kf8nh sine nomine associates allbery.b@gmail.com ballbery@sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

On Mon, Jan 5, 2015 at 5:51 PM, David Feuer wrote:

In case some people don't understand just how horrible this is: imagine a privileged user uses this to erase a user's home directory. It could easily hit a symbolic link to a critical system directory and hose the whole machine.

I think it's potentially even worse than that: this subtle behavior could easily be abused for this exact scenario by a hostile entity. Imagine a scenario where an attacker may have permission to place a symlink somewhere that a privileged process recursively removes; then your / (or any number of things) goes 'boom'. This could easily happen through a combination of a race condition (say, placing junk files in /tmp you later remove, and the attacker races to place the symlink there) and an improper umask setting. Or if the attacker simply may be part of a group that allows access to something a program will remove, etc etc. I agree this behavior is fundamentally wrong, and I'm strongly +1 on changing the existing behavior, which I think is the only sane thing to do. Otherwise any existing callers of this function in old code could very easily do The Wrong Thing if they don't get the memo. I have no opinion on warnings or adding a function that preserves the old behavior.

On Jan 5, 2015 6:44 PM, "Brandon Allbery" wrote:

...

On Mon, Jan 5, 2015 at 5:54 PM, Johan Tibell wrote:

...

Aside: can we look at what other languages with similar functions do?

You will find that essentially all other implementations do the right thing and not follow symlinks, because the other behavior is a severe bug. I really do not understand why anyone believes the current behavior is defensible.

-- brandon s allbery kf8nh sine nomine associates allbery.b@gmail.com ballbery@sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

-- Regards, Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/

I'm concerned that changing the behavior of the existing function would make it too easy to write vulnerable programs when compiled with older GHCs. Having a new safe function along with a deprecation warning on the old one would clue people in and avoid functionality varying subtly/dangerously based on the compiler used. On Mon, Jan 5, 2015, 5:17 PM Johan Tibell wrote:

Let me make a wider comment about backwards compatibility. Many successful programming languages (e.g. Java) *never* break backwards compatibility. They deprecate (and only if the old API is too error prone for the programmer) and add a new API. In my opinion breaking backwards compatibility is almost never worth it*. Our libraries are already full of #ifdefs and maintaining our core libraries (which I maintain some of) is a headache because the code gets worse every time we "clean it up".

* And it's only worth it sometimes because we're still a relatively small language, by usage.

On Mon, Jan 5, 2015 at 8:08 PM, Austin Seipp wrote:

...

On Mon, Jan 5, 2015 at 5:51 PM, David Feuer wrote:

...

In case some people don't understand just how horrible this is: imagine a privileged user uses this to erase a user's home directory. It could easily hit a symbolic link to a critical system directory and hose the whole machine.

I think it's potentially even worse than that: this subtle behavior could easily be abused for this exact scenario by a hostile entity. Imagine a scenario where an attacker may have permission to place a symlink somewhere that a privileged process recursively removes; then your / (or any number of things) goes 'boom'. This could easily happen through a combination of a race condition (say, placing junk files in /tmp you later remove, and the attacker races to place the symlink there) and an improper umask setting. Or if the attacker simply may be part of a group that allows access to something a program will remove, etc etc.

I agree this behavior is fundamentally wrong, and I'm strongly +1 on changing the existing behavior, which I think is the only sane thing to do. Otherwise any existing callers of this function in old code could very easily do The Wrong Thing if they don't get the memo.

I have no opinion on warnings or adding a function that preserves the old behavior.

...

On Jan 5, 2015 6:44 PM, "Brandon Allbery" wrote:

...

On Mon, Jan 5, 2015 at 5:54 PM, Johan Tibell wrote:

...

Aside: can we look at what other languages with similar functions do?

You will find that essentially all other implementations do the right thing and not follow symlinks, because the other behavior is a severe

bug. I

...

really do not understand why anyone believes the current behavior is defensible.

-- brandon s allbery kf8nh sine nomine associates allbery.b@gmail.com ballbery@sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

-- Regards,

Austin Seipp, Haskell Consultant Well-Typed LLP, http://www.well-typed.com/ _______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

On Mon, Jan 5, 2015 at 5:28 PM, Brandon Allbery wrote:

On Mon, Jan 5, 2015 at 8:22 PM, Eric Mertens wrote:

...

I'm concerned that changing the behavior of the existing function would make it too easy to write vulnerable programs when compiled with older GHCs. Having a new safe function along with a deprecation warning on the old one would clue people in and avoid functionality varying subtly/dangerously based on the compiler used.

Only really helpful if you can go back and retrofit that deprecation into already deployed older versions. Also, it's using the obvious name for the function. So the correctly working one needs to have some unobvious name and a 'warning you should not use this, use some_other_function instead' form this point on?

I don't know why 'removeDirectoryRecursive' would be considered by someone to be the only obvious name available. `removeDirectoryTree` would work also. Also, the warning does not have to be placed on older versions for it to have effect with older versions. The point is that the library author will be taught not to use this function at all. When I suggested deprecation, I assumed that following a symlink was a desirable behavior for someone. If it is not and it is 100% the case that this behavior is a defect, then this is just a bugfix then deprecation is not needed. However, since this is considered dangerous, I think Eric makes an excellent point that it makes it possible for the function to be used properly for one compilation of a library that depends on `directory`, but for another compilation to pick up an older version of `directory`. That means that just fixing the behavior in the newest version of `directory` is not very satisfactory. So I see 2 approaches to address these issues 1) the deprecation warning approach 2) produce an updated point releases with the bugfix for every major version of directory. It still could be a good idea to do the deprecation warning on top of this because there are still older versions of the function out there with the bad behavior.

Indeed, the Java community does do things that way. One of many reasons the Java ecosystem is an absolute, irredeemable mess.

-- brandon s allbery kf8nh sine nomine associates allbery.b@gmail.com ballbery@sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

On 2015-01-06 08:03, Ganesh Sittampalam wrote:

On 06/01/2015 05:30, Greg Weber wrote:

...

When I suggested deprecation, I assumed that following a symlink was a desirable behavior for someone. If it is not and it is 100% the case that this behavior is a defect, then this is just a bugfix then deprecation is not needed.

My general feeling is that it is just a bug.

That's what I thought too -- it's a typical rookie mistake to forget to check if "isDirecory?" will return "true" for symlinks to directories. But the documentation actually states the expected behavior correctly -- it's not nearly explicit enough about how dangerous it is, but the documentation is technically correct. However, even so, this is CVE-worthy behavior on its own (as pointed out by Brandon), and should be removed pronto. Perhaps with new minor versions for all affected major versions (excellent point by Greg Weber), depending on how much work that is.

On Mon, Jan 5, 2015 at 8:16 PM, Johan Tibell wrote:

Let me make a wider comment about backwards compatibility.

The community has already messed up backward compatibility in far more obvious and wide-reaching ways, for far worse reasons, many times. Suddenly deciding to stand on principle, in a case where the current behavior is *clearly* wrong and dangerous, seems "off" to me. Of course, you're welcome to do it... I'll just have to make sure it's understood that the decision was made to enshrine actively dangerous behavior. (I could make an argument for this being CVE-worthy. In fact, Austin Seipp has already made most of it. Should a major security hole also be protected and propagated by a sudden need to stand on backward compatibility principles that have never been given much more than lip service in the past?) -- brandon s allbery kf8nh sine nomine associates allbery.b@gmail.com ballbery@sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

On 2015-01-06 02:16, Johan Tibell wrote:

Let me make a wider comment about backwards compatibility. Many successful programming languages (e.g. Java) *never* break backwards compatibility.

That is not quite correct. They're certainly averse to it, but for example the JDK developers changed String's fundamental behavior wrt. whether "substring" should copy the relevant string slice or whether it should just point into the original string. You might argue that the semantics didn't change, but this change *did* (predictably) break quite a few programs which suddenly experienced pathological memory allocation behavior and crashed with OoM's where they had none before.

They deprecate (and only if the old API is too error prone for the programmer) and add a new API. In my opinion breaking backwards compatibility is almost never worth it*. Our libraries are already full of #ifdefs and maintaining our core libraries (which I maintain some of) is a headache because the code gets worse every time we "clean it up".

* And it's only worth it sometimes because we're still a relatively small language, by usage.

I don't disagree with the general point, but this in this case we're talking about absurdly dangerous and incorrect behavior which (as Austin points out is trivially exploitable by messing around in /tmp and waiting until e.g. a "clean-tmp" cron jobs starts running). AFAICT there isn't even a function that *does the right thing* in System.Directory! If someone wants this crazy behavior they can damn well code it themselves ;) Regards,

This is not a bugfix. A bug is failing to follow the functions specification, which *does* include following symlinks. On Tue, Jan 6, 2015 at 7:16 AM, Alois Cochard wrote:

+1 for fixing this

-1 for the deprecation cycle

I don't think it's sane to rely on the current behavior anyway.

Cheers

On 5 January 2015 at 23:25, Edward Z. Yang wrote:

...

Discussion period: one month

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.

This would bring its behavior inline with rm -rf.

I also opened a ticket here: https://ghc.haskell.org/trac/ghc/ticket/9266

Thanks, Edward _______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

-- *Λ\ois* http://twitter.com/aloiscochard http://github.com/aloiscochard

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

On 2015-01-06 14:57, Mike Meyer wrote:

On Tue, Jan 6, 2015 at 7:48 AM, Johan Tibell 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. The API it provides is useful, but dangerous and less useful than the safe version.

Breaking the API has been proposed. If we're going to do that, why not add a followSymlinks ::Bool argument as well, and document why the API was broken in this way. Going forward, at least. Not sure about backwards.

Because *nobody* wants to follow symlinks when doing "rm -rf". Even if they think they do, they *really* don't. Any design which relies on it is probably fundamentally broken. (If they're *really really* sure, they can write their own.) Btw, a Bool argumnent usually isn't a good idea. Better API design would be data SymlinkBehavior = FollowSymlinks | NoFollowSymlinks (so that it's easy to tell at all call sites what it's doing.). However, without default arguments this is forcing a pointless decision upon the user, i.e. 99.9%+ of all usages should be using NoFollow, so why force the user of the API to decide? Even with such a change, I don't think the functionality should exist in a "standard" library. (I won't repeat all the arguments; see the rest of the thread :)) Regards,

On 2015-01-06 16:43, Mike Meyer wrote:

On Tue, Jan 6, 2015 at 8:59 AM, Bardur Arantsson wrote:

...

Because *nobody* wants to follow symlinks when doing "rm -rf". Even if they think they do, they *really* don't. Any design which relies on it is probably fundamentally broken. (If they're *really really* sure, they can write their own.)

Don't need to reread the arguments - I've been aware of them since CSRG introduced symlinks in 4BSD, and the utilities didn't know about them in the beta releases.

You pretty much never want to call it on a file tree you didn't build. But if I built the file tree, then I know what's going on.

Indeed, but even in *that* case it's incredibly error-prone...

Making me write my own is probably going to result in less robust code, or copying the code out of the library and tweaking it, neither of which is desirable.

It's such a rare need and so dangerous that I'd argue that forcing a copy + tweak is actually desirable in this case. Regards,

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 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

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 mailto:malcolm.wallace@me.com> 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 mailto:johan.tibell@gmail.com> 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 Libraries@haskell.org mailto:Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

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 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" 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 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 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 Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing listLibraries@haskell.orghttp://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

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 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 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" 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 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 > 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 Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

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" mailto:hesselink@gmail.com> wrote:

Does cabal rely on this behavior?

Erik

On Tue, Jan 6, 2015 at 9:37 PM, Johan Tibell mailto:johan.tibell@gmail.com> 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 mailto:david.feuer@gmail.com> 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" mailto:gabriel439@gmail.com> 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 mailto:malcolm.wallace@me.com> >>> 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 mailto:johan.tibell@gmail.com> >>>> >> 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 >>>> Libraries@haskell.org mailto:Libraries@haskell.org >>>> http://www.haskell.org/mailman/listinfo/libraries >>> >>> >>> >>> >>> _______________________________________________ >>> Libraries mailing list >>> Libraries@haskell.org mailto:Libraries@haskell.org >>> http://www.haskell.org/mailman/listinfo/libraries >>> >>> >>> >>> _______________________________________________ >>> Libraries mailing list >>> Libraries@haskell.org mailto:Libraries@haskell.org >>> http://www.haskell.org/mailman/listinfo/libraries >>> >> >> _______________________________________________ >> Libraries mailing list >> Libraries@haskell.org mailto:Libraries@haskell.org >> http://www.haskell.org/mailman/listinfo/libraries >> > > > _______________________________________________ > Libraries mailing list > Libraries@haskell.org mailto:Libraries@haskell.org > http://www.haskell.org/mailman/listinfo/libraries >

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

+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 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 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" wrote:

...

Does cabal rely on this behavior?

Erik

...

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 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" 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,

...

...

...

could explicitly enforce that by putting a lower bound on the

...

...

...

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 < malcolm.wallace@me.com> 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 < johan.tibell@gmail.com> > >> 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

On Tue, Jan 6, 2015 at 9:37 PM, Johan Tibell wrote: the user library, but thing. We

...

...

...

> should fix both spec and implementation, as soon as possible. > > Regards, > Malcolm > _______________________________________________ > Libraries mailing list > Libraries@haskell.org > http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing listLibraries@haskell.orghttp://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

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 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 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" 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

...

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 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 Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing listLibraries@haskell.orghttp://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries

_______________________________________________ Libraries mailing list Libraries@haskell.org http://www.haskell.org/mailman/listinfo/libraries