Skip to content

Allow CMAKE_EXPERIMENTAL_CXX_IMPORT_STD to be overridden - #11

Closed
DockedFerret800 wants to merge 1 commit into
openalgz:mainfrom
DockedFerret800:update-experimental-string
Closed

Allow CMAKE_EXPERIMENTAL_CXX_IMPORT_STD to be overridden#11
DockedFerret800 wants to merge 1 commit into
openalgz:mainfrom
DockedFerret800:update-experimental-string

Conversation

@DockedFerret800

Copy link
Copy Markdown
Contributor

Allows CMAKE_EXPERIMENTAL_CXX_IMPORT_STD to be overridden. After merging #9, this can be replaced with a newer syntax from CMake 4.2: set(CACHE{CMAKE_EXPERIMENTAL_CXX_IMPORT_STD} VALUE "f35a9ac6-8463-4d38-8eec-5d6008153e7d").

@stephenberry

Copy link
Copy Markdown
Contributor

You found a real bug here. Confirmed on current main:

-- BEFORE set:      'USER-VALUE'   cache='USER-VALUE'
-- AFTER plain set: 'TABLE-VALUE'  cache='USER-VALUE'   <- -D silently discarded

The plain set() creates a normal variable that shadows the cache entry -D produces, so -DCMAKE_EXPERIMENTAL_CXX_IMPORT_STD=... was being ignored for every release listed in the table, leaving no escape hatch if an entry is ever wrong. That needed fixing.

The one problem with CACHE STRING "" specifically is that a non-FORCE cache write won't update an existing entry, so the UUID gets pinned in CMakeCache.txt. Upgrading CMake in an existing build tree then keeps the old value and the table's corrected one never applies:

### 1st configure:                    EFFECTIVE='UUID-FOR-CMAKE-4.3'
### reconfigure after table updates:  EFFECTIVE='UUID-FOR-CMAKE-4.3'   <- stale, silent

That's the same failure the table was added to prevent, in a quieter form.

#12 takes your fix and applies it as a guard instead, which gets the override without touching the cache:

if(UT_IMPORT_STD_GATE AND NOT DEFINED CMAKE_EXPERIMENTAL_CXX_IMPORT_STD)
scenario result
no -D table applies
-D <uuid> override honored
table bumped, same build tree new value applies — no staleness
reconfigure without repeating -D user value persists

On the set(CACHE{VAR} VALUE ...) suggestion in your description: that syntax needs CMake 4.2, which would raise the floor from cmake_minimum_required(VERSION 3.31), so it isn't a drop-in for now — worth revisiting if the minimum ever moves.

Closing in favour of #12, which is your fix with a different mechanism. Please do review it — if you see a case the guard misses that the cache form handled, I'd rather hear it before it merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants