Conversation
…tall Some conda Python builds on macOS inject the same -Wl,-rpath,$CONDA_PREFIX/lib fragment more than once via Python/sysconfig linker flags. During extension builds, that duplication is propagated into generated .so files as duplicate LC_RPATH load commands. Applicable scope: macOS builds environments where Python/linker config emits duplicate rpath flags most visible in conda-based developer installs Manifestation: make test fails during import/collection across many modules errors look like: ImportError: dlopen(...): duplicate LC_RPATH '.../env/lib' Solution: add a macOS-only post-install Make target invoked from install scan built rmgpy/*.so extensions if more than one LC_RPATH points to $CONDA_PREFIX/lib, remove extras with install_name_tool -delete_rpath until one remains Why this works: the failure is triggered by duplicate runtime search paths embedded in Mach-O extension binaries removing redundant LC_RPATH entries keeps the intended search path while satisfying the dynamic loader’s constraints fix is local, repeatable, and avoids patching user Python/conda internals
…fter install" This reverts commit b27610cc6b5ab8c0bbf124bedeb86fe7e437bd4a. The fix works, but it causes pip to re-compile all the Cython extensions every time, which is a huge slowdown. We need to find a better way to fix the duplicate LC_RPATH entries without modifying the install directory after the fact, which I will do in a subsequent commit.
On macOS, Python builds from conda-forge can include duplicate -Wl,-rpath entries in sysconfig variables (LDFLAGS, LDSHARED, BLDSHARED, PY_LDFLAGS). When these flags are used to compile extension modules, the duplicates propagate into the resulting Mach-O binaries as repeated LC_RPATH load commands. Recent macOS versions (e.g., Sequoia / 15.x) reject binaries with duplicate LC_RPATH, causing import failures such as: ImportError: dlopen(...): duplicate LC_RPATH '...' This change defensively deduplicates linker flags at build time by patching the in-memory sysconfig variables before setuptools/build_ext constructs the linker command. This ensures that generated extension modules contain only unique RPATH entries. Key points: Applies only on macOS (sys.platform == 'darwin') Leaves the conda environment and installed Python untouched Prevents duplicate LC_RPATH in compiled .so files Fixes import errors without requiring changes to user environments or toolchains This is a targeted workaround for upstream sysconfig duplication issues in conda-based Python builds.
|
The extended description for the commit causing this behavior explains: We may be able to undo this change (as done in this PR) with the latest Julia installation system. The CI will tell us shortly (the |
Co-authored-by: rwest <93807+rwest@users.noreply.github.com> Agent-Logs-Url: https://github.com/ReactionMechanismGenerator/RMG-Py/sessions/ab30faf9-6de6-465b-a312-014510c5497f
The goal is that `make` will install it if needed (with an editable install, which just links to the current source), but if it's already installed, then it'll just do an incremental build. Although calling setuptools directly is deprecated, it is one of the only clean ways to get incremental builds of the cython modules.
Updated .conda/meta.yaml to match the important version constraints from environment.yml: cantera from =2.6 to >=3.0 Also used a YAML anchor so host, run, and test.requires share one source of truth, instead of duplicated lists. Also removed the repeated graphviz entry
It was in there twice
That's what `git log - environment.yml` is helpful for. This list was out of date, and maintaining it is extra burden. Replaced it with a reminder to update the .conda/meta.yaml
Inspired by wanting some way to check the environment stays in sync.
rwest
force-pushed
the
copilot/restore-incremental-builds-cython-extensions
branch
from
81a6163 to
9ea1e25
Compare
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:00:53 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Failed Edge Comparison ❌Original model has 106 species. Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)(Cds-Cds)CsH) + group(Cs-(Cds-Cds)(Cds-Cds)CsH) + group(Cs-CsCsHH) + group(Cds-CdsCsCs) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + Estimated bicyclic component: polycyclic(s2_3_5_ane) - ring(Cyclopropane) - ring(Cyclopentane) + ring(Cyclopentene) + ring(Cyclopropane) + polycyclic(s2_3_6_ene_1) + polycyclic(s3_5_6_diene_1_5) - ring(Cyclopropane) - ring(Cyclopentene) - ring(Cyclohexene) + radical(cyclopentene-4) Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-CsCsCsH) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)CsCsH) + group(Cs-(Cds-Cds)CsHH) + group(Cds- Cds(Cds-Cds)Cs) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-Cds(Cds-Cds)H) + polycyclic(s2_3_5_ene_1) + polycyclic(s2_3_6_ene_1) + Estimated bicyclic component: polycyclic(s3_5_6_ane) - ring(Cyclopentane) - ring(Cyclohexane) + ring(Cyclopentene) + ring(Cyclohexene) - ring(Cyclopropane) - ring(Cyclopentene) - ring(Cyclohexene) + radical(cyclopentene-allyl) Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: DetailsObservables Test Case: Aromatics Comparison✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! aromatics Passed Observable Testing ✅Regression test liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:01:53 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 214 species. Non-identical kinetics! ❌
kinetics: DetailsObservables Test Case: liquid_oxidation Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! liquid_oxidation Passed Observable Testing ✅Regression test nitrogen:Reference: Execution time (DD:HH:MM:SS): 00:00:00:59 nitrogen Passed Core Comparison ✅Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 133 species. Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(O2s-CdN3d) + group(N3d-OCd) + group(Cd-HN3dO) + ring(Cyclopropene) + radical(CdJ-NdO) Non-identical kinetics! ❌
kinetics: DetailsObservables Test Case: NC Comparison✅ All Observables varied by less than 0.200 on average between old model and new model in all conditions! nitrogen Passed Observable Testing ✅Regression test oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:01:39 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species. DetailsObservables Test Case: Oxidation Comparison✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! oxidation Passed Observable Testing ✅Errors occurred during observable testing
WARNING:root:Initial mole fractions do not sum to one; normalizing.
|
makewas invokingpip install -e .on every run, causing full recompilation of all Cython extensions regardless of what changed. This replaces it withsetup.py build_ext --inplace, which respects timestamps and only rebuilds modified extensions.Changes
buildtarget (new): runspython setup.py build_ext --inplacefor incremental compilationinstalltarget (unchanged) still runspip install --no-build-isolation -vv -e .for initial setup or dependency changes.alltarget (default): installs if it is not already installed (first time, or after aclean), but otherwise runs the incrementalbuild. Uses a sentinel file to tell whether it's installed.cleantarget: uninstalls and deletes the sentinel file.Running
makeonce installs it.Running
maketwice without source changes no longer triggers recompilation.Running
makeafter editing a single cythonized source file triggers recompilation of that file.While wondering why the Conda build recipes in CI were failing, I also noticed that the conda recipe (.conda/meta.yaml) was out of date and hadn't had the Cantera update made. So I fixed that too.
We'll see if the Conda build starts working again - it might be unrelated, but still needed fixing.
First commit was by the github co-pilot, but subsequent ones by @rwest (with some guidance from Claude and copilot)
Fixes #2901
I rebased this onto #2901 since that is also fixing build issues. I think that should be merged first.