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

Removing deprecated distutils module + using vswhere instead of distutils.ccompiler to properly detect all valid MSVC compilers #311

Closed
eXhumer opened this issue Sep 21, 2021 · 1 comment

Comments

@eXhumer
Copy link

eXhumer commented Sep 21, 2021

distutils has been deprecated from Python 3.10 and is scheduled to be removed from Python 3.12. The current use of distutils in library is in setup.py is to determine the MSVC compiler generator string for CMake. However, the method used to determine MSVC compiler doesn't work properly if there are multiple MSVC compilers installed. With MSVC2019 & MSVC2022 installed, the library thinks I don't have a valid MSVC compiler and fails to compile properly. I have came up with an updated method to use subprocess and vswhere to check for valid MSVC compiler. If I can, I would like to open a PR to add this change to the library.

Updated method to determine generator string for CMake

def determine_generator_args():
    if sys.platform == 'win32':
        pf_x86_path = pathlib.Path(os.getenv("ProgramFiles(x86)"))
        vswhere_path = pf_x86_path.joinpath("Microsoft Visual Studio", "Installer", "vswhere.exe")

        if vswhere_path.is_file():
            vswhere_process = subprocess.Popen(
                [
                    str(vswhere_path),
                    "-products",
                    "*",
                    "-requires",
                    "Microsoft.VisualStudio.Component.VC.Tools.x86.x64",
                    "-format",
                    "json",
                    "-nologo",
                ],
                stdout=subprocess.PIPE,
            )

            compilers_data = json.loads(vswhere_process.communicate()[0])
            vs_version = None
            vs_year = None

            for compiler_data in compilers_data:
                compiler_version = int(compiler_data["catalog"]["productDisplayVersion"].split(".")[0])
                compiler_year = int(compiler_data["catalog"]["productLineVersion"])

                if compiler_year in [2015, 2017, 2019]:
                    if vs_year is None or compiler_year > vs_year:
                        vs_year = compiler_year
                        vs_version = compiler_version

            assert(vs_version and vs_year)

            print('Using Visual Studio', vs_version, vs_year)

            vs_version_gen_str = "Visual Studio {} {}".format(vs_version, vs_year)

            if vs_year <= 2017:
                # For VS2017 and earlier, architecture goes at end of generator string
                if is_64bit():
                    vs_version_gen_str += " Win64"
                return ['-G', vs_version_gen_str]

            # For VS2019 (and presumably later), architecture is passed via -A flag
            arch_str = "x64" if is_64bit() else "Win32"
            return ['-G', vs_version_gen_str, '-A', arch_str]

    return []
@jmklix
Copy link
Member

jmklix commented Dec 14, 2023

Thanks for taking the time to work on this and sorry for taking a long time to get back to you. I don't think we would accept this PR as it needs to work across multiple platforms, and it looks like your fix would only work for windows machines. It looks like setuptools is the better replacement for distutils.ccompiler. I'm closing this issue for now, but please let us know if you have other suggestions to improve this repo.

@jmklix jmklix closed this as completed Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants