build(macos): quote SHOULD_SIGN env expansion in install script#5185
Conversation
The unquoted $ENV{SHOULD_SIGN} expansion in the install(CODE ...) block
collapses to nothing when the env var is unset, leaving the if() with
only "STREQUAL \"true\"" and failing with "Unknown arguments specified".
Quoting the expansion makes the comparison safe whether SHOULD_SIGN is
set, empty, or unset.
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Improves robustness of macOS packaging/signing logic by safely comparing an environment variable even when it’s unset.
Changes:
- Quote
$ENV{SHOULD_SIGN}in theif()condition to avoid CMake parse/errors when the env var is empty/unset.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
ReenigneArcher
changed the title
Bundle ReportBundle size has no change ✅ |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5185 +/- ##
==========================================
- Coverage 17.81% 17.80% -0.01%
==========================================
Files 111 111
Lines 24143 24143
Branches 10687 10687
==========================================
- Hits 4301 4299 -2
- Misses 14681 16532 +1851
+ Partials 5161 3312 -1849
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
|
Description
The macOS
install(CODE ...)block incmake/packaging/macos.cmakecontains:When
SHOULD_SIGNis unset (the common case for local manual builds — the env var is only set by CI on publish-release runs), the unquoted expansion collapses to nothing and CMake sees:…which fails with
Unknown arguments specifiedat install time. The bundle is fully assembled by that point (fixup_bundle runs first), but the error makescmake --installexit non-zero and breaks scripts that chaininstallwithcpackor downstream packaging steps.Quote the expansion so the comparison is safe whether the env var is set, empty, or unset:
CI builds (where
SHOULD_SIGN=true) keep working identically because quoting a non-empty string is a no-op.Screenshot
N/A — build script change.
Issues Fixed or Closed
Roadmap Issues
Type of Change
Checklist
AI Usage