diff --git a/test/lib/ansible_test/_internal/python_requirements.py b/test/lib/ansible_test/_internal/python_requirements.py index fc88b637c2b..506b802c526 100644 --- a/test/lib/ansible_test/_internal/python_requirements.py +++ b/test/lib/ansible_test/_internal/python_requirements.py @@ -251,6 +251,13 @@ def collect_requirements( # installed packages may have run-time dependencies on setuptools uninstall_packages.remove('setuptools') + # hack to allow the package-data sanity test to keep wheel in the venv + install_commands = [command for command in commands if isinstance(command, PipInstall)] + install_wheel = any(install.has_package('wheel') for install in install_commands) + + if install_wheel: + uninstall_packages.remove('wheel') + commands.extend(collect_uninstall(packages=uninstall_packages)) return commands diff --git a/test/sanity/code-smell/package-data.py b/test/sanity/code-smell/package-data.py index 6df34ed254b..7a81b7597a9 100644 --- a/test/sanity/code-smell/package-data.py +++ b/test/sanity/code-smell/package-data.py @@ -1,338 +1,184 @@ +"""Verify the contents of the built sdist and wheel.""" from __future__ import annotations import contextlib import fnmatch -import glob import os import pathlib -import re import shutil import subprocess import sys import tarfile import tempfile - -import packaging.version +import typing as t +import zipfile from ansible.release import __version__ -def assemble_files_to_ship(complete_file_list): - """ - This looks for all files which should be shipped in the sdist - """ - # All files which are in the repository except these: +def collect_sdist_files(complete_file_list: list[str]) -> list[str]: + """Return a list of files which should be present in the sdist.""" ignore_patterns = ( - # Developer-only tools '.azure-pipelines/*', - '.github/*', - '.github/*/*', - 'changelogs/fragments/*', - 'hacking/*', - 'hacking/*/*', - 'test/results/.tmp/*', - 'test/results/.tmp/*/*', - 'test/results/.tmp/*/*/*', - 'test/results/.tmp/*/*/*/*', - 'test/results/.tmp/*/*/*/*/*', + '.cherry_picker.toml', '.git*', - ) - ignore_files = frozenset(( - # Developer-only tools + '.mailmap', 'changelogs/README.md', 'changelogs/config.yaml', - '.cherry_picker.toml', - '.mailmap', - )) + 'changelogs/fragments/*', + 'hacking/*', + ) - # These files are generated and then intentionally added to the sdist + sdist_files = [path for path in complete_file_list if not any(fnmatch.fnmatch(path, ignore) for ignore in ignore_patterns)] - # Misc - misc_generated_files = [ + egg_info = ( 'PKG-INFO', - ] - - shipped_files = misc_generated_files + 'SOURCES.txt', + 'dependency_links.txt', + 'entry_points.txt', + 'not-zip-safe', + 'requires.txt', + 'top_level.txt', + ) - for path in complete_file_list: - if path not in ignore_files: - for ignore in ignore_patterns: - if fnmatch.fnmatch(path, ignore): - break - else: - shipped_files.append(path) + sdist_files.append('PKG-INFO') + sdist_files.extend(f'lib/ansible_core.egg-info/{name}' for name in egg_info) - return shipped_files + return sdist_files -def assemble_files_to_install(complete_file_list): - """ - This looks for all of the files which should show up in an installation of ansible - """ - ignore_patterns = ( - # Tests excluded from sdist - ) +def collect_wheel_files(complete_file_list: list[str]) -> list[str]: + """Return a list of files which should be present in the wheel.""" + wheel_files = [] - pkg_data_files = [] for path in complete_file_list: - - if path.startswith("lib/ansible"): + if path.startswith('lib/ansible/'): prefix = 'lib' - elif path.startswith("test/lib/ansible_test"): + elif path.startswith('test/lib/ansible_test/'): prefix = 'test/lib' else: continue - for ignore in ignore_patterns: - if fnmatch.fnmatch(path, ignore): - break - else: - pkg_data_files.append(os.path.relpath(path, prefix)) - - return pkg_data_files - + wheel_files.append(os.path.relpath(path, prefix)) -@contextlib.contextmanager -def clean_repository(file_list): - """Copy the repository to clean it of artifacts""" - # Create a tempdir that will be the clean repo - with tempfile.TemporaryDirectory() as repo_root: - directories = {repo_root + os.path.sep} - - for filename in file_list: - # Determine if we need to create the directory - directory = os.path.dirname(filename) - dest_dir = os.path.join(repo_root, directory) - if dest_dir not in directories: - os.makedirs(dest_dir) - - # Keep track of all the directories that now exist - path_components = directory.split(os.path.sep) - path = repo_root - for component in path_components: - path = os.path.join(path, component) - if path not in directories: - directories.add(path) - - # Copy the file - shutil.copy2(filename, dest_dir, follow_symlinks=False) - - yield repo_root - - -def create_sdist(tmp_dir): - """Create an sdist in the repository""" - # Make sure a changelog exists for this version when testing from devel. - # When testing from a stable branch the changelog will already exist. - version = packaging.version.Version(__version__) - pathlib.Path(f'changelogs/CHANGELOG-v{version.major}.{version.minor}.rst').touch() - - create = subprocess.run( - [sys.executable, '-m', 'build', '--sdist', '--no-isolation', '--outdir', tmp_dir], - stdin=subprocess.DEVNULL, - capture_output=True, - text=True, - check=False, + dist_info = ( + 'COPYING', + 'METADATA', + 'RECORD', + 'WHEEL', + 'entry_points.txt', + 'top_level.txt', ) - stderr = create.stderr - stdout = create.stdout - - if create.returncode != 0: - raise Exception('make snapshot failed:\n%s' % stderr + '\n' + stdout) - - # Determine path to sdist - tmp_dir_files = os.listdir(tmp_dir) + wheel_files.append(f'ansible_core-{__version__}.data/scripts/ansible-test') + wheel_files.extend(f'ansible_core-{__version__}.dist-info/{name}' for name in dist_info) - if not tmp_dir_files: - raise Exception('sdist was not created in the temp dir') - elif len(tmp_dir_files) > 1: - raise Exception('Unexpected extra files in the temp dir') + return wheel_files - return os.path.join(tmp_dir, tmp_dir_files[0]) +@contextlib.contextmanager +def clean_repository(complete_file_list: list[str]) -> t.Generator[str, None, None]: + """Copy the files to a temporary directory and yield the path.""" + directories = sorted(set(os.path.dirname(path) for path in complete_file_list)) + directories.remove('') -def extract_sdist(sdist_path, tmp_dir): - """Untar the sdist""" - # Untar the sdist from the tmp_dir - with tarfile.open(os.path.join(tmp_dir, sdist_path), 'r|*') as sdist: - sdist.extractall(path=tmp_dir) - - # Determine the sdist directory name - sdist_filename = os.path.basename(sdist_path) - tmp_dir_files = os.listdir(tmp_dir) - try: - tmp_dir_files.remove(sdist_filename) - except ValueError: - # Unexpected could not find original sdist in temp dir - raise + with tempfile.TemporaryDirectory() as temp_dir: + for directory in directories: + os.makedirs(os.path.join(temp_dir, directory)) - if len(tmp_dir_files) > 1: - raise Exception('Unexpected extra files in the temp dir') - elif len(tmp_dir_files) < 1: - raise Exception('sdist extraction did not occur i nthe temp dir') + for path in complete_file_list: + shutil.copy2(path, os.path.join(temp_dir, path), follow_symlinks=False) - return os.path.join(tmp_dir, tmp_dir_files[0]) + yield temp_dir -def install_sdist(tmp_dir, sdist_dir): - """Install the extracted sdist into the temporary directory""" - install = subprocess.run( - ['python', 'setup.py', 'install', '--root=%s' % tmp_dir], +def build(source_dir: str, tmp_dir: str) -> tuple[pathlib.Path, pathlib.Path]: + """Create a sdist and wheel.""" + create = subprocess.run( + [sys.executable, '-m', 'build', '--no-isolation', '--outdir', tmp_dir], stdin=subprocess.DEVNULL, capture_output=True, text=True, - cwd=os.path.join(tmp_dir, sdist_dir), check=False, + cwd=source_dir, ) - stdout, stderr = install.stdout, install.stderr - - if install.returncode != 0: - raise Exception('sdist install failed:\n%s' % stderr) - - # Determine the prefix for the installed files - match = re.search('^copying .* -> (%s/.*?/(?:site|dist)-packages)/ansible$' % - tmp_dir, stdout, flags=re.M) - - return match.group(1) - - -def check_sdist_contains_expected(sdist_dir, to_ship_files): - """Check that the files we expect to ship are present in the sdist""" - results = [] - for filename in to_ship_files: - path = os.path.join(sdist_dir, filename) - if not os.path.exists(path): - results.append('%s: File was not added to sdist' % filename) - - # Also changelog - changelog_files = glob.glob(os.path.join(sdist_dir, 'changelogs/CHANGELOG-v2.[0-9]*.rst')) - if not changelog_files: - results.append('changelogs/CHANGELOG-v2.*.rst: Changelog file was not added to the sdist') - elif len(changelog_files) > 1: - results.append('changelogs/CHANGELOG-v2.*.rst: Too many changelog files: %s' - % changelog_files) - - return results - - -def check_sdist_files_are_wanted(sdist_dir, to_ship_files): - """Check that all files in the sdist are desired""" - results = [] - for dirname, dummy, files in os.walk(sdist_dir): - dirname = os.path.relpath(dirname, start=sdist_dir) - if dirname == '.': - dirname = '' - - for filename in files: - if filename == 'setup.cfg': - continue - - path = os.path.join(dirname, filename) - if path not in to_ship_files: - - if fnmatch.fnmatch(path, 'changelogs/CHANGELOG-v2.[0-9]*.rst'): - # changelog files are expected - continue - - if fnmatch.fnmatch(path, 'lib/ansible_core.egg-info/*'): - continue + if create.returncode != 0: + raise RuntimeError(f'build failed:\n{create.stderr}\n{create.stdout}') - # FIXME: ansible-test doesn't pass the paths of symlinks to us so we aren't - # checking those - if not os.path.islink(os.path.join(sdist_dir, path)): - results.append('%s: File in sdist was not in the repository' % path) + tmp_dir_files = list(pathlib.Path(tmp_dir).iterdir()) - return results + if len(tmp_dir_files) != 2: + raise RuntimeError(f'build resulted in {len(tmp_dir_files)} items instead of 2') + sdist_path = [path for path in tmp_dir_files if path.suffix == '.gz'][0] + wheel_path = [path for path in tmp_dir_files if path.suffix == '.whl'][0] -def check_installed_contains_expected(install_dir, to_install_files): - """Check that all the files we expect to be installed are""" - results = [] - for filename in to_install_files: - path = os.path.join(install_dir, filename) - if not os.path.exists(path): - results.append('%s: File not installed' % os.path.join('lib', filename)) + return sdist_path, wheel_path - return results +def list_sdist(path: pathlib.Path) -> list[str]: + """Return a list of the files in the sdist.""" + item: tarfile.TarInfo -EGG_RE = re.compile('ansible[^/]+\\.egg-info/(PKG-INFO|SOURCES.txt|' - 'dependency_links.txt|not-zip-safe|requires.txt|top_level.txt|entry_points.txt)$') + with tarfile.open(path) as sdist: + paths = ['/'.join(pathlib.Path(item.path).parts[1:]) for item in sdist.getmembers() if not item.isdir()] + return paths -def check_installed_files_are_wanted(install_dir, to_install_files): - """Check that all installed files were desired""" - results = [] - for dirname, dummy, files in os.walk(install_dir): - dirname = os.path.relpath(dirname, start=install_dir) - if dirname == '.': - dirname = '' +def list_wheel(path: pathlib.Path) -> list[str]: + """Return a list of the files in the wheel.""" + with zipfile.ZipFile(path) as wheel: + paths = [item.filename for item in wheel.filelist if not item.is_dir()] - for filename in files: - # If this is a byte code cache, look for the python file's name - directory = dirname - if filename.endswith('.pyc') or filename.endswith('.pyo'): - # Remove the trailing "o" or c" - filename = filename[:-1] + return paths - if directory.endswith('%s__pycache__' % os.path.sep): - # Python3 byte code cache, look for the basename of - # __pycache__/__init__.cpython-36.py - segments = filename.rsplit('.', 2) - if len(segments) >= 3: - filename = '.'.join((segments[0], segments[2])) - directory = os.path.dirname(directory) - path = os.path.join(directory, filename) +def check_files(source: str, expected: list[str], actual: list[str]) -> list[str]: + """Verify the expected files exist and no extra files exist.""" + missing = sorted(set(expected) - set(actual)) + extra = sorted(set(actual) - set(expected)) - # Test that the file was listed for installation - if path not in to_install_files: - # FIXME: ansible-test doesn't pass the paths of symlinks to us so we - # aren't checking those - if not os.path.islink(os.path.join(install_dir, path)): - if not EGG_RE.match(path): - results.append('%s: File was installed but was not supposed to be' % path) + errors = ( + [f'{path}: missing from {source}' for path in missing] + + [f'{path}: unexpected in {source}' for path in extra] + ) - return results + return errors -def main(): - """All of the files in the repository""" +def main() -> None: + """Main program entry point.""" complete_file_list = sys.argv[1:] or sys.stdin.read().splitlines() + errors = [] + # Limit visible files to those reported by ansible-test. # This avoids including files which are not committed to git. with clean_repository(complete_file_list) as clean_repo_dir: - os.chdir(clean_repo_dir) + if __version__.endswith('.dev0'): + # Make sure a changelog exists for this version when testing from devel. + # When testing from a stable branch the changelog will already exist. + major_minor_version = '.'.join(__version__.split('.')[:2]) + changelog_path = f'changelogs/CHANGELOG-v{major_minor_version}.rst' + pathlib.Path(clean_repo_dir, changelog_path).touch() + complete_file_list.append(changelog_path) - to_ship_files = assemble_files_to_ship(complete_file_list) - to_install_files = assemble_files_to_install(complete_file_list) + expected_sdist_files = collect_sdist_files(complete_file_list) + expected_wheel_files = collect_wheel_files(complete_file_list) - results = [] with tempfile.TemporaryDirectory() as tmp_dir: - sdist_path = create_sdist(tmp_dir) - sdist_dir = extract_sdist(sdist_path, tmp_dir) - - # Check that the files that are supposed to be in the sdist are there - results.extend(check_sdist_contains_expected(sdist_dir, to_ship_files)) - - # Check that the files that are in the sdist are in the repository - results.extend(check_sdist_files_are_wanted(sdist_dir, to_ship_files)) - - # install the sdist - install_dir = install_sdist(tmp_dir, sdist_dir) + sdist_path, wheel_path = build(clean_repo_dir, tmp_dir) - # Check that the files that are supposed to be installed are there - results.extend(check_installed_contains_expected(install_dir, to_install_files)) + actual_sdist_files = list_sdist(sdist_path) + actual_wheel_files = list_wheel(wheel_path) - # Check that the files that are installed are supposed to be installed - results.extend(check_installed_files_are_wanted(install_dir, to_install_files)) + errors.extend(check_files('sdist', expected_sdist_files, actual_sdist_files)) + errors.extend(check_files('wheel', expected_wheel_files, actual_wheel_files)) - for message in results: - print(message) + for error in errors: + print(error) if __name__ == '__main__': diff --git a/test/sanity/code-smell/package-data.requirements.in b/test/sanity/code-smell/package-data.requirements.in index 0eb4bd30288..aa53fb89940 100644 --- a/test/sanity/code-smell/package-data.requirements.in +++ b/test/sanity/code-smell/package-data.requirements.in @@ -1,4 +1,5 @@ -build +build # required to build sdist +wheel # required to build wheel jinja2 pyyaml # ansible-core requirement resolvelib < 1.1.0 diff --git a/test/sanity/code-smell/package-data.requirements.txt b/test/sanity/code-smell/package-data.requirements.txt index ba714afa911..9d43030216c 100644 --- a/test/sanity/code-smell/package-data.requirements.txt +++ b/test/sanity/code-smell/package-data.requirements.txt @@ -16,3 +16,4 @@ setuptools==45.2.0 tomli==2.0.1 types-docutils==0.18.3 typing_extensions==4.5.0 +wheel==0.41.0