From ce322b9f32c0f21a5c02f962a6a8f874fbc5e5ce Mon Sep 17 00:00:00 2001 From: Gregory Szorc Date: Thu, 26 Mar 2026 18:56:15 -0700 Subject: [PATCH] Revert "Port `verify_distribution.py` into `pythonbuild` (#1058)" This reverts commit db15e1ab82c15059ba43ac705540ba0c4e26a376. --- .github/workflows/linux.yml | 9 +- .github/workflows/macos.yml | 6 +- .github/workflows/windows.yml | 3 +- Cargo.lock | 146 +++++++++++++++++- Cargo.toml | 1 + docs/status.rst | 2 +- pythonbuild/testdist.py | 80 +--------- src/main.rs | 6 + src/validation.rs | 56 ++++++- .../__init__.py => src/verify_distribution.py | 9 +- test-distribution.py | 6 +- 11 files changed, 221 insertions(+), 103 deletions(-) rename pythonbuild/disttests/__init__.py => src/verify_distribution.py (97%) diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 2e934c5fd..3ff91a3a4 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -318,19 +318,18 @@ jobs: run: | chmod +x build/pythonbuild - build/pythonbuild validate-distribution dist/*.tar.zst - if [ "${MATRIX_RUN}" == "true" ]; then if [ "${MATRIX_LIBC}" == "musl" ]; then sudo apt install musl-dev - + # GitHub's setup-python action sets `LD_LIBRARY_PATH` which overrides `RPATH` # as used in the musl builds. unset LD_LIBRARY_PATH fi - - ./test-distribution.py dist/*.tar.zst + EXTRA_ARGS="--run" fi + + build/pythonbuild validate-distribution ${EXTRA_ARGS} dist/*.tar.zst env: MATRIX_RUN: ${{ matrix.run }} MATRIX_LIBC: ${{ matrix.libc }} diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 3a992a9a4..1386312d6 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -178,10 +178,10 @@ jobs: run: | chmod +x build/pythonbuild - build/pythonbuild validate-distribution --macos-sdks-path macosx-sdks dist/*.tar.zst - if [ "${MATRIX_RUN}" == "true" ]; then - ./test-distribution.py dist/*.tar.zst + EXTRA_ARGS="--run" fi + + build/pythonbuild validate-distribution --macos-sdks-path macosx-sdks ${EXTRA_ARGS} dist/*.tar.zst env: MATRIX_RUN: ${{ matrix.run }} diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 19fc4eade..40a58538c 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -185,5 +185,4 @@ jobs: if: ${{ ! matrix.dry-run }} run: | $Dists = Resolve-Path -Path "dist/*.tar.zst" -Relative - .\pythonbuild.exe validate-distribution $Dists - uv run test-distribution.py $Dists + .\pythonbuild.exe validate-distribution --run $Dists diff --git a/Cargo.lock b/Cargo.lock index 1e4ec0faa..e26a67fc7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -583,6 +583,18 @@ dependencies = [ "syn", ] +[[package]] +name = "duct" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e66e9c0c03d094e1a0ba1be130b849034aa80c3a2ab8ee94316bc809f3fa684" +dependencies = [ + "libc", + "os_pipe", + "shared_child", + "shared_thread", +] + [[package]] name = "dunce" version = "1.0.5" @@ -1723,6 +1735,16 @@ version = "0.1.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d05e27ee213611ffe7d6348b942e8f942b37114c00cc03cec254295a4a17852e" +[[package]] +name = "os_pipe" +version = "1.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "db335f4760b14ead6290116f2427bf33a14d4f0617d49f78a246de10c1831224" +dependencies = [ + "libc", + "windows-sys 0.59.0", +] + [[package]] name = "p256" version = "0.13.2" @@ -1963,6 +1985,7 @@ dependencies = [ "apple-sdk", "bytes", "clap", + "duct", "flate2", "futures", "goblin", @@ -2666,12 +2689,59 @@ dependencies = [ "digest", ] +[[package]] +name = "shared_child" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e362d9935bc50f019969e2f9ecd66786612daae13e8f277be7bfb66e8bed3f7" +dependencies = [ + "libc", + "sigchld", + "windows-sys 0.60.2", +] + +[[package]] +name = "shared_thread" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "52b86057fcb5423f5018e331ac04623e32d6b5ce85e33300f92c79a1973928b0" + [[package]] name = "shlex" version = "1.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0fda2ff0d084019ba4d7c6f371c95d8fd75ce3524c3cb8fb653a3023f6323e64" +[[package]] +name = "sigchld" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47106eded3c154e70176fc83df9737335c94ce22f821c32d17ed1db1f83badb1" +dependencies = [ + "libc", + "os_pipe", + "signal-hook", +] + +[[package]] +name = "signal-hook" +version = "0.3.18" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d881a16cf4426aa584979d30bd82cb33429027e42122b169753d6ef1085ed6e2" +dependencies = [ + "libc", + "signal-hook-registry", +] + +[[package]] +name = "signal-hook-registry" +version = "1.4.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b2a4719bff48cee6b39d12c020eeb490953ad2443b7055bd0b21fca26bd8c28b" +dependencies = [ + "libc", +] + [[package]] name = "signature" version = "2.2.0" @@ -3499,6 +3569,15 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "windows-sys" +version = "0.60.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f2f500e4d28234f72040990ec9d39e3a6b950f9f22d3dba18416c35882612bcb" +dependencies = [ + "windows-targets 0.53.4", +] + [[package]] name = "windows-sys" version = "0.61.1" @@ -3532,13 +3611,30 @@ dependencies = [ "windows_aarch64_gnullvm 0.52.6", "windows_aarch64_msvc 0.52.6", "windows_i686_gnu 0.52.6", - "windows_i686_gnullvm", + "windows_i686_gnullvm 0.52.6", "windows_i686_msvc 0.52.6", "windows_x86_64_gnu 0.52.6", "windows_x86_64_gnullvm 0.52.6", "windows_x86_64_msvc 0.52.6", ] +[[package]] +name = "windows-targets" +version = "0.53.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2d42b7b7f66d2a06854650af09cfdf8713e427a439c97ad65a6375318033ac4b" +dependencies = [ + "windows-link 0.2.0", + "windows_aarch64_gnullvm 0.53.0", + "windows_aarch64_msvc 0.53.0", + "windows_i686_gnu 0.53.0", + "windows_i686_gnullvm 0.53.0", + "windows_i686_msvc 0.53.0", + "windows_x86_64_gnu 0.53.0", + "windows_x86_64_gnullvm 0.53.0", + "windows_x86_64_msvc 0.53.0", +] + [[package]] name = "windows_aarch64_gnullvm" version = "0.42.2" @@ -3551,6 +3647,12 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a4622180e7a0ec044bb555404c800bc9fd9ec262ec147edd5989ccd0c02cd3" +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.53.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "86b8d5f90ddd19cb4a147a5fa63ca848db3df085e25fee3cc10b39b6eebae764" + [[package]] name = "windows_aarch64_msvc" version = "0.42.2" @@ -3563,6 +3665,12 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "09ec2a7bb152e2252b53fa7803150007879548bc709c039df7627cabbd05d469" +[[package]] +name = "windows_aarch64_msvc" +version = "0.53.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c7651a1f62a11b8cbd5e0d42526e55f2c99886c77e007179efff86c2b137e66c" + [[package]] name = "windows_i686_gnu" version = "0.42.2" @@ -3575,12 +3683,24 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8e9b5ad5ab802e97eb8e295ac6720e509ee4c243f69d781394014ebfe8bbfa0b" +[[package]] +name = "windows_i686_gnu" +version = "0.53.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c1dc67659d35f387f5f6c479dc4e28f1d4bb90ddd1a5d3da2e5d97b42d6272c3" + [[package]] name = "windows_i686_gnullvm" version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0eee52d38c090b3caa76c563b86c3a4bd71ef1a819287c19d586d7334ae8ed66" +[[package]] +name = "windows_i686_gnullvm" +version = "0.53.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9ce6ccbdedbf6d6354471319e781c0dfef054c81fbc7cf83f338a4296c0cae11" + [[package]] name = "windows_i686_msvc" version = "0.42.2" @@ -3593,6 +3713,12 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "240948bc05c5e7c6dabba28bf89d89ffce3e303022809e73deaefe4f6ec56c66" +[[package]] +name = "windows_i686_msvc" +version = "0.53.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "581fee95406bb13382d2f65cd4a908ca7b1e4c2f1917f143ba16efe98a589b5d" + [[package]] name = "windows_x86_64_gnu" version = "0.42.2" @@ -3605,6 +3731,12 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "147a5c80aabfbf0c7d901cb5895d1de30ef2907eb21fbbab29ca94c5b08b1a78" +[[package]] +name = "windows_x86_64_gnu" +version = "0.53.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e55b5ac9ea33f2fc1716d1742db15574fd6fc8dadc51caab1c16a3d3b4190ba" + [[package]] name = "windows_x86_64_gnullvm" version = "0.42.2" @@ -3617,6 +3749,12 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "24d5b23dc417412679681396f2b49f3de8c1473deb516bd34410872eff51ed0d" +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.53.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0a6e035dd0599267ce1ee132e51c27dd29437f63325753051e71dd9e42406c57" + [[package]] name = "windows_x86_64_msvc" version = "0.42.2" @@ -3629,6 +3767,12 @@ version = "0.52.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" +[[package]] +name = "windows_x86_64_msvc" +version = "0.53.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "271414315aff87387382ec3d271b52d7ae78726f5d44ac98b4f4030c91880486" + [[package]] name = "wit-bindgen" version = "0.51.0" diff --git a/Cargo.toml b/Cargo.toml index 7cc511c48..240a2871a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,7 @@ anyhow = "1.0.100" apple-sdk = "0.6.0" bytes = "1.11.0" clap = "4.5.52" +duct = "1.1.1" flate2 = "1.1.5" futures = "0.3.30" goblin = "0.10.3" diff --git a/docs/status.rst b/docs/status.rst index 9a8f067cb..41f4de1db 100644 --- a/docs/status.rst +++ b/docs/status.rst @@ -41,7 +41,7 @@ This repository contains ``test-distribution.py`` script that can be used to run the Python test harness from a distribution archive. Here, we track the various known failures when running -``test-distribution.py --stdlib -- /path/to/distribution.tar.zst -u all``. +``test-distribution.py /path/to/distribution.tar.zst -u all``. ``test__locale`` ---------------- diff --git a/pythonbuild/testdist.py b/pythonbuild/testdist.py index 96f65790e..34a5af3cd 100644 --- a/pythonbuild/testdist.py +++ b/pythonbuild/testdist.py @@ -4,65 +4,13 @@ import argparse import json -import os import subprocess import tempfile from pathlib import Path -from typing import Optional from .utils import extract_python_archive -def run_dist_python( - dist_root: Path, - python_info, - args: list[str], - extra_env: Optional[dict[str, str]] = None, - **runargs, -) -> subprocess.CompletedProcess[str]: - """Runs a `python` process from an extracted PBS distribution. - - This function attempts to isolate the spawned interpreter from any - external interference (PYTHON* environment variables), etc. - """ - env = dict(os.environ) - - # Wipe PYTHON environment variables. - for k in env: - if k.startswith("PYTHON"): - del env[k] - - if extra_env: - env.update(extra_env) - - return subprocess.run( - [str(dist_root / python_info["python_exe"])] + args, - cwd=dist_root, - env=env, - **runargs, - ) - - -def run_custom_unittests(pbs_source_dir: Path, dist_root: Path, python_info) -> int: - """Runs custom PBS unittests against a distribution.""" - - args = [ - "-m", - "unittest", - "pythonbuild.disttests", - ] - - env = { - "PYTHONPATH": str(pbs_source_dir), - "TARGET_TRIPLE": python_info["target_triple"], - "BUILD_OPTIONS": python_info["build_options"], - } - - res = run_dist_python(dist_root, python_info, args, env, stderr=subprocess.STDOUT) - - return res.returncode - - def run_stdlib_tests(dist_root: Path, python_info, harness_args: list[str]) -> int: """Run Python stdlib tests for a PBS distribution. @@ -70,24 +18,19 @@ def run_stdlib_tests(dist_root: Path, python_info, harness_args: list[str]) -> i archive. """ args = [ + str(dist_root / python_info["python_exe"]), str(dist_root / python_info["run_tests"]), ] args.extend(harness_args) - return run_dist_python(dist_root, python_info, args).returncode + return subprocess.run(args).returncode -def main(pbs_source_dir: Path, raw_args: list[str]) -> int: +def main(raw_args: list[str]) -> int: """test-distribution.py functionality.""" - parser = argparse.ArgumentParser() - parser.add_argument( - "--stdlib", - action="store_true", - help="Run the stdlib test harness", - ) parser.add_argument( "dist", nargs=1, @@ -116,22 +59,7 @@ def main(pbs_source_dir: Path, raw_args: list[str]) -> int: with python_json.open("r", encoding="utf-8") as fh: python_info = json.load(fh) - codes = [] - - codes.append(run_custom_unittests(pbs_source_dir, dist_path, python_info)) - - if args.stdlib: - codes.append(run_stdlib_tests(dist_path, python_info, args.harness_args)) - - if len(codes) == 0: - print("no tests run") - return 1 - - if any(code != 0 for code in codes): - return 1 - - return 0 - + return run_stdlib_tests(dist_path, python_info, args.harness_args) finally: if td: td.cleanup() diff --git a/src/main.rs b/src/main.rs index f30b63b4f..a09ef8a17 100644 --- a/src/main.rs +++ b/src/main.rs @@ -172,6 +172,12 @@ fn main_impl() -> Result<()> { let app = app.subcommand( Command::new("validate-distribution") .about("Ensure a distribution archive conforms to standards") + .arg( + Arg::new("run") + .long("run") + .action(ArgAction::SetTrue) + .help("Run the interpreter to verify behavior"), + ) .arg( Arg::new("macos_sdks_path") .long("macos-sdks-path") diff --git a/src/validation.rs b/src/validation.rs index af54073a9..2528cc25e 100644 --- a/src/validation.rs +++ b/src/validation.rs @@ -4,21 +4,21 @@ use { crate::{json::*, macho::*}, - anyhow::{Context, Result, anyhow}, + anyhow::{anyhow, Context, Result}, clap::ArgMatches, normalize_path::NormalizePath, object::{ - Architecture, Endianness, FileKind, Object, SectionIndex, SymbolScope, elf::{ - ET_DYN, ET_EXEC, FileHeader32, FileHeader64, SHN_UNDEF, STB_GLOBAL, STB_WEAK, + FileHeader32, FileHeader64, ET_DYN, ET_EXEC, SHN_UNDEF, STB_GLOBAL, STB_WEAK, STV_DEFAULT, STV_HIDDEN, }, - macho::{LC_CODE_SIGNATURE, MH_OBJECT, MH_TWOLEVEL, MachHeader32, MachHeader64}, + macho::{MachHeader32, MachHeader64, LC_CODE_SIGNATURE, MH_OBJECT, MH_TWOLEVEL}, read::{ elf::{Dyn, FileHeader, SectionHeader, Sym}, macho::{LoadCommandVariant, MachHeader, Nlist, Section, Segment}, pe::{ImageNtHeaders, PeFile, PeFile32, PeFile64}, }, + Architecture, Endianness, FileKind, Object, SectionIndex, SymbolScope, }, once_cell::sync::Lazy, std::{ @@ -854,6 +854,8 @@ const SHARED_LIBRARY_EXTENSIONS: &[&str] = &[ "_tkinter", ]; +const PYTHON_VERIFICATIONS: &str = include_str!("verify_distribution.py"); + fn allowed_dylibs_for_triple(triple: &str) -> Vec { match triple { "aarch64-apple-darwin" => DARWIN_ALLOWED_DYLIBS.clone(), @@ -2151,7 +2153,47 @@ fn validate_distribution( Ok(context.errors) } +fn verify_distribution_behavior(dist_path: &Path) -> Result> { + let mut errors = vec![]; + + let temp_dir = tempfile::TempDir::new()?; + + let mut tf = crate::open_distribution_archive(dist_path)?; + + tf.unpack(temp_dir.path())?; + + let python_json_path = temp_dir.path().join("python").join("PYTHON.json"); + let python_json_data = std::fs::read(python_json_path)?; + let python_json = parse_python_json(&python_json_data)?; + + let python_exe = temp_dir.path().join("python").join(python_json.python_exe); + + let test_file = temp_dir.path().join("verify.py"); + std::fs::write(&test_file, PYTHON_VERIFICATIONS.as_bytes())?; + + eprintln!(" running interpreter tests (output should follow)"); + let output = duct::cmd(&python_exe, [test_file.display().to_string()]) + .stdout_to_stderr() + .unchecked() + .env("TARGET_TRIPLE", &python_json.target_triple) + .env("BUILD_OPTIONS", &python_json.build_options) + .run() + .context(format!( + "Failed to run `{} {}`", + python_exe.display(), + test_file.display() + ))?; + + if !output.status.success() { + errors.push("errors running interpreter tests".to_string()); + } + + Ok(errors) +} + pub fn command_validate_distribution(args: &ArgMatches) -> Result<()> { + let run = args.get_flag("run"); + let macos_sdks = if let Some(path) = args.get_one::("macos_sdks_path") { Some(IndexedSdks::new(path)?) } else { @@ -2162,7 +2204,11 @@ pub fn command_validate_distribution(args: &ArgMatches) -> Result<()> { for path in args.get_many::("path").unwrap() { println!("validating {}", path.display()); - let errors = validate_distribution(path, macos_sdks.as_ref())?; + let mut errors = validate_distribution(path, macos_sdks.as_ref())?; + + if run { + errors.extend(verify_distribution_behavior(path)?.into_iter()); + } if errors.is_empty() { println!(" {} OK", path.display()); diff --git a/pythonbuild/disttests/__init__.py b/src/verify_distribution.py similarity index 97% rename from pythonbuild/disttests/__init__.py rename to src/verify_distribution.py index 8cd9d44a5..c573bdfb9 100644 --- a/pythonbuild/disttests/__init__.py +++ b/src/verify_distribution.py @@ -10,7 +10,6 @@ import tempfile import unittest from pathlib import Path -from typing import Optional TERMINFO_DIRS = [ "/etc/terminfo", @@ -124,12 +123,12 @@ def test_hashlib(self): "_testcapi not available on statically-linked distributions", ) def test_testcapi(self): - import _testcapi # type: ignore + import _testcapi self.assertIsNotNone(_testcapi) if sys.version_info[0:2] >= (3, 13): - import _testlimitedcapi # type: ignore + import _testlimitedcapi self.assertIsNotNone(_testlimitedcapi) @@ -236,7 +235,7 @@ def test_gil_disabled(self): "zstd is only available in 3.14+", ) def test_zstd_multithreaded(self): - from compression import zstd # type: ignore + from compression import zstd max_threads = zstd.CompressionParameter.nb_workers.bounds()[1] assert max_threads > 0, ( @@ -295,7 +294,7 @@ def assertLibc(value): ) @unittest.skipIf(os.name == "nt", "no symlinks or argv[0] on Windows") def test_getpath(self): - def assertPythonWorks(path: Path, argv0: Optional[str] = None): + def assertPythonWorks(path: Path, argv0: str = None): output = subprocess.check_output( [argv0 or path, "-c", "print(42)"], executable=path, text=True ) diff --git a/test-distribution.py b/test-distribution.py index c4018f102..49d793705 100755 --- a/test-distribution.py +++ b/test-distribution.py @@ -5,19 +5,15 @@ """Script to run Python tests from a distribution archive.""" -import os -import pathlib import sys from pythonbuild.testdist import main -ROOT = pathlib.Path(os.path.abspath(__file__)).parent - if __name__ == "__main__": # Unbuffer stdout. sys.stdout.reconfigure(line_buffering=True) try: - sys.exit(main(ROOT, sys.argv[1:])) + sys.exit(main(sys.argv[1:])) except KeyboardInterrupt: sys.exit(1)