Skip to content

Commit eed3355

Browse files
hjmjohnsonhjmjohnson
committed
ENH: Remove obsolete comments and improve error handling in build scripts
Cleaned up unused commented-out code across multiple build scripts (`build_wheels.py`, `build_python_instance_base.py`, `windows_build_python_instance.py`). Replaced `sys.exit` with `RuntimeError` for consistent error handling. Fixed typos in variable names and adjusted build commands for clarity.
1 parent 77f982b commit eed3355

3 files changed

Lines changed: 15 additions & 36 deletions

File tree

‎scripts/build_python_instance_base.py‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ def __init__(
7070
self.cleanup = cleanup
7171
self.build_itk_tarball_cache = build_itk_tarball_cache
7272
self.cmake_options = cmake_options
73-
# NEVER CLEANUP FOR DEBUGGING cleanup
7473
self.windows_extra_lib_paths = windows_extra_lib_paths
7574
self.dist_dir = dist_dir
7675
# Needed for processing remote modules and their dependencies
@@ -274,12 +273,12 @@ def build_superbuild_support_components(self):
274273
self.echo_check_call(cmd)
275274
self.echo_check_call(
276275
[
277-
# self.package_env_config["NINJA_EXECUTABLE"],
278-
# f"-j{self.build_node_cpu_count}",
279-
# f"-l{self.build_node_cpu_count}",
280-
# "-C",
281276
"cmake",
282277
"--build",
278+
"--load-average",
279+
str(self.build_node_cpu_count),
280+
"--parallel",
281+
str(self.build_node_cpu_count),
283282
str(self.package_env_config["IPP_SUPERBUILD_BINARY_DIR"]),
284283
],
285284
)
@@ -759,18 +758,18 @@ def _normalize(entry: str) -> str:
759758
)
760759

761760
if (dependant_module_clone_dir / "setup.py").exists():
762-
print(
761+
msg: str = (
763762
f"Old sci-kit-build with setup.py is no longer supported for {dependant_module_clone_dir} at {tag}"
764763
)
765-
sys.exit(1)
764+
raise RuntimeError(msg)
766765

767-
# Clonen the current build environment, and modify for current module
768-
dependannt_module_build_setup = self.clone()
769-
dependannt_module_build_setup.module_source_dir = Path(
766+
# Clone the current build environment and modify for the current module
767+
dependent_module_build_setup = self.clone()
768+
dependent_module_build_setup.module_source_dir = Path(
770769
dependant_module_clone_dir
771770
)
772-
dependannt_module_build_setup.itk_module_deps = None # Prevent recursion
773-
dependannt_module_build_setup.run()
771+
dependent_module_build_setup.itk_module_deps = None # Prevent recursion
772+
dependent_module_build_setup.run()
774773

775774
# After building dependency, copy includes and wrapping files
776775
# 1) Top-level include/* -> include/

‎scripts/build_wheels.py‎

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -359,25 +359,6 @@ def build_wheels_main() -> None:
359359
str(_ipp_dir_path / ".pixi" / "bin") + os.pathsep + os.environ.get("PATH", "")
360360
)
361361
pixi_exec_path: Path = _which("pixi" + binary_ext)
362-
# pixi_bin_path: Path = pixi_exec_path.parent
363-
# Required executables (paths recorded)
364-
# platform_pixi_packages = ["doxygen", "ninja", "cmake", "git"]
365-
# if os_name == "linux":
366-
# platform_pixi_packages += ["patchelf"]
367-
#
368-
# missing_packages: list[str] = []
369-
# for ppp in platform_pixi_packages:
370-
# # Search for binaries herefull_path: Path = pixi_exec_path / (ppp + binary_ext)
371-
# if not full_path.is_file():
372-
# missing_packages.append(ppp)
373-
# if len(missing_packages) > 0:
374-
# print diagnostic messaging and exit
375-
376-
# NOT NEEDED
377-
# ipp_latest_tag: str = get_git_id(
378-
# _ipp_dir_path, pixi_exec_path, os.environ, os.environ.get("ITKPYTHONPACKAGE_TAG", "v0.0.0")
379-
# )
380-
381362
package_env_config: dict[str, str | Path | None] = {}
382363

383364
args.build_dir_root = Path(args.build_dir_root)
@@ -521,10 +502,10 @@ def build_wheels_main() -> None:
521502
# Native builds without dockercross need a separate dist dir to avoid conflicts with manylinux
522503
# dist_dir = IPP_SOURCE_DIR / f"{platform}_dist"
523504
if os.environ.get("CROSS_TRIPLE", None) is None:
524-
print(
505+
msg: str = (
525506
f"ERROR: MANYLINUX_VERSION={manylinux_version} but not building in dockcross."
526507
)
527-
sys.exit(1)
508+
raise RuntimeError(msg)
528509

529510
builder_cls = LinuxBuildPythonInstance
530511
else:

‎scripts/windows_build_python_instance.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,6 @@ def get_python_version(platform_env: str) -> None | tuple[int, int]:
257257
venv_base_dir: Path = primary_python_base_dir
258258
venv_bin_path = self.package_env_config["PYTHON_EXECUTABLE"].parent
259259

260-
python_executable = self.package_env_config["PYTHON_EXECUTABLE"]
261260
python_include_dir = primary_python_base_dir / "include"
262261
python_major, python_minor = get_python_version(self.platform_env)
263262
if python_minor >= 11:
@@ -266,8 +265,8 @@ def get_python_version(platform_env: str) -> None | tuple[int, int]:
266265
primary_python_base_dir / "libs" / f"python{python_major}.lib"
267266
)
268267
else:
269-
# XXX It should be possible to query skbuild for the library dir associated
270-
# with a given interpreter.
268+
# It should be possible to query skbuild for the library dir associated
269+
# with a given interpreter.
271270
xy_lib_ver = f"{python_major}{python_minor}"
272271
python_library = (
273272
primary_python_base_dir / "libs" / f"python{xy_lib_ver}.lib"

0 commit comments

Comments
 (0)