Skip to content

Commit f7086ea

Browse files
hjmjohnsonhjmjohnson
committed
ENH: Centralize and streamline venv paths management across platforms
- Introduced `venv_info_dict` to encapsulate venv-related paths and executables in a unified structure. - Refactored `venv_paths` to populate `venv_info_dict`, reducing redundancy in path handling. - Updated platform-specific scripts (Linux, macOS, Windows) to utilize `venv_info_dict`. - Improved modularity and maintainability of virtual environment logic in build scripts.
1 parent 1dce0f6 commit f7086ea

4 files changed

Lines changed: 172 additions & 194 deletions

File tree

‎scripts/build_python_instance_base.py‎

Lines changed: 85 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -158,25 +158,27 @@ def __init__(
158158
self.windows_extra_lib_paths = windows_extra_lib_paths
159159
self.dist_dir = dist_dir
160160
self._cmake_executable = None
161-
self._ninja_executable = None
162161
self._doxygen_executable = None
163162
self._use_tbb = "ON"
164163
self._tbb_dir = None
165164
self._build_type = "Release"
165+
self.venv_info_dict = {
166+
"python_executable": None,
167+
"python_include_dir": None,
168+
"python_library": None,
169+
"pip_executable": None,
170+
"ninja_executable": None,
171+
"venv_bin_path": None,
172+
"venv_base_dir": None,
173+
}
166174

167175
def run(self) -> None:
168176
"""Run the full build flow for this Python instance."""
169177
# Use BuildManager to persist and resume build steps
170178
python_packabge_build_steps: dict = {
171179
"00_prepare_build_env": self.prepare_build_env,
172180
"01_superbuild_support_components": self.build_superbuild_support_components,
173-
"02_build_wheels": lambda: self.build_wheel(
174-
self.py_env,
175-
self._build_type,
176-
cleanup=self.cleanup,
177-
wheel_names=self.wheel_names,
178-
cmake_options=self.cmake_options,
179-
),
181+
"02_build_wheels": self.build_wheel,
180182
"03_post_build_fixup": self.post_build_fixup,
181183
"04_final_import_test": self.final_import_test,
182184
}
@@ -204,7 +206,7 @@ def build_superbuild_support_components(self):
204206
"-DITKPythonPackage_BUILD_PYTHON:BOOL=OFF",
205207
f"-DITKPythonPackage_USE_TBB:BOOL={self._use_tbb}",
206208
f"-DCMAKE_BUILD_TYPE:STRING={self._build_type}",
207-
f"-DCMAKE_MAKE_PROGRAM:FILEPATH={self._ninja_executable}",
209+
f"-DCMAKE_MAKE_PROGRAM:FILEPATH={self.venv_info_dict['ninja_executable']}",
208210
f"-DITK_SOURCE_DIR:PATH={self.package_env_config['ITK_SOURCE_DIR']}",
209211
f"-DITK_GIT_TAG:STRING={self.package_env_config['ITK_GIT_TAG']}",
210212
]
@@ -220,7 +222,11 @@ def build_superbuild_support_components(self):
220222

221223
echo_check_call(cmd)
222224
echo_check_call(
223-
[self._ninja_executable, "-C", str(self.IPP_SUPERBUILD_BINARY_DIR)]
225+
[
226+
self.venv_info_dict["ninja_executable"],
227+
"-C",
228+
str(self.IPP_SUPERBUILD_BINARY_DIR),
229+
]
224230
)
225231

226232
def fixup_wheels(self, lib_paths: str = ""):
@@ -230,17 +236,9 @@ def fixup_wheels(self, lib_paths: str = ""):
230236
self.fixup_wheel(str(wheel), lib_paths)
231237

232238
def final_wheel_import_test(self, installed_dist_dir: Path):
233-
(
234-
python_executable,
235-
_python_include_dir,
236-
_python_library,
237-
pip,
238-
_ninja_executable,
239-
_path,
240-
) = self.venv_paths()
241239
echo_check_call(
242240
[
243-
pip,
241+
self.venv_info_dict["pip_executable"],
244242
"install",
245243
"itk",
246244
"--no-cache-dir",
@@ -251,20 +249,27 @@ def final_wheel_import_test(self, installed_dist_dir: Path):
251249
)
252250
print("Wheel successfully installed.")
253251
# Basic imports
254-
echo_check_call([python_executable, "-c", "import itk;"])
252+
echo_check_call([self.venv_info_dict["python_executable"], "-c", "import itk;"])
255253
echo_check_call(
256-
[python_executable, "-c", "import itk; image = itk.Image[itk.UC, 2].New()"]
254+
[
255+
self.venv_info_dict["python_executable"],
256+
"-c",
257+
"import itk; image = itk.Image[itk.UC, 2].New()",
258+
]
257259
)
258260
echo_check_call(
259261
[
260-
python_executable,
262+
self.venv_info_dict["python_executable"],
261263
"-c",
262264
"import itkConfig; itkConfig.LazyLoading=False; import itk;",
263265
]
264266
)
265267
# Full doc tests
266268
echo_check_call(
267-
[python_executable, str(self.IPP_SOURCE_DIR / "docs" / "code" / "test.py")]
269+
[
270+
self.venv_info_dict["python_executable"],
271+
str(self.IPP_SOURCE_DIR / "docs" / "code" / "test.py"),
272+
]
268273
)
269274
print("Documentation tests passed.")
270275

@@ -310,9 +315,9 @@ def find_unix_exectable_paths(
310315
python_executable = venv_dir / "bin" / "python3"
311316
if not python_executable.exists():
312317
raise FileNotFoundError(f"Python executable not found: {python_executable}")
313-
pip = venv_dir / "bin" / "pip3"
314-
if not pip.exists():
315-
raise FileNotFoundError(f"pip executable not found: {pip}")
318+
pip_executable = venv_dir / "bin" / "pip3"
319+
if not pip_executable.exists():
320+
raise FileNotFoundError(f"pip executable not found: {pip_executable}")
316321

317322
# Prefer venv's ninja, else fall back to PATH
318323
ninja_executable_path: Path = venv_dir / "bin" / "ninja"
@@ -340,14 +345,14 @@ def find_unix_exectable_paths(
340345
python_library = ""
341346

342347
# Update PATH
343-
path = venv_dir / "bin"
348+
venv_bin_path = venv_dir / "bin"
344349
return (
345350
str(python_executable),
346351
python_include_dir,
347352
python_library,
348-
str(pip),
353+
str(pip_executable),
349354
str(ninja_executable),
350-
str(path),
355+
str(venv_bin_path),
351356
venv_dir,
352357
)
353358

@@ -386,77 +391,64 @@ def discover_python_venvs(
386391
) -> list[str]:
387392
pass
388393

389-
def build_wheel(
390-
self,
391-
python_version,
392-
build_type="Release",
393-
cleanup=True,
394-
wheel_names=None,
395-
cmake_options=None,
396-
):
397-
if cmake_options is None:
398-
cmake_options = []
394+
def build_wheel(self):
395+
"""
396+
REDO: cmake_options do not belong here, they should be at intialization
397+
need to make cmake configure module settings as dictionary to be re-user_data_dir
398+
for standard cmake.exe and build wrapped cmake, and remove the replace magic
399+
below:
400+
make ITK specific build flags dictionary
401+
make compiler environment specific dictionary
402+
make other features specific dictionary
399403
400-
(
401-
python_executable,
402-
python_include_dir,
403-
python_library,
404-
pip,
405-
ninja_executable,
406-
path,
407-
_venv_dir,
408-
) = self.venv_paths()
404+
"""
409405

410-
with push_env(PATH=f"{path}{pathsep}{environ['PATH']}"):
406+
with push_env(
407+
PATH=f"{self.venv_info_dict['venv_bin_path']}{pathsep}{environ['PATH']}"
408+
):
411409

412410
source_path = f"{self.package_env_config['ITK_SOURCE_DIR']}"
413411
# Build path naming per platform
414412
if self.platform_name == "windows":
415-
build_path = self.IPP_SOURCE_DIR / f"ITK-win_{python_version}"
413+
build_path = self.IPP_SOURCE_DIR / f"ITK-win_{self.py_env}"
416414
elif self.platform_name == "darwin":
417415
osx_arch = (
418416
"arm64" if self.platform_architechture == "arm64" else "x86_64"
419417
)
420418
build_path = (
421-
self.IPP_SOURCE_DIR / f"ITK-{python_version}-macosx_{osx_arch}"
419+
self.IPP_SOURCE_DIR / f"ITK-{self.py_env}-macosx_{osx_arch}"
422420
)
423421
elif self.platform_name == "linux":
424422
# TODO: do not use environ here, get from package_env instead
425423
manylinux_ver = environ.get("MANYLINUX_VERSION", "")
426424
build_path = (
427425
self.IPP_SOURCE_DIR
428-
/ f"ITK-{python_version}-manylinux{manylinux_ver}_{self.platform_architechture}"
426+
/ f"ITK-{self.py_env}-manylinux{manylinux_ver}_{self.platform_architechture}"
429427
)
430428
else:
431429
raise ValueError(f"Unknown platform {self.platform_name}")
432430
pyproject_configure = self.SCRIPT_DIR / "pyproject_configure.py"
433431

434432
# Clean up previous invocations
435-
if cleanup and Path(build_path).exists():
433+
if self.cleanup and Path(build_path).exists():
436434
_remove_tree(Path(build_path))
437435

438436
print("#")
439437
print("# Build multiple ITK wheels")
440438
print("#")
441439

442440
self.build_wrapped_itk(
443-
ninja_executable,
444-
build_type,
445-
source_path,
446441
build_path,
447-
python_executable,
448-
python_include_dir,
449-
python_library,
450442
)
451443

452444
# Build wheels
453445

454446
env_file = self.IPP_SOURCE_DIR / "build" / "package.env"
455-
for wheel_name in wheel_names:
447+
for wheel_name in self.wheel_names:
456448
# Configure pyproject.toml
457449
echo_check_call(
458450
[
459-
python_executable,
451+
str(self.venv_info_dict["python_executable"]),
460452
pyproject_configure,
461453
"--env-file",
462454
env_file,
@@ -466,7 +458,7 @@ def build_wheel(
466458

467459
# Generate wheel
468460
cmd = [
469-
python_executable,
461+
str(self.venv_info_dict["python_executable"]),
470462
"-m",
471463
"build",
472464
"--verbose",
@@ -480,10 +472,10 @@ def build_wheel(
480472
f"--config-setting=cmake.define.ITKPythonPackage_USE_TBB:BOOL={self._use_tbb}",
481473
"--config-setting=cmake.define.ITKPythonPackage_ITK_BINARY_REUSE:BOOL=ON",
482474
f"--config-setting=cmake.define.ITKPythonPackage_WHEEL_NAME:STRING={wheel_name}",
483-
f"--config-setting=cmake.define.Python3_EXECUTABLE:FILEPATH={python_executable}",
475+
f"--config-setting=cmake.define.Python3_EXECUTABLE:FILEPATH={self.venv_info_dict['python_executable']}",
484476
"--config-setting=cmake.define.DOXYGEN_EXECUTABLE:FILEPATH="
485477
+ f"{self.package_env_config['DOXYGEN_EXECUTABLE']}",
486-
f"--config-setting=cmake.build-type={build_type}",
478+
f"--config-setting=cmake.build-type={self._build_type}",
487479
]
488480
if self.platform_name == "darwin":
489481
macosx_target = self.package_env_config.get(
@@ -517,26 +509,26 @@ def build_wheel(
517509
cmd.append(
518510
"--config-setting=cmake.define.CMAKE_C_FLAGS:STRING=-O3 -DNDEBUG"
519511
)
520-
if python_include_dir:
512+
if self.venv_info_dict["python_include_dir"]:
521513
cmd.append(
522-
f"--config-setting=cmake.define.Python3_INCLUDE_DIR:PATH={python_include_dir}"
514+
f"--config-setting=cmake.define.Python3_INCLUDE_DIR:PATH={self.venv_info_dict['python_include_dir']}"
523515
)
524516
cmd.append(
525-
f"--config-setting=cmake.define.Python3_INCLUDE_DIRS:PATH={python_include_dir}"
517+
f"--config-setting=cmake.define.Python3_INCLUDE_DIRS:PATH={self.venv_info_dict['python_include_dir']}"
526518
)
527-
if python_library:
519+
if self.venv_info_dict["python_library"]:
528520
cmd.append(
529-
f"--config-setting=cmake.define.Python3_LIBRARY:FILEPATH={python_library}"
521+
f"--config-setting=cmake.define.Python3_LIBRARY:FILEPATH={self.venv_info_dict['python_library']}"
530522
)
531523
cmd += [
532524
o.replace("-D", "--config-setting=cmake.define.")
533-
for o in cmake_options
525+
for o in self.cmake_options
534526
]
535527
cmd += [str(self.IPP_SOURCE_DIR)]
536528
echo_check_call(cmd)
537529

538530
# Remove unnecessary files for building against ITK
539-
if cleanup:
531+
if self.cleanup:
540532
bp = Path(build_path)
541533
for p in bp.rglob("*"):
542534
if p.is_file() and p.suffix in [".cpp", ".xml", ".obj", ".o"]:
@@ -548,23 +540,17 @@ def build_wheel(
548540

549541
def build_wrapped_itk(
550542
self,
551-
ninja_executable,
552-
build_type,
553-
itk_source_dir,
554543
build_path,
555-
python_executable,
556-
python_include_dir,
557-
python_library,
558544
):
559545

560546
# Build ITK python
561547
cmd = [
562548
"cmake",
563549
"-G",
564550
"Ninja",
565-
f"-DCMAKE_MAKE_PROGRAM:FILEPATH={ninja_executable}",
566-
f"-DCMAKE_BUILD_TYPE:STRING={build_type}",
567-
f"-DITK_SOURCE_DIR:PATH={itk_source_dir}",
551+
f"-DCMAKE_MAKE_PROGRAM:FILEPATH={self.venv_info_dict['ninja_executable']}",
552+
f"-DCMAKE_BUILD_TYPE:STRING={self._build_type}",
553+
f"-DITK_SOURCE_DIR:PATH={self.ITK_SOURCE_DIR}",
568554
f"-DITK_BINARY_DIR:PATH={build_path}",
569555
"-DBUILD_TESTING:BOOL=OFF",
570556
]
@@ -587,13 +573,23 @@ def build_wrapped_itk(
587573

588574
# Python settings
589575
cmd.append("-DSKBUILD:BOOL=ON")
590-
cmd.append(f"-DPython3_EXECUTABLE:FILEPATH={python_executable}")
591-
if python_include_dir:
592-
cmd.append(f"-DPython3_INCLUDE_DIR:PATH={python_include_dir}")
593-
cmd.append(f"-DPython3_INCLUDE_DIRS:PATH={python_include_dir}")
594-
if python_library:
595-
cmd.append(f"-DPython3_LIBRARY:FILEPATH={python_library}")
596-
cmd.append(f"-DPython3_SABI_LIBRARY:FILEPATH={python_library}")
576+
cmd.append(
577+
f"-DPython3_EXECUTABLE:FILEPATH={self.venv_info_dict['python_executable']}"
578+
)
579+
if self.venv_info_dict["python_include_dir"]:
580+
cmd.append(
581+
f"-DPython3_INCLUDE_DIR:PATH={self.venv_info_dict['python_include_dir']}"
582+
)
583+
cmd.append(
584+
f"-DPython3_INCLUDE_DIRS:PATH={self.venv_info_dict['python_include_dir']}"
585+
)
586+
if self.venv_info_dict["python_library"]:
587+
cmd.append(
588+
f"-DPython3_LIBRARY:FILEPATH={self.venv_info_dict['python_library']}"
589+
)
590+
cmd.append(
591+
f"-DPython3_SABI_LIBRARY:FILEPATH={self.venv_info_dict['python_library']}"
592+
)
597593

598594
# ITK wrapping options
599595
cmd += [
@@ -611,9 +607,9 @@ def build_wrapped_itk(
611607
f"-DModule_ITKTBB:BOOL={self._use_tbb}",
612608
f"-DTBB_DIR:PATH={self._tbb_dir}",
613609
"-S",
614-
itk_source_dir,
610+
self.ITK_SOURCE_DIR,
615611
"-B",
616612
build_path,
617613
]
618614
echo_check_call(cmd)
619-
echo_check_call([ninja_executable, "-C", build_path])
615+
echo_check_call([self.venv_info_dict["ninja_executable"], "-C", build_path])

0 commit comments

Comments
 (0)