Skip to content

CMake: keep derived AMReX options in sync on re-configure - #1637

Open
ax3l wants to merge 1 commit into
BLAST-ImpactX:developmentfrom
ax3l:cmake-cache-staleness
Open

CMake: keep derived AMReX options in sync on re-configure#1637
ax3l wants to merge 1 commit into
BLAST-ImpactX:developmentfrom
ax3l:cmake-cache-staleness

Conversation

@ax3l

@ax3l ax3l commented Aug 26, 2026

Copy link
Copy Markdown
Member

AMReX_PIC, ABLASTR_POSITION_INDEPENDENT_CODE and AMReX_BUILD_SHARED_LIBS were written to the cache from the "on" branch of if(ImpactX_PYTHON OR BUILD_SHARED_LIBS). Those entries outlive the requirement that created them, so re-configuring an existing build directory with -DImpactX_PYTHON=OFF silently kept the cached ON and left AMReX a shared library with no Python bindings asking for it.

Set them as normal variables instead. With CMP0077 NEW — which we already request for the ABLASTR superbuild — the downstream option() calls honor a normal variable and skip creating the cache entry, so the requirement applies for exactly the configure that asks for it and leaves nothing behind for the next one. Explicit -D values from the caller still take precedence, because they are cache entries and we no longer FORCE over them.

Details

There is deliberately no else() branch. With nothing cached there is nothing to reset, and assigning OFF there would override ABLASTR's own ABLASTR_POSITION_INDEPENDENT_CODE default of ON and change what a fresh non-Python configure builds today.

AMReX_INSTALL keeps working: ABLASTR derives it from if(DEFINED AMReX_BUILD_SHARED_LIBS), and DEFINED is true for a normal variable, so the Python configure still gets AMReX_INSTALL=ON and install(EXPORT "pyAMReXTargets" ...) is generated.

Testing

Configured ImpactX_PYTHON=ON, re-configured the same directory with ImpactX_PYTHON=OFF, and compared against a fresh ImpactX_PYTHON=OFF configure. Probed the effective variable values plus the resulting amrex_3d/ablastr_3d target TYPE and POSITION_INDEPENDENT_CODE, since the fix intentionally keeps these values out of the cache.

fresh PYTHON=OFF ON → re-configure OFF
development AMReX_PIC=ON, amrex STATIC AMReX_PIC=ON, amrex SHARED
this PR AMReX_PIC=ON, amrex STATIC AMReX_PIC=ON, amrex STATIC

A re-configured build directory now matches a fresh one, and the fresh configure still matches what development produces. The ImpactX_PYTHON=ON configure is unchanged from development as well: amrex SHARED + PIC, ablastr STATIC + PIC, AMReX_INSTALL=ON.

Same class of issue as BLAST-WarpX/warpx#7204 on the WarpX side.

@ax3l ax3l changed the title CMake: keep derived AMReX options in sync on re-configure [WIP] CMake: keep derived AMReX options in sync on re-configure Aug 26, 2026
@ax3l ax3l added bug Something isn't working bug: affects latest release Bug also exists in latest release version install labels Aug 26, 2026
@codspeed-hq

codspeed-hq Bot commented Aug 26, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 70 untouched benchmarks


Comparing ax3l:cmake-cache-staleness (ba84b39) with development (ecf785f)

Open in CodSpeed

@ax3l ax3l changed the title [WIP] CMake: keep derived AMReX options in sync on re-configure CMake: keep derived AMReX options in sync on re-configure Sep 5, 2026
@ax3l
ax3l force-pushed the cmake-cache-staleness branch from d96cde1 to 196dc6f Compare September 5, 2026 05:31
`AMReX_PIC`, `ABLASTR_POSITION_INDEPENDENT_CODE` and
`AMReX_BUILD_SHARED_LIBS` were written to the cache from the "on" branch
of `if(ImpactX_PYTHON OR BUILD_SHARED_LIBS)`. Those entries outlive the
requirement that created them, so re-configuring an existing build
directory with `-DImpactX_PYTHON=OFF` kept the cached `ON` and produced
a build that differs from a fresh configure of the same command line:
AMReX stayed a shared library with no Python bindings asking for it.

Set them as normal variables instead. With `CMP0077` `NEW`, which we
already request for the ABLASTR superbuild, the `option()` calls
downstream honor a normal variable and skip creating the cache entry, so
the requirement applies for exactly the configure that asks for it and
leaves nothing behind for the next one. Explicit `-D` values from the
caller still take precedence, because they are cache entries and we no
longer `FORCE` over them.

No `else()` branch: with nothing cached there is nothing to reset, and
assigning `OFF` there would override ABLASTR's own
`ABLASTR_POSITION_INDEPENDENT_CODE` default of `ON` and change what a
fresh non-Python configure builds today.

Verified by configuring `ImpactX_PYTHON=ON`, re-configuring the same
directory with `ImpactX_PYTHON=OFF`, and comparing against a fresh
`ImpactX_PYTHON=OFF` configure. The effective values and the resulting
`amrex_3d`/`ablastr_3d` target type and `POSITION_INDEPENDENT_CODE` now
match the fresh configure exactly, and both match what `development`
produces for a fresh configure.

Same class of issue as
BLAST-WarpX/warpx#7204

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pg2X154siq9ZHxZqosufjp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug: affects latest release Bug also exists in latest release version bug Something isn't working install

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant