Remove top-level dependency on ansicolors

[thejcannon]

(First PR towards #19282)

Removes the top-level dependence on ansicolors by shifting the code we need into our own utility (the code is simple enough that providing ansicolors through a Pex, or using a native function provided by the engine is unnecessary).

$ pants dependents 3rdparty/python#ansicolors
3rdparty/python:python
testprojects/src/python/hello/greet/greet.py
testprojects/src/python/native/setup.py:lib
tests/python/pants_test/pantsd/pantsd_integration_test_base.py:pantsd_integration_test_base

(NOTE ansicolors most likely will still be a transitive dependency, this is just step 1)

[kaos]

(First PR towards #19285)

guess you meant #19282

[benjyw]

Specifically for ansicolors this is fine since it really is a tiny amount of code. But even then, why? I don't think we have agreement on the principle of #19282 yet.

[benjyw]

A wise man once told me: "Features are an asset, code is a liability". By that standard, post this change, we are net even on features, but with another file's worth of liability.

[thejcannon]

Those features don't apparate from nothing. They come from code. So they have a liability. Even worse, it's code you can't directly control. So it has a higher liability. The library might include more code than you need. Liability. The library might depend on other libraries. Liability.

We also pass those liabilities onto our consumers.

I'm not saying we don't rely on other libraries. I won't be doing HTTP from scratch. But depending on them has a cost, and for a build tool that cost has a multiplicative effect

[benjyw]

Those features don't apparate from nothing. They come from code. So they have a liability. Even worse, it's code you can't directly control. So it has a higher liability. The library might include more code than you need. Liability. The library might depend on other libraries. Liability.

Yes, but that's code that we don't have to maintain. For example, you copied the ansicolors implementation, but not its tests. Now someone comes along in the future, makes a tweak to our copy, there are no tests, so things go awry and we don't know about it at test time.

So, maybe we copy the tests? Now that's more tests to run, and even more code to maintain.

[thejcannon]

I actually didn't copy the code. 🙂

But tests are a good idea. Unfortunately we can't test how it actually looks on a terminal. But we can do a verification once and then bless the strings the tests use.

[thejcannon]

I acknowledge the larger point though. And I want to reiterate, I LOVE libraries. I gobble them like pacman gobbles yellow dots (do they have a name?).

But Pants is a build system, not your everyday CLI app. Therefore it needs to be treated very delicately when it comes to things like security and impact. Infecting an app with malware is one thing. But infecting a build system means the possibility of infecting anything it builds.

[kaos]

I agree with Benjy regarding the maintainability aspect. In-sourcing libaries adds to our maintenance. The net "cost" of the feature is unchanged, but we've shifted who'll pay for it.

I also agree with Josh on minimizing our dependence on 3rdparty libs (as much as sensible). So, there's a line of trade offs to be walked, I think for a case by case basis. For ansicolors in particular, it may be worth to take ownership of the maintenance cost given the small amount of rarely changing (stable) code.
But we need to stay vigilant for each and every library we want to consider to in-source like this, as to not bog down our ability to move forward.

[benjyw]

OTOH ansicolors is extremely unlikely to collide with some third-party plugin's requirements, is tiny, is stable, and takes ~no time to resolve. So the lowest-hanging fruit is also the least tasty...

[benjyw]

There will be no escaping the fact that plugins must run on a specific interpreter (the one we ship with scie-pants) and be compatible with some specific requirements. So your plugins will almost certainly need their own lockfile. We can selectively reduce the blast radius of the requirements problem by removing requirements, but then I wouldn't start with ansicolors necessarily. Although also no harm in starting there as a proof-of-concept I suppose.

Maybe rather than talk about this as an abstract principle, we should audit our requirements.txt and pick specific requirements that we think are the most troubling? We certainly wouldn't have picked ansicolors but I'm not sure what we would have picked.

[thejcannon]

I don't actually plan on in-sourcing, the plan was/is to shift to the engine or PEX. This just happened because the amount of code is tiny so it didn't seem like it was worth either of the other options. 🤷‍♂️

I can audit our reqs and suggest the right shift.

[thejcannon]

@benjyw given our discussion in the Pants meetup, where do you stand on this PR?

[benjyw]

@benjyw given our discussion in the Pants meetup, where do you stand on this PR?

I'm mildly negative on it, since this is now code we have to maintain (and we're not testing), whereas ansicolors is a trivial download that is rarely updated, and that is very unlikely to cause issues for plugin developers.

But I won't stand in the way either.