Skip to content

Commit

Permalink
Make compiler.include_dirs expose all paths correct again for MSVC
Browse files Browse the repository at this point in the history
including the SDK dirs after compiler.initialize()

Changes in pypa#153 broke compatibility (pywin32), used shadowed class
variables in an odd manner and conceiled the MSVC dirs from public
access.
  • Loading branch information
kxrob committed Aug 20, 2022
1 parent c802880 commit 45d5e83
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 39 deletions.
12 changes: 3 additions & 9 deletions distutils/_msvccompiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,14 +224,6 @@ def __init__(self, verbose=0, dry_run=0, force=0):
self.plat_name = None
self.initialized = False

@classmethod
def _configure(cls, vc_env):
"""
Set class-level include/lib dirs.
"""
cls.include_dirs = cls._parse_path(vc_env.get('include', ''))
cls.library_dirs = cls._parse_path(vc_env.get('lib', ''))

@staticmethod
def _parse_path(val):
return [dir.rstrip(os.sep) for dir in val.split(os.pathsep) if dir]
Expand All @@ -250,12 +242,14 @@ def initialize(self, plat_name=None):
# Get the vcvarsall.bat spec for the requested platform.
plat_spec = PLAT_TO_VCVARS[plat_name]

# Add include/lib dirs from vcvarsall.bat
vc_env = _get_vc_env(plat_spec)
if not vc_env:
raise DistutilsPlatformError(
"Unable to find a compatible " "Visual Studio installation."
)
self._configure(vc_env)
self.include_dirs.extend(self._parse_path(vc_env.get('include', '')))
self.library_dirs.extend(self._parse_path(vc_env.get('lib', '')))

self._paths = vc_env.get('path', '')
paths = self._paths.split(os.pathsep)
Expand Down
16 changes: 0 additions & 16 deletions distutils/ccompiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,16 +93,6 @@ class CCompiler:
}
language_order = ["c++", "objc", "c"]

include_dirs = []
"""
include dirs specific to this compiler class
"""

library_dirs = []
"""
library dirs specific to this compiler class
"""

def __init__(self, verbose=0, dry_run=0, force=0):
self.dry_run = dry_run
self.force = force
Expand Down Expand Up @@ -395,9 +385,6 @@ def _fix_compile_args(self, output_dir, macros, include_dirs):
else:
raise TypeError("'include_dirs' (if supplied) must be a list of strings")

# add include dirs for class
include_dirs += self.__class__.include_dirs

return output_dir, macros, include_dirs

def _prep_compile(self, sources, output_dir, depends=None):
Expand Down Expand Up @@ -454,9 +441,6 @@ def _fix_lib_args(self, libraries, library_dirs, runtime_library_dirs):
else:
raise TypeError("'library_dirs' (if supplied) must be a list of strings")

# add library dirs for class
library_dirs += self.__class__.library_dirs

if runtime_library_dirs is None:
runtime_library_dirs = self.runtime_library_dirs
elif isinstance(runtime_library_dirs, (list, tuple)):
Expand Down
11 changes: 6 additions & 5 deletions distutils/command/build_ext.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,11 +314,6 @@ def run(self): # noqa: C901
force=self.force,
)
customize_compiler(self.compiler)
# If we are cross-compiling, init the compiler now (if we are not
# cross-compiling, init would not hurt, but people may rely on
# late initialization of compiler even if they shouldn't...)
if os.name == 'nt' and self.plat_name != get_platform():
self.compiler.initialize(self.plat_name)

# And make sure that any compile/link-related options (which might
# come from the command-line or from the setup script) are set in
Expand All @@ -342,6 +337,12 @@ def run(self): # noqa: C901
if self.link_objects is not None:
self.compiler.set_link_objects(self.link_objects)

# If we are cross-compiling, init the compiler now (if we are not
# cross-compiling, init would not hurt, but people may rely on
# late initialization of compiler even if they shouldn't...)
if os.name == 'nt' and self.plat_name != get_platform():
self.compiler.initialize(self.plat_name)

# Now actually compile and link everything.
self.build_extensions()

Expand Down
41 changes: 32 additions & 9 deletions distutils/tests/test_ccompiler.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,21 @@
import os
import io
import sys
import platform
import textwrap
import sysconfig
import tempfile
from test import support

import pytest

from distutils import ccompiler
from distutils.core import Distribution
from distutils.extension import Extension
from distutils.command.build_ext import build_ext


is_windows = platform.system() == "Windows"


def _make_strs(paths):
Expand All @@ -22,8 +31,7 @@ def _make_strs(paths):
def c_file(tmp_path):
c_file = tmp_path / 'foo.c'
gen_headers = ('Python.h',)
is_windows = platform.system() == "Windows"
plat_headers = ('windows.h',) * is_windows
plat_headers = ('stdlib.h',) + ('Windows.h',) * is_windows
all_headers = gen_headers + plat_headers
headers = '\n'.join(f'#include <{header}>\n' for header in all_headers)
payload = (
Expand All @@ -40,16 +48,31 @@ def c_file(tmp_path):
return c_file


def test_set_include_dirs(c_file):
# @pytest.mark.skipif(not is_windows)
def test_include_dirs(c_file):
"""
Extensions should build even if set_include_dirs is invoked.
In particular, compiler-specific paths should not be overridden.
Check basic standard include dirs to be available, including SDK on Windows.
"""
compiler = ccompiler.new_compiler()
python = sysconfig.get_paths()['include']
compiler.set_include_dirs([python])
compiler.add_include_dir(python)
compiler.compile(_make_strs([c_file]))

# do it again, setting include dirs after any initialization
compiler.set_include_dirs([python])
compiler.compile(_make_strs([c_file]))
# ~ return

ext = Extension('foo', _make_strs([c_file]))
dist = Distribution({'name': 'foo', 'ext_modules': [ext]})
cmd = build_ext(dist)
tmp_dir = tempfile.mkdtemp()
cmd.build_lib = tmp_dir
cmd.build_temp = tmp_dir

old_stdout = sys.stdout
if not support.verbose:
# silence compiler output
sys.stdout = io.StringIO()
try:
cmd.ensure_finalized()
cmd.run()
finally:
sys.stdout = old_stdout

0 comments on commit 45d5e83

Please sign in to comment.