uv icon indicating copy to clipboard operation
uv copied to clipboard

Patch additional sysconfig values such as clang at install time

Open samypr100 opened this issue 1 year ago • 5 comments

Summary

Minor follow up to https://github.com/astral-sh/uv/pull/9905 to patch clang with cc.

Implements the replacements used in sysconfigpatcher, namely

DEFAULT_VARIABLE_UPDATES = {
    "CC": WordReplace("clang", "cc"),
    "CXX": WordReplace("clang++", "c++"),
    "BLDSHARED": WordReplace("clang", "cc"),
    "LDSHARED": WordReplace("clang", "cc"),
    "LDCXXSHARED": WordReplace("clang++", "c++"),
    "LINKCC": WordReplace("clang", "cc"),
    "AR": "ar",
}

Test Plan

Added an additional test. Tested local python installs.

Related traces

TRACE Updated `AR` from `/tools/clang-linux64/bin/llvm-ar` to `ar`
TRACE Updated `CC` from `clang -pthread` to `cc -pthread`
TRACE Updated `CXX` from `clang++ -pthread` to `c++ -pthread`
TRACE Updated `BLDSHARED` from `clang -pthread -shared -L/tools/deps/lib` to `cc -pthread -shared -L/tools/deps/lib`
TRACE Updated `LDSHARED` from `clang -pthread -shared -L/tools/deps/lib` to `cc -pthread -shared -L/tools/deps/lib`
TRACE Updated `LDCXXSHARED` from `clang++ -pthread -shared` to `c++ -pthread -shared`
TRACE Updated `LINKCC` from `clang -pthread` to `cc -pthread

Pending Discussion Items

https://github.com/astral-sh/uv/pull/9905#issuecomment-2543879587

samypr100 avatar Dec 15 '24 15:12 samypr100

I would be surprised if we replaced clang with cc on a system with clang but not cc. Is that silly?

zanieb avatar Dec 15 '24 16:12 zanieb

I would be surprised if we replaced clang with cc on a system with clang but not cc. Is that silly?

Fair, on the other hand I rather find cc to be more pervasive than clang. Even on cases where say gcc is not installed but clang is, cc would often point to clang.

I think this approach would also help systems that have gcc installed (or others) but not clang as cc would be interopable with either, unless we really want users under all cases to have clang installed on their system which is harder in some environments where you can't control the toolchains provided.

samypr100 avatar Dec 15 '24 17:12 samypr100

@mitsuhiko did say that this was the most impactful of the sysconfig patches so tagging him.

charliermarsh avatar Dec 15 '24 18:12 charliermarsh

A relevant comment at https://github.com/indygreg/python-build-standalone/issues/210#issuecomment-2029007939 too

zanieb avatar Dec 16 '24 17:12 zanieb

A relevant comment at indygreg/python-build-standalone#210 (comment) too

Good insights, at least from my perspective I see a unique distinction between python-build-standalone goals and uv with regards to these types of modifications. For example, I can see the strong desire in python-build-standalone to maintain ground truth / pristine nature of the builds (avoid modifying anything beyond what's absolutely necessary) for a lot of reasons. Given that uv is a consumer of python-build-standalone, I think uv is the ideal place to make these type of minor modifications to the downloads in order to achieve the desired level of interopability with the end user's system.

samypr100 avatar Dec 17 '24 02:12 samypr100

What if we test for clang, and rewrite to gcc if it's not present?

charliermarsh avatar Dec 17 '24 19:12 charliermarsh

Why prefer clang / gcc over cc? Just wondering. I think it is pretty common for it to be linked?

zanieb avatar Dec 17 '24 19:12 zanieb

Oh sorry. I guess cc is fine. What's the problem with just using cc?

charliermarsh avatar Dec 17 '24 19:12 charliermarsh

Is there one? haha

It might break my system, I alias that to cargo check 😭 but I kind of imagine build commands don't invoke a shell.

I think cc is really reasonable, honestly.

zanieb avatar Dec 17 '24 20:12 zanieb