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

Various setup fixes #564

Merged
merged 1 commit into from Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 0 additions & 25 deletions setup.cfg

This file was deleted.

36 changes: 29 additions & 7 deletions setup.py
Expand Up @@ -32,6 +32,7 @@
"Programming Language :: Python :: 3.7",
"Programming Language :: Python :: 3.8",
"Programming Language :: Python :: 3.9",
"Programming Language :: Python :: 3.10",
"Programming Language :: Python :: Implementation :: CPython",
"Programming Language :: Python :: Implementation :: PyPy",
"Topic :: Software Development :: Libraries :: Python Modules",
Expand Down Expand Up @@ -63,11 +64,15 @@
"""


import sys, os, os.path, platform, warnings
import sys, os, os.path, pathlib, platform, shutil, tempfile, warnings

# for newer setuptools, enable the embedded distutils before importing setuptools/distutils to avoid warnings
os.environ['SETUPTOOLS_USE_DISTUTILS'] = 'local'

from distutils import log
from setuptools import setup, Command, Distribution as _Distribution, Extension as _Extension
from setuptools.command.build_ext import build_ext as _build_ext
# NB: distutils imports must remain below setuptools to ensure we use the embedded version
from distutils import log
from distutils.errors import DistutilsError, CompileError, LinkError, DistutilsPlatformError

with_cython = False
Expand Down Expand Up @@ -246,11 +251,28 @@ def finalize_options(self):
def run(self):
build_cmd = self.get_finalized_command('build')
build_cmd.run()
sys.path.insert(0, build_cmd.build_lib)
sys.path.insert(0, 'tests/lib')
import test_all
if not test_all.main([]):
raise DistutilsError("Tests failed")

# running the tests this way can pollute the post-MANIFEST build sources
# (see https://github.com/yaml/pyyaml/issues/527#issuecomment-921058344)
# until we remove the test command, run tests from an ephemeral copy of the intermediate build sources
tempdir = tempfile.TemporaryDirectory(prefix='test_pyyaml')
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we put this in a with?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's the way I originally had it, but since Windows has the .pyd locked after the tests run (since it's loaded in the current process), the deletion of the tempdir on the CM exit blows an exception that's a lot harder to deal with when guarding the entire CM block. This way, only the cleanup is "best effort".

Copy link
Member Author

Choose a reason for hiding this comment

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

Ultimately this whole "test from setup.py" thing needs to just go away and be replaced by pytest, but I didn't get around to dusting off my custom pytest discovery wrapper for pyyaml's "special" tests, and hiding the entire thing inside pytest as a single opaque test blob is worse.


try:
# have to create a subdir since we don't get dir_exists_ok on copytree until 3.8
temp_test_path = pathlib.Path(tempdir.name) / 'pyyaml'
shutil.copytree(build_cmd.build_lib, temp_test_path)
sys.path.insert(0, str(temp_test_path))
sys.path.insert(0, 'tests/lib')

import test_all
if not test_all.main([]):
raise DistutilsError("Tests failed")
finally:
try:
# this can fail under Windows; best-effort cleanup
tempdir.cleanup()
except Exception:
pass


cmdclass = {
Expand Down