Skip to content

Regex anchor CMake PORTABLE match - #15577

Open
jtmoon79 wants to merge 1 commit into
meta:mainfrom
jtmoon79:jtmoon79/20261103_cmake_PORTABLE_match
Open

jtmoon79 wants to merge 1 commit into
meta:mainfrom
jtmoon79:jtmoon79/20261103_cmake_PORTABLE_match

Conversation

@jtmoon79

@jtmoon79 jtmoon79 commented Nov 3, 2026

Copy link
Copy Markdown

PORTABLE MATCHES "1|ON|YES|TRUE|Y" is an unanchored regex, so any arch name containing one of those substrings is treated as a boolean instead of being passed to -march=. For example, PORTABLE=znver1 or skylake-avx512 builds as PORTABLE=1, and armv8.0-a builds as -march=native. Anchor both patterns so that only the exact boolean values match.

I've affirmed the CLA.

`PORTABLE MATCHES "1|ON|YES|TRUE|Y"` is an unanchored regex, so any
arch name containing one of those substrings is treated as a boolean
instead of being passed to -march=. For example, PORTABLE=znver1 or
skylake-avx512 builds as PORTABLE=1, and armv8.0-a builds as -march=native.
Anchor both patterns so that only the exact boolean values match.

Co-Authored-By: Claude Code <[email protected]>
Copilot AI balanced review requested due to automatic review settings November 3, 2026 20:24
@meta-cla meta-cla Bot added the CLA Signed label Nov 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are recorded for the change to CMake’s PORTABLE handling.

No open issues 🎉

What changed in this PR

Fixes CMake’s PORTABLE handling so CPU architecture names are not mistaken for boolean values.

Changes:

  • Anchors both boolean regular expressions to match only complete values.
  • Preserves explicit architecture names, such as znver1 and armv8.0-a, for use with -march=.
File Description
CMakeLists.txt Anchors the true and false patterns used to interpret PORTABLE.

Review effort: Balanced

This branch has not been deployed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants