Sitelet https://github.com/flutter/flutter/pull/189658
Skip to content

fix(tool): make copied frameworks writable to fix macOS 15.4+ openrsync regression - #189658

Merged
auto-submit[bot] merged 12 commits into
flutter:masterfrom
Kryonex-Labs:fix/openrsync-chmod
Aug 19, 2026
Merged

fix(tool): make copied frameworks writable to fix macOS 15.4+ openrsync regression#189658
auto-submit[bot] merged 12 commits into
flutter:masterfrom
Kryonex-Labs:fix/openrsync-chmod

Conversation

@GhagSagar23

@GhagSagar23 GhagSagar23 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes a regression of #147142 where read-only Flutter SDK builds fail on macOS 15.4+.

Root Cause Analysis

Starting in macOS 15.4+, Apple transitioned from GNU rsync to BSD-licensed openrsync. A major behavioral difference is that openrsync silently ignores the --chmod flag during local directory synchronization.

As a result, the --chmod parameter passed to rsync during framework copy operations is ignored, and framework binaries inherit the read-only permissions from the read-only SDK cache (e.g. Nix store or system-wide read-only path). Subsequent lipo and codesigning operations fail with a Permission denied error.

Solution

This PR adds an explicit chmod -R u+w execution on the copied framework/xcframework target path immediately after rsync copy completes to ensure writability, completely decoupling file writability from rsync's specific parameters.

Fixes #189645

@GhagSagar23
GhagSagar23 requested review from a team as code owners July 17, 2026 13:46
@flutter-dashboard

flutter-dashboard Bot commented Jul 17, 2026

Copy link
Copy Markdown

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@google-cla

google-cla Bot commented Jul 17, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions Bot added tool Affects the "flutter" command-line tool. See also t: labels. team-ios Owned by iOS platform team team-macos Owned by the macOS platform team labels Jul 17, 2026
gemini-code-assist[bot]
gemini-code-assist Bot reviewed Jul 17, 2026
View reviewed changes

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request adds steps to explicitly make copied frameworks and XCFrameworks writable by running chmod -R u+w on their paths. The feedback suggests wrapping these external process calls in try-catch blocks to prevent potential unhandled ProcessException crashes on non-POSIX host platforms or in restricted environments.

Comment thread packages/flutter_tools/lib/src/build_system/targets/darwin.dart Outdated
Comment thread packages/flutter_tools/lib/src/commands/build_swift_package.dart Outdated
cbracken
cbracken reviewed Jul 21, 2026
View reviewed changes

@cbracken cbracken left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thinks for adding the exception handling susgested by the review bot.

The updated code lgtm modulo my one nit re printTrace vs throwing ToolExit.

As noted by the test checker bot, this will need tests before it can land.

Comment thread packages/flutter_tools/lib/src/build_system/targets/darwin.dart
@cbracken
cbracken requested a review from vashworth July 21, 2026 07:08
@cbracken

cbracken commented Jul 21, 2026

Copy link
Copy Markdown
Member

This will need a second reviewer. Added @vashworth for a pass once the tests have been added.

@GhagSagar23

GhagSagar23 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

I have pushed the requested changes. The chmod command is now wrapped in a try-catch block, throwing a ToolExit on failure. I've also added/updated test expectations across macos_test.dart, ios_test.dart, and build_swift_package_test.dart to ensure the tests correctly expect chmod for the framework.

@vashworth

vashworth commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

@GhagSagar23 Just checking to see if you saw my comment: #189658 (comment)

@GhagSagar23

GhagSagar23 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

@GhagSagar23 Just checking to see if you saw my comment: #189658 (comment)

Hello @vashworth, will be pushing the code by tomorrow EOD.

@GhagSagar23

GhagSagar23 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

@GhagSagar23 Just checking to see if you saw my comment: #189658 (comment)

Hi @vashworth, thanks for the suggestion. I’ve updated the implementation to use printXcodeWarning instead of failing with a ToolExit. Both a non-zero chmod exit and a ProcessException now emit a non-fatal warning explaining that the build may subsequently fail when using lipo. I’ve applied the same behavior to the XCFramework copy path as well. The focused Flutter tool tests are passing.

vashworth
vashworth reviewed Aug 6, 2026
View reviewed changes
Comment thread packages/flutter_tools/lib/src/commands/build_swift_package.dart Outdated
@vashworth vashworth added the CICD Run CI/CD label Aug 13, 2026
@vashworth
vashworth requested a review from cbracken August 13, 2026 20:22
@flutter-dashboard flutter-dashboard Bot removed the CICD Run CI/CD label Aug 15, 2026
@GhagSagar23

GhagSagar23 commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

Hi @cbracken, can you please review and merge this PR?

@cbracken cbracken added the CICD Run CI/CD label Aug 16, 2026
@vashworth vashworth added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 19, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Aug 19, 2026
Merged via the queue into flutter:master with commit 9204165 Aug 19, 2026
28 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 19, 2026
This was referenced Aug 20, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cbracken cbracken cbracken approved these changes
@vashworth vashworth vashworth approved these changes
+1 more reviewer
@gemini-code-assist gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

CICD Run CI/CD team-ios Owned by iOS platform team team-macos Owned by the macOS platform team tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Regression of #147142: read-only SDK builds fail on macOS 15.4+ — the rsync --chmod fix from #148580 is now a silent no-op

3 participants

@GhagSagar23 @cbracken @vashworth