Skip to content

Make setuptools optional at runtime. #8476

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.

Sign up for GitHub

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

Merged
sklam merged 5 commits into from
Mar 23, 2023
Merged

Make setuptools optional at runtime. #8476

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2b94898
Make setuptools optional at runtime.
stuartarchibald Sep 30, 2022
c3ca38c
Fix flake8 issues.
stuartarchibald Sep 30, 2022
e3bb289
Remove libm linkage in pycc for Windows.
stuartarchibald Sep 30, 2022
ce5aa59
Merge remote-tracking branch 'main' into wip/setuptools_optional_runtime
stuartarchibald Mar 22, 2023
4c3948b
Move assignments below imports for flake8
stuartarchibald Mar 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions numba/pycc/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

import warnings
from numba.core.errors import NumbaPendingDeprecationWarning
# The pycc module requires setuptools.
try:
import setuptools
except ImportError:
msg = "The 'setuptools' package is required at runtime for pycc support."
raise ImportError(msg)

# Public API
from .cc import CC
Expand Down
9 changes: 6 additions & 3 deletions numba/pycc/cc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
from distutils import dir_util, log
from distutils.command import build_ext
from distutils.extension import Extension
from setuptools import distutils as dutils
dir_util = dutils.dir_util
log = dutils.log

@flying-sheep flying-sheep Oct 11, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not like this?

Suggested change
from setuptools import distutils as dutils
dir_util = dutils.dir_util
log = dutils.log
from setuptools.distutils import dir_util, log

@stuartarchibald stuartarchibald Oct 11, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking a look. The issue is:

>>> from setuptools.distutils import dir_util, log
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ModuleNotFoundError: No module named 'setuptools.distutils'

@flying-sheep flying-sheep Oct 12, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, it’s a not really a submodule, it’s a re-export.

But since setuptools just directly imports distutils, what’s the advantage of importing it from there instead of just doing import distutils?

If the intention is to to not import distutils because it’s deprecated, you need to actually remove the import instead of importing the same thing using a more roundabout path. Should be easy enough, there doesn’t seem to be any good reason to import either symbol from it: log is just a logging.Logger (you can just use another logger I guess) and dir_util.mkpath(...) provides the same functionality as pathlib.Path(...).mkdir(parents=True, exists_ok=True)

@stuartarchibald stuartarchibald Oct 14, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are good points, thanks for raising them.

I see, it’s a not really a submodule, it’s a re-export.

But since setuptools just directly imports distutils, what’s the advantage of importing it from there instead of just doing import distutils?

I was looking to use the setuptools vendored distutils but think I have done this incorrectly. It's probably setuptools._distutils.

If the intention is to to not import distutils because it’s deprecated, you need to actually remove the import instead of importing the same thing using a more roundabout path. Should be easy enough, there doesn’t seem to be any good reason to import either symbol from it: log is just a logging.Logger (you can just use another logger I guess) and dir_util.mkpath(...) provides the same functionality as pathlib.Path(...).mkdir(parents=True, exists_ok=True)

This patch is trying to do the absolute minimum necessary to make pycc not use distutils and instead use setuptools's internal copy, and then to make sure that setuptools an optional runtime dependency. Given pycc is scheduled for deprecation in the next Numba release (xref: #8509), I'm not sure that it is worth spending time on finding a full replacement for distutils, then writing and testing it, though can probably make the changes RE: log and mkpath as noted.

@sklam sklam Mar 22, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: move the assignments below all import for flake8

@stuartarchibald stuartarchibald Mar 22, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4c3948b.

from setuptools.command import build_ext
from setuptools.extension import Extension

import os
import shutil
import sys
Expand Down
82 changes: 10 additions & 72 deletions numba/pycc/platform.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
from distutils.ccompiler import CCompiler, new_compiler
from distutils.command.build_ext import build_ext
from distutils.sysconfig import customize_compiler
from distutils import log

import setuptools
from setuptools.command.build_ext import build_ext
from setuptools.dist import Distribution
import numpy as np

import functools
Expand All @@ -13,6 +11,13 @@
from contextlib import contextmanager
from pathlib import Path

# Wire in distutils components from setuptools
CCompiler = setuptools.distutils.ccompiler.CCompiler
new_compiler = setuptools.distutils.ccompiler.new_compiler
customize_compiler = setuptools.distutils.sysconfig.customize_compiler
log = setuptools.distutils.log


_configs = {
# DLL suffix, Python C extension suffix
'win': ('.dll', '.pyd'),
Expand Down Expand Up @@ -78,16 +83,13 @@ def __init__(self):
if not external_compiler_works():
self._raise_external_compiler_error()

# Need to import it here since setuptools may monkeypatch it
from distutils.dist import Distribution
self._verbose = False
self._compiler = new_compiler()
customize_compiler(self._compiler)
self._build_ext = build_ext(Distribution())
self._build_ext.finalize_options()
self._py_lib_dirs = self._build_ext.library_dirs
self._py_include_dirs = self._build_ext.include_dirs

np_compile_args = {'include_dirs': [np.get_include(),],}
if sys.platform == 'win32':
np_compile_args['libraries'] = []
Expand Down Expand Up @@ -195,70 +197,6 @@ def get_ext_filename(self, ext_name):
return self._build_ext.get_ext_filename(ext_name)


#
# Patch Numpy's exec_command() to avoid random crashes on Windows in test_pycc
# see https://github.com/numpy/numpy/pull/7614
# and https://github.com/numpy/numpy/pull/7862
#

def _patch_exec_command():
# Patch the internal worker _exec_command()
import numpy.distutils.exec_command as mod
orig_exec_command = mod._exec_command
mod._exec_command = _exec_command


def _exec_command(command, use_shell=None, use_tee=None, **env):
"""
Internal workhorse for exec_command().
Code from https://github.com/numpy/numpy/pull/7862
"""
if use_shell is None:
use_shell = os.name == 'posix'
if use_tee is None:
use_tee = os.name == 'posix'

executable = None

if os.name == 'posix' and use_shell:
# On POSIX, subprocess always uses /bin/sh, override
sh = os.environ.get('SHELL', '/bin/sh')
if _is_sequence(command):
command = [sh, '-c', ' '.join(command)]
else:
command = [sh, '-c', command]
use_shell = False

elif os.name == 'nt' and _is_sequence(command):
# On Windows, join the string for CreateProcess() ourselves as
# subprocess does it a bit differently
command = ' '.join(_quote_arg(arg) for arg in command)

# Inherit environment by default
env = env or None
try:
proc = subprocess.Popen(command, shell=use_shell, env=env,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
universal_newlines=True)
except OSError:
# Return 127, as os.spawn*() and /bin/sh do
return '', 127
text, err = proc.communicate()
# Only append stderr if the command failed, as otherwise
# the output may become garbled for parsing
if proc.returncode:
if text:
text += "\n"
text += err
# Another historical oddity
if text[-1:] == '\n':
text = text[:-1]
if use_tee:
print(text)
return proc.returncode, text


def _quote_arg(arg):
"""
Quote the argument for safe use in a shell command line.
Expand Down
8 changes: 3 additions & 5 deletions numba/tests/pycc_distutils_usecase/setup_distutils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
from distutils.core import setup

from setuptools import distutils
from source_module import cc

from numba.pycc.platform import _patch_exec_command

setup = distutils.core.setup


def run_setup():
# Avoid sporadic crashes on Windows due to MSVCRT spawnve()
_patch_exec_command()
setup(ext_modules=[cc.distutils_extension()])


Expand Down
7 changes: 3 additions & 4 deletions numba/tests/pycc_distutils_usecase/setup_distutils_nested.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
from distutils.core import setup
from setuptools import distutils

from nested.source_module import cc

from numba.pycc.platform import _patch_exec_command

setup = distutils.core.setup


def run_setup():
# Avoid sporadic crashes on Windows due to MSVCRT spawnve()
_patch_exec_command()
setup(ext_modules=[cc.distutils_extension()])


Expand Down
4 changes: 0 additions & 4 deletions numba/tests/pycc_distutils_usecase/setup_setuptools.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,8 @@

from source_module import cc

from numba.pycc.platform import _patch_exec_command


def run_setup():
# Avoid sporadic crashes on Windows due to MSVCRT spawnve()
_patch_exec_command()
setup(ext_modules=[cc.distutils_extension()])


Expand Down
4 changes: 0 additions & 4 deletions numba/tests/pycc_distutils_usecase/setup_setuptools_nested.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,8 @@

from nested.source_module import cc

from numba.pycc.platform import _patch_exec_command


def run_setup():
# Avoid sporadic crashes on Windows due to MSVCRT spawnve()
_patch_exec_command()
setup(ext_modules=[cc.distutils_extension()])


Expand Down
11 changes: 11 additions & 0 deletions numba/tests/support.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,17 @@ def expected_failure_py311(fn):
needs_subprocess = unittest.skipUnless(_exec_cond, "needs subprocess harness")


try:
import setuptools
has_setuptools = True
except ImportError:
has_setuptools = False


# decorator for a test that need setuptools
needs_setuptools = unittest.skipUnless(has_setuptools, 'Test needs setuptools')


def ignore_internal_warnings():
"""Use in testing within a ` warnings.catch_warnings` block to filter out
warnings that are unrelated/internally generated by Numba.
Expand Down
8 changes: 1 addition & 7 deletions numba/tests/test_extending.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import multiprocessing
import ctypes
import warnings
from distutils.version import LooseVersion
import re

import numpy as np
Expand Down Expand Up @@ -61,12 +60,7 @@


try:
import scipy

if LooseVersion(scipy.__version__) < "0.19":
sc = None
else:
import scipy.special.cython_special as sc
import scipy.special.cython_special as sc
except ImportError:
sc = None

Expand Down
4 changes: 3 additions & 1 deletion numba/tests/test_pycc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

from numba.core import utils
from numba.tests.support import (TestCase, tag, import_dynamic, temp_directory,
has_blas)
has_blas, needs_setuptools)

import unittest


Expand Down Expand Up @@ -88,6 +89,7 @@ def check_c_ext(self, extdir, name):
sys.modules.pop(name, None)


@needs_setuptools
class TestCC(BasePYCCTest):

def setUp(self):
Expand Down