[llvm] r187874 - Add support for linking against a curses library when available and

James Molloy james at jamesmolloy.co.uk
Thu Aug 8 02:44:00 PDT 2013


Hey Chandler,

Apologies if I've come across strong (it was late when I wrote my last
reply), but I don't understand why you feel it necessary to remove the old
code as a fallback codepath in the case where curses is not compiled in.

I come from the perspective that there needs to be a really good reason for
adding an extra mandatory dependency. If it's an optional dependency, i.e.
"things work better if you have this", I don't have a problem at all. With
this patch you're ripping out core functionality unless this extra
dependency is around (and also, while everyone has curses.so not every
builder has libncurses-dev installed for the headers...). I just don't see
why this is necessary. But I'm repeating myself.

> The old algorithm wasn't flaky and didn't have "a" false positive. It
assumed that all terminals except for one supported colors. This is pretty
obviously false to me, and we have users that are specifically complaining
about that assumption so I assume it is false in practice.

It doesn't seem that bad of a heuristic to me, and if it's a problem then
just compile against curses.

To repeat: I'm not objecting to the part of your patch that queries curses
to check for color support. Not at all. I'm just objecting to the removal
of the old algorithm if curses isn't available.

James


On 8 August 2013 01:01, Chandler Carruth <chandlerc at google.com> wrote:

> You seem to feel very strongly about this, and I'd like to understand the
> context in addition to the specifics of our argument. Is this a problem in
> practice for you or anyone else you know of? Do you have a concrete example
> where Clang now does the wrong thing on a particular system?
>
> On Wed, Aug 7, 2013 at 2:42 PM, James Molloy <james at jamesmolloy.co.uk>wrote:
>
>> The difference between zlib and curses is that the dependency on zlib was
>> added to support a *new feature*. Without it, you don't get the new
>> feature. Fair enough.
>>
>
> This may be a new feature to Clang, but it was not really new. This was
> absolutely necessary for compatibility with GCC and other things.
>
>
>> I think it's unreasonable to gate an already existing feature of the user
>> interface on a new dependency when there's no clear reason. OK, the old
>> algorithm was flaky and had a false positive. So if I, a user or packager,
>> care about that I can just now link against libcurses. Sorted.
>>
>> So why remove the old fallback algorithm when those who care about the
>> false positive have a clear and obvious new workaround?
>>
>
> The old algorithm wasn't flaky and didn't have "a" false positive. It
> assumed that all terminals except for one supported colors. This is pretty
> obviously false to me, and we have users that are specifically complaining
> about that assumption so I assume it is false in practice.
>
> I don't have any objection to adding fallback special cases for terminals
> we know do or don't support colors. (Hence my question about practical
> cases that we should handle here) I do have an objection to assuming colors
> in the absence of any evidence to indicate that assumption is safe. That
> objection is based on the fact that showing colors when they are not valid
> causes significantly more harm to the user experience than failing to show
> colors when they are valid.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130808/3ef5cd51/attachment.html>


More information about the llvm-commits mailing list