Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[darwin] Move common targets to common/BUILD.gn #44335

Merged

Conversation

cbracken
Copy link
Member

@cbracken cbracken commented Aug 3, 2023

Previously, some common Darwin framework targets existed in:
//flutter/shell/platform/darwin/BUILD.gn

This moves all targets into:
//flutter/shell/platform/darwin/common/BUILD.gn

We also de-duplicate targets with existing targets.

  • The flutter_channels target duplicates the existing framework_shared target.
  • The common target already includes the buffer conversions translation units,
    which are only used by the iOS embedder.

No test changes since there are no semantic changes, just a restructuring of build targets.

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 Hixie said 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.

Previously, some common Darwin framework targets existed in:
//flutter/shell/platform/darwin/BUILD.gn

This moves all targets into:
//flutter/shell/platform/darwin/common/BUILD.gn
@cbracken cbracken force-pushed the move-darwin-common-targets-to-common branch from 6b4a50a to 3fba7fe Compare August 3, 2023 18:34
@cbracken
Copy link
Member Author

cbracken commented Aug 3, 2023

Taking a quick look to see how much pain would be involved in renaming the unittest executable to framework_shared_unittests.

@cbracken
Copy link
Member Author

cbracken commented Aug 3, 2023

Pushed a second commit that renames the test target so we can add more to it later.

gaaclarke
gaaclarke approved these changes Aug 3, 2023
Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@cbracken cbracken merged commit ae2cd64 into flutter:main Aug 3, 2023
@cbracken cbracken deleted the move-darwin-common-targets-to-common branch August 3, 2023 20:21
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 3, 2023
eyebrowsoffire pushed a commit to flutter/flutter that referenced this pull request Aug 3, 2023
…131897)

flutter/engine@b08e141...0c1de9b

2023-08-03 [email protected] [web] fix clicks on merged semantic nodes
(flutter/engine#43620)
2023-08-03 [email protected] Remove WARNINGs from JNI load path as
we can't suppress them (flutter/engine#44348)
2023-08-03 [email protected] Roll Skia from 872dc53233cf to
bae32428c1c7 (3 revisions) (flutter/engine#44341)
2023-08-03 [email protected] Build iOS unittest target in unopt builds
(flutter/engine#44301)
2023-08-03 [email protected] [darwin] Move common targets to
common/BUILD.gn (flutter/engine#44335)
2023-08-03 [email protected] Roll Skia from 2babe68de295 to
872dc53233cf (2 revisions) (flutter/engine#44339)
2023-08-03 [email protected] Migrate GL calls of
GrBackend* (flutter/engine#44334)
2023-08-03 [email protected] Roll Skia from 36072a994f11 to
2babe68de295 (1 revision) (flutter/engine#44336)
2023-08-03 [email protected] Roll Clang from 07c592048780
to 020d2fb7711d (flutter/engine#44332)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on
the revert to ensure that a human
is aware of the problem.

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

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

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

---------

Co-authored-by: Zachary Anderson <[email protected]>
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
Previously, some common Darwin framework targets existed in: 
//flutter/shell/platform/darwin/BUILD.gn

This moves all targets into:
//flutter/shell/platform/darwin/common/BUILD.gn

The framework_shared target has been renamed framework_common for consistency with the directory name, and flutter_channels_unittests has been renamed framework_common_unittests since it's a reasonable target for adding other tests of common framework code.

We also de-duplicate targets with existing targets.
* The `flutter_channels` target duplicates the existing
`framework_shared` target.
* The `common` target already includes the buffer conversions
translation units,
  which are only used by the iOS embedder.

No test changes since there are no semantic changes, just a
restructuring of build targets.

## 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] and the [C++,
Objective-C, Java style guides].
- [X] 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 Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [X] I updated/added relevant documentation (doc comments with `///`).
- [X] I signed the [CLA].
- [X] All existing and new tests are passing.

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants