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

[DO NOT LAND] Update DEPS to point to forked build_root #56631

Conversation

hellohuanlin
Copy link
Contributor

This is to kick off CI for flutter/buildroot#919

List which issues are fixed by this PR. You must list at least one issue.

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

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 and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

auto-submit bot pushed a commit that referenced this pull request Nov 16, 2024
Spot another clang-tidy linter failure from: #56631

In release mode, if we remove NSAssert, then weakFlutterEngine is not used at all. This should have been an XCTAssert rather than NSAssert in the first place. 

```
� Failures for clang-tidy on /Volumes/Work/s/w/ir/cache/builder/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm:
/Volumes/Work/s/w/ir/cache/builder/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm:239:5: error: Value stored to 'weakFlutterEngine' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
  239 |     weakFlutterEngine = flutterEngine;
      |     ^                   ~~~~~~~~~~~~~
/Volumes/Work/s/w/ir/cache/builder/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm:239:5: note: Value stored to 'weakFlutterEngine' is never read
  239 |     weakFlutterEngine = flutterEngine;
      |     ^                   ~~~~~~~~~~~~~
Suppressed 9240 warnings (9111 in non-user code, 129 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
```

*List which issues are fixed by this PR. You must list at least one issue.*
flutter/flutter#157837

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
@hellohuanlin hellohuanlin force-pushed the update_deps_for_nsassert_change_in_build_root branch from 8185a08 to 9d7634b Compare November 18, 2024 18:59
@hellohuanlin hellohuanlin force-pushed the update_deps_for_nsassert_change_in_build_root branch from 9d7634b to d6c9d3d Compare November 19, 2024 00:59
@chinmaygarde
Copy link
Member

Looks like this is no longer necessary. May I close it if your experiments are done?

@hellohuanlin
Copy link
Contributor Author

@chinmaygarde sorry let's keep it open - i still need it as it failed more cases and im distracted by another urgent bug and will circle back soon.

@hellohuanlin
Copy link
Contributor Author

Let me convert it to draft until then.

@hellohuanlin hellohuanlin marked this pull request as draft December 2, 2024 19:17
@flutter-dashboard
Copy link

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
Spot another clang-tidy linter failure from: flutter/engine#56631

In release mode, if we remove NSAssert, then weakFlutterEngine is not used at all. This should have been an XCTAssert rather than NSAssert in the first place. 

```
� Failures for clang-tidy on /Volumes/Work/s/w/ir/cache/builder/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm:
/Volumes/Work/s/w/ir/cache/builder/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm:239:5: error: Value stored to 'weakFlutterEngine' is never read [clang-analyzer-deadcode.DeadStores,-warnings-as-errors]
  239 |     weakFlutterEngine = flutterEngine;
      |     ^                   ~~~~~~~~~~~~~
/Volumes/Work/s/w/ir/cache/builder/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm:239:5: note: Value stored to 'weakFlutterEngine' is never read
  239 |     weakFlutterEngine = flutterEngine;
      |     ^                   ~~~~~~~~~~~~~
Suppressed 9240 warnings (9111 in non-user code, 129 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
```

*List which issues are fixed by this PR. You must list at least one issue.*
flutter#157837

*If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].*

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

Monorepo Migration Completed

TL;DR: Please migrate your PR to flutter/flutter

The flutter/engine repository has been migrated to the flutter/flutter repository. This PR will no longer be landed here and must be migrated. To migrate your PR to the flutter repository, please follow these steps:

  1. Create a patch for this PR:

    • Generate a patch set that represents the changes in this PR. The exact method will vary depending on your PR's history. If your PR includes merges, it is highly recommended that you rebase it onto main first.
    git format-patch $START_COMMIT..$END_COMMIT --stdout > combined_patch.patch
  2. Update the patch for the new engine location:

    • The engine source code now resides in the engine/ subdirectory within the flutter/flutter repository. You'll need to update the file paths in your patch accordingly.
    # Insert the `engine/` prefix to all paths. Note that this usage works on macOS and
    # linux versions of sed.
    sed -i.bak \
    -e 's|^\(diff --git a/\)\(.*\) b/\(.*\)|\1engine/\2 b/engine/src/flutter/\3|' \
    -e 's|^\(--- a/\)\(.*\)|\1engine/src/flutter/\2|' \
    -e 's|^\(\+\+\+ b/\)\(.*\)|\1engine/src/flutter/\2|' \
    combined_patch.patch
  3. Checkout and set up your Flutter development environment:

  4. Set up the engine development environment within the monorepo:

  5. Apply the patch to a new branch:

    • Create a new branch in your flutter/flutter repository.
    • Apply the updated patch to this branch:
    git apply combined_patch.patch
  6. Open a new PR:

    • Open a new PR in the flutter/flutter repository with your changes.
    • Reference this original PR in the new PR's description using flutter/engine#<PR_NUMBER>.

This is a canned message

@jtmcdole jtmcdole closed this Jan 14, 2025
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.

3 participants