Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] -requiresAny is not recognized by Visual Studio 2017 < v15.6 #3946

Closed
AndrewTsao opened this issue Jun 9, 2023 · 12 comments · Fixed by #3950
Closed

[BUG] -requiresAny is not recognized by Visual Studio 2017 < v15.6 #3946

AndrewTsao opened this issue Jun 9, 2023 · 12 comments · Fixed by #3950
Labels
bug Needs Triage Issues that need to be evaluated for severity and status.

Comments

@AndrewTsao
Copy link

setuptools version

setuptools==67.8.0

Python version

Python 3.09, 3.10, 3.11

OS

Windows Server 2012r2

Additional environment information

No response

Description

Hi,

I encountered an error when using setup.py to compile a module related to pybind11. The error message was pybind11\detail/common.h(86): fatal error C1189: #error: pybind11 2.10+ requires MSVC 2017 or newer. But I actually had VS2017 installed. Later, I found that the _msvccompiler.py module in setuptools seemed to be highly optimized, causing the _get_vc_env function to always return the environment variables of MSVC14. And I tried to add some print statements in that function, but nothing was printed, proving that the function was never called. Finally, I renamed _get_vc_env to _get_vc_env1, and setup.py worked fine.

I tried to print the name of the _get_env_vc function, and the actual output was “<function msvc14_get_vc_env at 0xxxxxxxx>.”, which looked like a function that was optimized for a specific compiler.

Expected behavior

setuptools can dected vs2017 environment.

How to Reproduce

  1. Call a python setup.py to compile c++ extension.

Output

\.py3.11\lib\site-packages\pybind11\include\pybind11\detail/common.h(86): fatal error C1189: #error:  pybind11 2.10+ requires MSVC 2017 or newer
@AndrewTsao AndrewTsao added bug Needs Triage Issues that need to be evaluated for severity and status. labels Jun 9, 2023
@abravalheri
Copy link
Contributor

abravalheri commented Jun 9, 2023

Hi @AndrewTsao, thank you very much for writing this report.

Is there any chance you can expand a little bit more in the reproducer. Right now it is a bit difficult to understand what is going on. A good methodology for how to write reproducers is the following: https://stackoverflow.com/help/minimal-reproducible-example.


Also could you please include the commands of how you are building your package? Does it behave the same way if you use python -m build (calling python setup.py directly is deprecated).


Please note that the naming MSVC14 used in setuptools is not exactly related to the year of release of the toolkit.

You can see in the file below that this number seems to be related to the runtime library version instead:

Improved support for Microsoft Visual C++ compilers.
Known supported compilers:
--------------------------
Microsoft Visual C++ 14.X:
Microsoft Visual C++ Build Tools 2015 (x86, x64, arm)
Microsoft Visual Studio Build Tools 2017 (x86, x64, arm, arm64)
Microsoft Visual Studio Build Tools 2019 (x86, x64, arm, arm64)
This may also support compilers shipped with compatible Visual Studio versions.


This is probably how setuptools is finding the latest installation of the toolkit:

"""Python 3.8 "distutils/_msvccompiler.py" backport
Returns "15, path" based on the result of invoking vswhere.exe
If no install is found, returns "None, None"
The version is returned to avoid unnecessarily changing the function
result. It may be ignored when the path is not None.
If vswhere.exe is not available, by definition, VS 2017 is not
installed.
"""
root = environ.get("ProgramFiles(x86)") or environ.get("ProgramFiles")
if not root:
return None, None
try:
path = subprocess.check_output([
join(root, "Microsoft Visual Studio", "Installer", "vswhere.exe"),
"-latest",
"-prerelease",
"-requiresAny",
"-requires", "Microsoft.VisualStudio.Component.VC.Tools.x86.x64",
"-requires", "Microsoft.VisualStudio.Workload.WDExpress",
"-property", "installationPath",
"-products", "*",
]).decode(encoding="mbcs", errors="strict").strip()
except (subprocess.CalledProcessError, OSError, UnicodeDecodeError):
return None, None
path = join(path, "VC", "Auxiliary", "Build")
if isdir(path):
return 15, path
return None, None

Do you know what the vswhere command is returning in your system (and if the VC/Auxiliary/Build directory exists inside of it)?

@abravalheri abravalheri added the Needs Repro Issues that need a reproducible example. label Jun 9, 2023
@AndrewTsao
Copy link
Author

My virtualenv:

e7f60e52ba1fb64c0c26b583faa77d6

My setup.py file:

from setuptools import setup, Extension
import numpy as np

__version__ = '0.0.1'

requires = ['numpy']

setup_requires = ['pybind11','wheel']

class get_pybind_include(object):
    """Helper class to determine the pybind11 include path
    The purpose of this class is to postpone importing pybind11
    until it is actually installed, so that the ``get_include()``
    method can be invoked. """

    def __init__(self, user=False):
        self.user = user

    def __str__(self):
        import pybind11
        return pybind11.get_include(self.user)

ext_modules = [
    Extension(
        'example',
        ['src/main.cpp'],
        include_dirs=[
            # Path to pybind11 headers
            get_pybind_include(),
            get_pybind_include(user=True),
            np.get_include()
        ],
        language='c++',
        # 如果是调试模式,添加以下选项。
        extra_compile_args=['/Od', '/Zi'],
        extra_link_args=['/DEBUG:FULL']
    ),
]

setup(
    name='example',
    ext_modules=ext_modules,
    install_requires=requires,
    setup_requires=setup_requires,
    platforms='win_amd64',
    python_requires='>=2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*'
)

`src/main.cc'

#include <Python.h>
#define PY_ARRAY_UNIQUE_SYMBOL MYLIBRARY_ARRAY_API
#define PY_UFUNC_UNIQUE_SYMBOL MYLIBRARY_UFUNC_API
#include <numpy/arrayobject.h>
#include <pybind11/numpy.h>
#include <pybind11/pybind11.h>
#include <time.h>

#include <iostream>
#include <string>
#include <vector>
namespace py = pybind11;
using namespace py::literals;

struct Point
{
  int x;
  int y;
  double z;
};

struct Points
{
  Points()
  {
    points.resize(10);
    for (int i = 0; i < 10; i++) {
      points[i].x = i;
      points[i].y = i;
      points[i].z = i * 6.6;
    }
  }

  ~Points()
  {
    std::cout << (void*)this << ": Points bye, p:" << (void*)points.data() << std::endl;
  }

  std::vector<Point> points;
  py::buffer_info Buffer();
};

py::buffer_info Points::Buffer()
{
  auto buf = py::buffer_info(
      points.data(),                          /* Pointer to buffer */
      sizeof(Point),                          /* Size of one scalar */
      py::format_descriptor<Point>::format(), /* Python struct-style format descriptor */
      1,                                      /* Number of dimensions */
      { 10 },                 /* Buffer dimensions */
      {             /* Strides (in bytes) for each index */
        sizeof(Point) }
  );
  return buf;
}

PYBIND11_MODULE(example, m) {

  PYBIND11_NUMPY_DTYPE(Point, x, y, z);
  py::class_<Points>(m, "Points", py::buffer_protocol())
   .def_buffer([](Points &m) -> py::buffer_info {return m.Buffer();});

  m.doc() = "test pybind11 module";
  import_array1();
}

build command: python setup.py bdist, get error:

image

@AndrewTsao
Copy link
Author

Interactive call _get_vc_env and _find_vc2017.

image

@AndrewTsao
Copy link
Author

After I rename _get_vc_env to _get_vc_env1, call it in python console

image

image

this is result before rename _get_vc_env,

image

image

@abravalheri
Copy link
Contributor

abravalheri commented Jun 9, 2023

Hi @AndrewTsao, can you try running python -m build1 instead of python setup.py bdist? I don't think this command is supposed to work like that after the implementations of PEP 517.

The thing about renaming _get_vc_env is more complicated... setuptools._distutils is not supposed to be accessed directly. The functions there are patched with the functions in setuptools.msvc. If you want to debug these functions, please run import setuptools; import distutils._msvccompiler and use the functions from distutils not setuptools._distutils, this will guarantee you are getting the same thing setuptools uses during the build.

The patch for _get_vc_env, is the following:

def _msvc14_get_vc_env(plat_spec):
"""Python 3.8 "distutils/_msvccompiler.py" backport"""
if "DISTUTILS_USE_SDK" in environ:
return {
key.lower(): value
for key, value in environ.items()
}
vcvarsall, vcruntime = _msvc14_find_vcvarsall(plat_spec)
if not vcvarsall:
raise distutils.errors.DistutilsPlatformError(
"Unable to find vcvarsall.bat"
)
try:
out = subprocess.check_output(
'cmd /u /c "{}" {} && set'.format(vcvarsall, plat_spec),
stderr=subprocess.STDOUT,
).decode('utf-16le', errors='replace')
except subprocess.CalledProcessError as exc:
raise distutils.errors.DistutilsPlatformError(
"Error executing {}".format(exc.cmd)
) from exc
env = {
key.lower(): value
for key, _, value in
(line.partition('=') for line in out.splitlines())
if key and value
}
if vcruntime:
env['py_vcruntime_redist'] = vcruntime
return env

Here you can see that setuptools will use a _msvc14_find_vcvarsall instead of _find_vcvarsall...
The functions in both implementations (setuptools and distutils), differ in very subtle ways, but in general setuptools considers more edge cases that distutils. Probably the key here is the different vswhere commands that setuptools and distutils run.

If you compare distuils._msvccompiler._find_vc2017 and setuptools.msvc._msvc14_find_vcvarsall, you can see that setuptools include a few extra flags to vswhere (-requiresAny -requires Microsoft.VisualStudio.Workload.WDExpress). There are also other conditions that setuptools checks...

Footnotes

  1. You need to install https://pypa-build.readthedocs.io/en/stable/ and configure pyproject.toml accordingly (https://pybind11.readthedocs.io/en/stable/compiling.html).

@abravalheri abravalheri removed the Needs Repro Issues that need a reproducible example. label Jun 9, 2023
@AndrewTsao
Copy link
Author

Yes, you are right!

I try call setuptools.msvc module functions, maybe get reason.

image

I build a vswhere command and run, got results.

image

@abravalheri
Copy link
Contributor

abravalheri commented Jun 9, 2023

It seems that the -requiresAny parameter was added in version 2.3 (according to this text: https://github.com/Microsoft/vswhere/wiki/Find-VSTest).

So probably what is happening is that setuptools is currently not supporting the version of vswhere you have installed in your system.

Now we have to trace down why the author of the patch in setuptools added this parameter and what edge cases it covers... If we can simply remove it, or if it is worth to implement a workaround.

I don't know what is the relationship between the versions of the VC toolkit and vswhere and how you ended up with an older version installed.

@abravalheri
Copy link
Contributor

abravalheri commented Jun 9, 2023

It seems the -requiresAny argument was added in #2663 to support Visual Studio Express.

@abravalheri
Copy link
Contributor

abravalheri commented Jun 9, 2023

If we see the release pages for vswhere, we find out that -requiresAny was added in v2.3.2, released on 23 Jan 2018.

By correlating the date with the Visual Studio 2017 releases, we can say that the earliest possible Visual Studio to include this (or a later) version of vswhere (supporting -requiresAny) is version 15.5.5. But it is more likely that the update was made in version 15.6.

This means that setuptools currently is not compatible with Visual Studio 2017 with versions prior to 15.6.

I asked the original author of the PR about suggestions for how to tackle this problem.

@abravalheri abravalheri changed the title [BUG] The _get_vc_env function in _msvccompiler.py is precompiled and cannot recognize the VS2017 environment [BUG] -requiresAny is not recognized by Visual Studio 2017 < v15.6 Jun 12, 2023
@abravalheri
Copy link
Contributor

Hi @AndrewTsao, could you please confirm if the PR #3950 would solve your problem?
I believe you can install the version in the PR with:

pip install https://github.com/abravalheri/setuptools/archive/refs/heads/issue-3946.zip

@AndrewTsao
Copy link
Author

In my environment, the problem was solved after pip install setuptools-issue-3956.zip. Thank you for your efforts.

@abravalheri
Copy link
Contributor

abravalheri commented Jun 13, 2023

Thank you very much for trying it out Andrew.

Let's see if we can have feedback from the other maintainers about the PR, so it can land on the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Needs Triage Issues that need to be evaluated for severity and status.
Projects
None yet
2 participants