python - link statically against vc_redist

[K0lb3]

This PR makes the python setup link against vc_runtime statically if msvc is used as compiler.
As cibuildwheel uses msvc on Windows, this PR solves #782 for good.
#782 is still an active issue and was closed despite it not being solved,
besides basically forcing all dependents to have to write about Windows users having to vc_redist.

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[eustas]

@anthrotype this PR looks reasonable, WDYT?

Basically, brotli library is free standing and dependency on something is more an artifact of MSVC compiler; so static link is in the line of the initial intent...

[K0lb3]

An alternative solution to this problem would be using in the cibuildwheel workflow for fixing the Windows wheels.

  CIBW_REPAIR_WHEEL_COMMAND_WINDOWS: delvewheel repair -w {dest_dir} {wheel}

the only problem with this is the building of the win arm64 wheels,
as the arm64 wheels gets cross-compiled on x86, so the path to the vc_runtime has to be added to the command in a pretty hacky way.

          CIBW_BUILD: "cp39-win_arm64"
          CIBW_REPAIR_WHEEL_COMMAND_WINDOWS: >-
            delvewheel repair -w {dest_dir} {wheel} --add-path "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Redist\MSVC\14.40.33807\arm64\Microsoft.VC143.CRT"

This is what I currently use in one of my projects with multiple precompiled libs in the wheel.
Static binding isn't a good solution there, as it would increase the size of the wheel too much due to multiple statically linked vc_runtimes.

[anthrotype]

I'm not an expert on MSVC or Windows in general, but I don't think statically linking the Microsfot C runtime library is the right thing to do for brotli, I fear this might cause other unwanted issues.
I suggest we check and see what other comparable projects who ship C/C++ extension modules for Windows are doing.

[K0lb3]

I suggest we check and see what other comparable projects who ship C/C++ extension modules for Windows are doing.

Usually they either use mingw for compiling the wheels, statically link it, or well, leave it dangling like currently in brotli, causing issues for Windows users.
e.g. Pillow uses static linking on Windows if mingw isn't used.
Delvewheel puts the used vc_runtime into the wheel, which more or less produces the same result as static linking.

[K0lb3]

As you don't want any static linking, then the mingw usage would have to be reworked as well, as it also uses static linking.

See setup.py#L109.

You're right about the static linking against vc_runtime, Microsoft themselves advice against it, and say that programs should just make the user install the corresponding runtime....

Mingw seems to bypass the issue slightly by linking against private Windows apis, but that doesn't seem like a good solution either...

[K0lb3]

I digged a bit deeper into this.

On Windows, since Python 3.9, Python versions contains the vc_runtime they were build with and put them into the python installation directory.
So for these version the vc_runtime issue doesn't exist as long as vc_runtime140 was used for the native extension, which seems to have been used for all Python versions so far.

For older Python versions, usually 3.6 to 3.8 this is not the case.
Their installer installs VC Redistributable, solving the issue, but if a user or the workflow system use the embedded version, then vc_runtime might not be available.

So the whole vc_runtime issue only exists for Windows Python <= 3.8.
Embedded Python <= 3.8 aren't that rare though, as programs tend to embed older Python versions, and then usually use the embedded version.

[anthrotype]

thanks for digging deeper on this. Have you actually encountered this issue yourself? What python and windows version was that?

the vc_runtime issue doesn't exist as long as vc_runtime140 was used for the native extension

I'm not sure exactly which MSVC version was used to build the brotli wheels to be honest, we use this repo https://github.com/google/brotli-wheels to build and publish the wheels to pypi and over there we set up Github CI to use windows-latest hosted runner, I wonder if that's using a too new compiler compared to the one used to build python itself which in turn makes brotli extension module require a newer or additional msvc runtime dlls from those already packaged with the python runtime.

So the whole vc_runtime issue only exists for Windows Python <= 3.8.

3.8 just reached end of life https://devguide.python.org/versions/, maybe brotli should stop supporting those old pythons

[K0lb3]

Have you actually encountered this issue yourself? What python and windows version was that?

I encountered it once or twice some years back myself.
I think it was once on a Windows runner and another time within some program that embedded python, but I'm not sure anymore which program that was. I got some questions / error reports regarding this on my own project that depends on brotli.
Overall it's indeed a pretty rare issue, as more or less every program one installs on Windows will install the runtime.

I wonder if that's using a too new compiler compared to the one used to build python itself which in turn makes brotli extension module require a newer or additional msvc runtime dlls from those already packaged with the python runtime.

As brotli only depends on memset, memmove, memcpy and two internal commands from vc_runtime, this shouldn't really be an issue, as these are constant along all vc_runtime versions. From what I could gather the runtime basically just maps this to the internal Windows api of the local system, while allowing custom user hooks.
There are other issues in regards to the compiler and runtime version which could case issues though, e.g. mutex constructors.

3.8 just reached end of life https://devguide.python.org/versions/, maybe brotli should stop supporting those old pythons

Just my two cents about this, as this is entirely up to you and the other maintainers.
I've been thinking about this for my own projects myself, as it can be a bit of a hassle, especially for type-hints, but overall I think it doesn't hurt to support older versions as long as it doesn't cause too much trouble.
As the runtime issue is rare, and relatively easy to investigate for most people, I think that it's fine to keep supporting the older versions, as it doesn't cause that much overheard.

[K0lb3]

Before today, I also didn't know about the exact msvc runtime situation, especially not that Python 3.9+ ships it by default.
As 3.8 officially reached its end of life, this PR can be closed, as it does more bad than good for most users.
Alternatively the setup.py could conditionally link msvc runtime statically if cibuildwheel gets used and the Python version is below 3.8.