Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes incorrect read/write permissions on Flutter.framework and FlutterMacOS.framework #148580

Merged
merged 6 commits into from
May 20, 2024

Conversation

LouiseHsu
Copy link
Contributor

@LouiseHsu LouiseHsu commented May 17, 2024

Fixes #148354

Fixes #147142
Closes #147144

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
  • All existing and new tests are passing.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label May 17, 2024
@LouiseHsu LouiseHsu changed the title before tests Fixes incorrect read/write permissions on Flutter.framework and FlutterMacOS.framework May 17, 2024
@LouiseHsu LouiseHsu marked this pull request as ready for review May 17, 2024 23:33
@LouiseHsu LouiseHsu requested a review from jmagman May 17, 2024 23:38
@nilsreichardt
Copy link
Contributor

Thanks for the fix! When this fix is merged, can we cherry pick it and release it as a hotfix since many Flutter users are affected and can't publish MacOS apps with Flutter v3.22.0 anymore?

Comment on lines 167 to 174
final File flutterMacOSFramework = fileSystem.file(
fileSystem.path.join(
outputApp.path,
'Contents',
'Frameworks',
'FlutterMacOS.framework',
),
);
Copy link
Member

Choose a reason for hiding this comment

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

outputFlutterFramework is the same thing immediately above, so you should be able to use that. And it's a directory, not a file.

@@ -456,6 +456,19 @@ void main() {
],
);
expect(appCodesign, const ProcessResultMatcher());

final File flutterIOSFramework = fileSystem.file(
Copy link
Member

Choose a reason for hiding this comment

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

flutterFramework is already the same thing, you should be able to use that, and it's a directory.

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

LGTM

@jmagman jmagman added the autosubmit Merge PR when tree becomes green via auto submit App label May 20, 2024
Copy link
Contributor

auto-submit bot commented May 20, 2024

auto label is removed for flutter/flutter/148580, due to - The status or check suite Mac_arm64 tool_host_cross_arch_tests has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 20, 2024
Comment on lines 433 to 436
final String flutterFramework = fileSystem.path.join(
appBundle.path,
'Frameworks',
'Flutter.framework',
flutterFrameworkDir.path,
'Flutter',
);
Copy link
Member

@jmagman jmagman May 20, 2024

Choose a reason for hiding this comment

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

Shouldn't this be:

      final String flutterFramework = fileSystem.path.join(
        flutterFrameworkDir.path,
        'Flutter',
      );

@LouiseHsu LouiseHsu added the autosubmit Merge PR when tree becomes green via auto submit App label May 20, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 20, 2024
Copy link
Contributor

auto-submit bot commented May 20, 2024

auto label is removed for flutter/flutter/148580, due to - The status or check suite Linux tool_integration_tests_4_4 has failed. Please fix the issues identified (or deflake) before re-applying this label.

@LouiseHsu LouiseHsu added the autosubmit Merge PR when tree becomes green via auto submit App label May 20, 2024
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 20, 2024
Copy link
Contributor

auto-submit bot commented May 20, 2024

auto label is removed for flutter/flutter/148580, due to - The status or check suite Mac tool_integration_tests_4_4 has failed. Please fix the issues identified (or deflake) before re-applying this label.

@github-actions github-actions bot added the engine flutter/engine repository. See also e: labels. label May 20, 2024
@github-actions github-actions bot removed the engine flutter/engine repository. See also e: labels. label May 20, 2024
@LouiseHsu LouiseHsu added the autosubmit Merge PR when tree becomes green via auto submit App label May 20, 2024
@LouiseHsu LouiseHsu merged commit 185f526 into flutter:master May 20, 2024
130 checks passed
@LouiseHsu LouiseHsu added the cp: stable cherry pick this pull request to stable release candidate branch label May 20, 2024
flutteractionsbot pushed a commit to flutteractionsbot/flutter that referenced this pull request May 20, 2024
…erMacOS.framework (flutter#148580)

Fixes flutter#148354

Fixes flutter#147142
Closes flutter#147144

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.
auto-submit bot pushed a commit to flutter/engine that referenced this pull request May 20, 2024
During FlutterMacOS.framework engine universal fat framework creation, make sure the framework is `u=rwx,go=rx` when it's created.

There's a framework-side workaround in-flight flutter/flutter#148580. 

However, this is still a good thing to do engine-side, particularly for add-to-app when they don't go through the `flutter assemble` tooling.

I'll also add a test around here, once this rolls https://github.com/flutter/flutter/blob/02a6c91e4d37d28f42c8f8e4d4335b0defed41c1/packages/flutter_tools/test/host_cross_arch.shard/macos_content_validation_test.dart#L30

Fixes flutter/flutter#148354

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2024
@jmagman
Copy link
Member

jmagman commented May 21, 2024

Thanks for the fix! When this fix is merged, can we cherry pick it and release it as a hotfix since many Flutter users are affected and can't publish MacOS apps with Flutter v3.22.0 anymore?

@nilsreichardt Yes, this will be hotfixed to 3.22. More details at #148354 (comment)

flutteractionsbot pushed a commit to flutteractionsbot/engine that referenced this pull request May 21, 2024
During FlutterMacOS.framework engine universal fat framework creation, make sure the framework is `u=rwx,go=rx` when it's created.

There's a framework-side workaround in-flight flutter/flutter#148580. 

However, this is still a good thing to do engine-side, particularly for add-to-app when they don't go through the `flutter assemble` tooling.

I'll also add a test around here, once this rolls https://github.com/flutter/flutter/blob/02a6c91e4d37d28f42c8f8e4d4335b0defed41c1/packages/flutter_tools/test/host_cross_arch.shard/macos_content_validation_test.dart#L30

Fixes flutter/flutter#148354

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
@nilsreichardt
Copy link
Contributor

@nilsreichardt Yes, this will be hotfixed to 3.22. More details at #148354 (comment)

Thanks, that's awesome <3

auto-submit bot pushed a commit to flutter/packages that referenced this pull request May 21, 2024
flutter/flutter@02a6c91...d02292d

2024-05-21 ian@hixie.ch Make FileSystem dependency explicit througout (more). (flutter/flutter#148095)
2024-05-20 magder@google.com Remove add-to-app bitcode warning (flutter/flutter#148587)
2024-05-20 rmolivares@renzo-olivares.dev SelectionArea's selection should not be cleared on loss of window focus (flutter/flutter#148067)
2024-05-20 katelovett@google.com [wiki migration] Engine team pages (flutter/flutter#148696)
2024-05-20 goderbauer@google.com Manual roll camera dependency (flutter/flutter#148426)
2024-05-20 katelovett@google.com [wiki migration] Framework team pages (flutter/flutter#148721)
2024-05-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from a8fb9daae8d0 to c2ef01f6f1ab (3 revisions) (flutter/flutter#148722)
2024-05-20 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.5 to 3.25.6 (flutter/flutter#148715)
2024-05-20 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 4.4.0 to 4.4.1 (flutter/flutter#148714)
2024-05-20 louisehsu@google.com Fixes incorrect read/write permissions on Flutter.framework and FlutterMacOS.framework (flutter/flutter#148580)
2024-05-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from c6fecf65fbf3 to a8fb9daae8d0 (3 revisions) (flutter/flutter#148700)
2024-05-20 jason-simmons@users.noreply.github.com Remove the no-shuffle tag on the flutter_tools create_test suite (flutter/flutter#148688)
2024-05-20 andrewrkolos@gmail.com log incoming vm service messages in `FlutterVMService::runInView` (flutter/flutter#148596)
2024-05-20 sokolovskyi.konstantin@gmail.com Add tests for shared_app_data.#.dart API examples. (flutter/flutter#147830)
2024-05-20 sokolovskyi.konstantin@gmail.com Add tests for logical_key_set.0.dart API example. (flutter/flutter#147735)
2024-05-20 katelovett@google.com [wiki migration] Ecosystem team pages (flutter/flutter#148589)
2024-05-20 sokolovskyi.konstantin@gmail.com Fix painting API examples tests directories structure. (flutter/flutter#148177)
2024-05-20 102401667+Dimilkalathiya@users.noreply.github.com fixes `CupertinoModalPopupRoute` (flutter/flutter#147823)
2024-05-20 nate.w5687@gmail.com Implement new `AnimationStatus` getters (flutter/flutter#148570)
2024-05-20 nate.w5687@gmail.com Reland "`if` chains � `switch` expressions" (flutter/flutter#148634)
2024-05-20 gspencergoog@users.noreply.github.com Factor out `RawView`, make `View` listen to engine generated view focus events (flutter/flutter#143259)
2024-05-20 zanderso@users.noreply.github.com Remove all tests from a02s. Replace with mokey in bringup (flutter/flutter#148563)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC rmistry@google.com,stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
hello-coder-xu added a commit to hello-coder-xu/flutter that referenced this pull request May 22, 2024
…-coder-xu/flutter into fix/slider_text_null_error

* 'fix/slider_text_null_error' of https://github.com/hello-coder-xu/flutter: (65 commits)
  Change implementation method
  fix slider text null error
  Make FileSystem dependency explicit througout (more). (flutter#148095)
  Remove add-to-app bitcode warning (flutter#148587)
  SelectionArea's selection should not be cleared on loss of window focus (flutter#148067)
  [wiki migration] Engine team pages (flutter#148696)
  Manual roll camera dependency (flutter#148426)
  [wiki migration] Framework team pages (flutter#148721)
  Roll Flutter Engine from a8fb9daae8d0 to c2ef01f6f1ab (3 revisions) (flutter#148722)
  Bump github/codeql-action from 3.25.5 to 3.25.6 (flutter#148715)
  Bump codecov/codecov-action from 4.4.0 to 4.4.1 (flutter#148714)
  Fixes incorrect read/write permissions on Flutter.framework and FlutterMacOS.framework (flutter#148580)
  Roll Flutter Engine from c6fecf65fbf3 to a8fb9daae8d0 (3 revisions) (flutter#148700)
  Remove the no-shuffle tag on the flutter_tools create_test suite (flutter#148688)
  log incoming vm service messages in `FlutterVMService::runInView` (flutter#148596)
  Add tests for shared_app_data.#.dart API examples. (flutter#147830)
  Add tests for logical_key_set.0.dart API example. (flutter#147735)
  [wiki migration] Ecosystem team pages (flutter#148589)
  Fix painting API examples tests directories structure. (flutter#148177)
  fixes `CupertinoModalPopupRoute` (flutter#147823)
  ...
TecHaxter pushed a commit to TecHaxter/flutter_packages that referenced this pull request May 22, 2024
…r#6778)

flutter/flutter@02a6c91...d02292d

2024-05-21 ian@hixie.ch Make FileSystem dependency explicit througout (more). (flutter/flutter#148095)
2024-05-20 magder@google.com Remove add-to-app bitcode warning (flutter/flutter#148587)
2024-05-20 rmolivares@renzo-olivares.dev SelectionArea's selection should not be cleared on loss of window focus (flutter/flutter#148067)
2024-05-20 katelovett@google.com [wiki migration] Engine team pages (flutter/flutter#148696)
2024-05-20 goderbauer@google.com Manual roll camera dependency (flutter/flutter#148426)
2024-05-20 katelovett@google.com [wiki migration] Framework team pages (flutter/flutter#148721)
2024-05-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from a8fb9daae8d0 to c2ef01f6f1ab (3 revisions) (flutter/flutter#148722)
2024-05-20 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.5 to 3.25.6 (flutter/flutter#148715)
2024-05-20 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 4.4.0 to 4.4.1 (flutter/flutter#148714)
2024-05-20 louisehsu@google.com Fixes incorrect read/write permissions on Flutter.framework and FlutterMacOS.framework (flutter/flutter#148580)
2024-05-20 engine-flutter-autoroll@skia.org Roll Flutter Engine from c6fecf65fbf3 to a8fb9daae8d0 (3 revisions) (flutter/flutter#148700)
2024-05-20 jason-simmons@users.noreply.github.com Remove the no-shuffle tag on the flutter_tools create_test suite (flutter/flutter#148688)
2024-05-20 andrewrkolos@gmail.com log incoming vm service messages in `FlutterVMService::runInView` (flutter/flutter#148596)
2024-05-20 sokolovskyi.konstantin@gmail.com Add tests for shared_app_data.#.dart API examples. (flutter/flutter#147830)
2024-05-20 sokolovskyi.konstantin@gmail.com Add tests for logical_key_set.0.dart API example. (flutter/flutter#147735)
2024-05-20 katelovett@google.com [wiki migration] Ecosystem team pages (flutter/flutter#148589)
2024-05-20 sokolovskyi.konstantin@gmail.com Fix painting API examples tests directories structure. (flutter/flutter#148177)
2024-05-20 102401667+Dimilkalathiya@users.noreply.github.com fixes `CupertinoModalPopupRoute` (flutter/flutter#147823)
2024-05-20 nate.w5687@gmail.com Implement new `AnimationStatus` getters (flutter/flutter#148570)
2024-05-20 nate.w5687@gmail.com Reland "`if` chains � `switch` expressions" (flutter/flutter#148634)
2024-05-20 gspencergoog@users.noreply.github.com Factor out `RawView`, make `View` listen to engine generated view focus events (flutter/flutter#143259)
2024-05-20 zanderso@users.noreply.github.com Remove all tests from a02s. Replace with mokey in bringup (flutter/flutter#148563)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC rmistry@google.com,stuartmorgan@google.com,tarrinneal@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@nilsreichardt
Copy link
Contributor

nilsreichardt commented May 23, 2024

Verified that it works with Flutter v3.22.1 👍 See: https://github.com/SharezoneApp/sharezone-app/actions/runs/9207185401/job/25328215964

@itsjustkevin I think this change is missing in the changelog of v3.22.1

@jmagman
Copy link
Member

jmagman commented May 23, 2024

@nilsreichardt Thanks for validating! You're right about the changelog, I'll follow up.

itsjustkevin added a commit that referenced this pull request May 30, 2024
Update changelog to include
#148580 in the list of fixes.
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App cp: stable cherry pick this pull request to stable release candidate branch tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
3 participants