diff --git a/cppimport/__init__.py b/cppimport/__init__.py index 96f29d9..0690538 100644 --- a/cppimport/__init__.py +++ b/cppimport/__init__.py @@ -11,8 +11,8 @@ force_rebuild=False, # `force_rebuild` with multiple processes is not supported file_exts=[".cpp", ".c"], rtld_flags=ctypes.RTLD_LOCAL, - lock_suffix='.lock', - lock_timeout=10*60, + lock_suffix=".lock", + lock_timeout=10 * 60, remove_strict_prototypes=True, release_mode=os.getenv("CPPIMPORT_RELEASE_MODE", "0").lower() in ("true", "yes", "1"), @@ -59,16 +59,23 @@ def imp_from_filepath(filepath, fullname=None): module : the compiled and loaded Python extension module """ from cppimport.importer import ( + build_safely, is_build_needed, load_module, setup_module_data, - build_safely, try_load, ) + filepath = os.path.abspath(filepath) if fullname is None: fullname = os.path.splitext(os.path.basename(filepath))[0] module_data = setup_module_data(fullname, filepath) + # The call to try_load is necessary here because there are times when the + # only evidence a rebuild is needed comes from attempting to load an + # existing extension module. For example, if the extension was built on a + # different architecture or with different Python headers and will produce + # an error when loaded, then the load will fail. In that situation, we will + # need to rebuild. if is_build_needed(module_data) or not try_load(module_data): build_safely(filepath, module_data) load_module(module_data) @@ -110,11 +117,12 @@ def build_filepath(filepath, fullname=None): ext_path : the path to the compiled extension. """ from cppimport.importer import ( - is_build_needed, - setup_module_data, build_safely, + is_build_needed, load_module, + setup_module_data, ) + filepath = os.path.abspath(filepath) if fullname is None: fullname = os.path.splitext(os.path.basename(filepath))[0] diff --git a/cppimport/checksum.py b/cppimport/checksum.py index 6e717ac..57b8a07 100644 --- a/cppimport/checksum.py +++ b/cppimport/checksum.py @@ -45,6 +45,9 @@ def _load_checksum_trailer(module_data): except FileNotFoundError: logger.info("Failed to find compiled extension; rebuilding.") return None, None + except OSError: + logger.info("Checksum trailer invalid. Rebuilding.") + return None, None try: deps, old_checksum = json.loads(json_s) @@ -79,7 +82,7 @@ def _save_checksum_trailer(module_data, dep_filepaths, cur_checksum): # legal (see e.g. https://stackoverflow.com/questions/10106447). dump = json.dumps([dep_filepaths, cur_checksum]).encode("ascii") dump += _FMT.pack(len(dump), _TAG) - with open(module_data["ext_path"], "ab") as file: + with open(module_data["ext_path"], "ab", buffering=0) as file: file.write(dump) diff --git a/cppimport/importer.py b/cppimport/importer.py index 3388771..bd5dcbd 100644 --- a/cppimport/importer.py +++ b/cppimport/importer.py @@ -4,7 +4,8 @@ import sys import sysconfig from contextlib import suppress -from time import time, sleep +from time import sleep, time + import filelock import cppimport @@ -16,23 +17,31 @@ def build_safely(filepath, module_data): - """Protect against race conditions when multiple processes executing `template_and_build`""" - binary_path = module_data['ext_path'] - lock_path = binary_path + cppimport.settings['lock_suffix'] + """Protect against race conditions when multiple processes executing + `template_and_build`""" + binary_path = module_data["ext_path"] + lock_path = binary_path + cppimport.settings["lock_suffix"] - build_completed = lambda: os.path.exists(binary_path) and is_checksum_valid(module_data) + def build_completed(): + return os.path.exists(binary_path) and is_checksum_valid(module_data) t = time() # Race to obtain the lock and build. Other processes can wait - while not build_completed() and time() - t < cppimport.settings['lock_timeout']: + while not build_completed() and time() - t < cppimport.settings["lock_timeout"]: try: with filelock.FileLock(lock_path, timeout=1): if build_completed(): break template_and_build(filepath, module_data) except filelock.Timeout: - logging.debug(f'Could not obtain lock (pid {os.getpid()})') + logging.debug(f"Could not obtain lock (pid {os.getpid()})") + if cppimport.settings["force_rebuild"]: + raise ValueError( + "force_build must be False to build concurrently." + "This process failed to claim a filelock indicating that" + " a concurrent build is in progress" + ) sleep(1) if os.path.exists(lock_path): @@ -41,8 +50,10 @@ def build_safely(filepath, module_data): if not build_completed(): raise Exception( - f'Could not compile binary as lock already taken and timed out. Try increasing the timeout setting if ' - f'the build time is longer (pid {os.getpid()}).') + f"Could not compile binary as lock already taken and timed out." + f" Try increasing the timeout setting if " + f"the build time is longer (pid {os.getpid()})." + ) def template_and_build(filepath, module_data): @@ -112,7 +123,8 @@ def is_build_needed(module_data): def try_load(module_data): - """Try loading the module to test if it's not corrupt and for the correct architecture""" + """Try loading the module to test if it's not corrupt and for the correct + architecture""" try: load_module(module_data) return True diff --git a/tests/test_cppimport.py b/tests/test_cppimport.py index fdc3bb0..a81c8fd 100644 --- a/tests/test_cppimport.py +++ b/tests/test_cppimport.py @@ -43,14 +43,17 @@ def subprocess_check(test_code, returncode=0): stdout=subprocess.PIPE, stderr=subprocess.PIPE, ) - print(p.stdout.decode("utf-8")) - print(p.stderr.decode("utf-8")) + if len(p.stdout) > 0: + print(p.stdout.decode("utf-8")) + if len(p.stderr) > 0: + print(p.stderr.decode("utf-8")) assert p.returncode == returncode @contextlib.contextmanager def tmp_dir(files=None): - """Create a temporary directory and copy `files` into it. `files` can also include directories.""" + """Create a temporary directory and copy `files` into it. `files` can also + include directories.""" files = files if files else [] with TemporaryDirectory() as tmp_path: @@ -190,14 +193,16 @@ def test_import_hook(): def test_multiple_processes(): - with tmp_dir(['hook_test.cpp']) as tmp_path: + with tmp_dir(["tests/hook_test.cpp"]) as tmp_path: test_code = f""" import os; os.chdir('{tmp_path}'); import cppimport.import_hook; import hook_test; """ - processes = [Process(target=subprocess_check, args=(test_code, )) for i in range(100)] + processes = [ + Process(target=subprocess_check, args=(test_code,)) for i in range(100) + ] for p in processes: p.start()