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

Move libpng to //flutter/third_party/libpng #50571

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

zanderso
Copy link
Member

@zanderso zanderso commented Feb 12, 2024

This is an example of moving a dependency to //flutter/third_party that requires forward/aliasing through the secondary build directory. Without forwarding, and only making changes to the DEPS file and rewriting paths in the engine repo, a gn invocation gets the following error:

ERROR at //third_party/freetype2/BUILD.gn:85:5: Can't load input file.
    "//third_party/libpng",
    ^---------------------
Unable to load:
  /Users/zra/flutter/engine/src/third_party/libpng/BUILD.gn
I also checked in the secondary tree for:
  /Users/zra/flutter/engine/src/flutter/build/secondary/third_party/libpng/BUILD.gn

Another dependency, with a BUILD.gn file outside of the engine repo, is still referring to libpng by the old path. However, gn tells us that it is also checking under the secondary tree. We can put whatever we want in that BUILD.gn file to make freetype2 happy. In this case it's easy, we just add a build rule there that forwards to libpng's new location. There's only one wrinkle, which is that the dependency in the group target has to be public_deps rather than a simple deps. This is so that public_deps and public_configs of //flutter/third_party/libpng are correctly inherited by dependents of //third_party/libpng.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

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.

Copy link
Contributor

@matanlurey matanlurey left a comment

Choose a reason for hiding this comment

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

TIL. Thanks Zach, I'll try applying this once and see if I get it

(and I'll add it to the FAQ).

@zanderso zanderso added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 13, 2024
@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #50571 at sha 01b185b

@zanderso zanderso removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 13, 2024
@zanderso zanderso merged commit fc500b2 into flutter:main Feb 13, 2024
28 checks passed
@zanderso zanderso deleted the shift-libpng branch February 13, 2024 15:08
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 13, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 13, 2024
…sions) (#143380)

Manual roll Flutter Engine from 1c3ecee77350 to 920874bf2c1a (34 revisions)

Manual roll requested by [email protected]

Cannot build log URL because revision "920874bf2c1a" is invalid: Luci builds of "Linux Fuchsia FEMU" for 920874bf2c1a9ba2e9b7260b79ede8f1e26ea4f0 was FAILURE

2024-02-13 [email protected] Do not use AChoreographer on 32 bit devices (flutter/engine#50586)
2024-02-13 [email protected] Move libpng to //flutter/third_party/libpng (flutter/engine#50571)
2024-02-13 [email protected] Roll Skia from b150107d536d to aaed347b2dbb (2 revisions) (flutter/engine#50595)
2024-02-13 [email protected] Roll Dart SDK from 8d3a8d85c883 to 105312b80125 (2 revisions) (flutter/engine#50593)
2024-02-13 [email protected] Roll Skia from d503bc9c6e46 to b150107d536d (1 revision) (flutter/engine#50591)
2024-02-13 [email protected] Roll Skia from a1d9b83034ac to d503bc9c6e46 (3 revisions) (flutter/engine#50587)
2024-02-13 [email protected] [Impeller] cleaned up and removed golden test exceptions (flutter/engine#50572)
2024-02-13 [email protected] Roll Skia from 17d00f9241b3 to a1d9b83034ac (2 revisions) (flutter/engine#50582)
2024-02-13 [email protected] Roll Fuchsia Linux SDK from RVHAIKU50ogkiV-Lj... to l6mWjvlO1xJg5ZFKK... (flutter/engine#50579)
2024-02-13 [email protected] Roll Dart SDK from 94f7dec34cef to 8d3a8d85c883 (1 revision) (flutter/engine#50577)
2024-02-13 [email protected] Add Impeller runtime options to Android `scenario_app` (flutter/engine#50487)
2024-02-13 [email protected] Roll Skia from c464143dfaab to 17d00f9241b3 (1 revision) (flutter/engine#50576)
2024-02-13 [email protected] Roll Skia from ecf2f622d0b3 to c464143dfaab (2 revisions) (flutter/engine#50574)
2024-02-12 [email protected] [Impeller] CommandPoolVK recycles command buffers too. (flutter/engine#50468)
2024-02-12 [email protected] Make `dart:ui#Paint` `final` to allow changes/disallow implementing. (flutter/engine#50557)
2024-02-12 [email protected] Replace usage of an integer for GdkModifierType (flutter/engine#50481)
2024-02-12 [email protected] Remove FlKeyEvent.dispose_origin and use GdkEvent type for origin (flutter/engine#50483)
2024-02-12 [email protected] Move Linux impeller_unittests to linux_unopt (flutter/engine#50558)
2024-02-12 [email protected] [Impeller] Don't suppress outdated validation error. (flutter/engine#50568)
2024-02-12 [email protected] [Impeller] Remove references to moved impeller/image from README. (flutter/engine#50513)
2024-02-12 [email protected] Roll Dart SDK from dca13675e65e to 94f7dec34cef (1 revision) (flutter/engine#50567)
2024-02-12 [email protected] Roll Skia from f88eeb658358 to ecf2f622d0b3 (1 revision) (flutter/engine#50566)
2024-02-12 [email protected] Roll Skia from 867c1e8390e2 to f88eeb658358 (5 revisions) (flutter/engine#50564)
2024-02-12 [email protected] Roll vulkan-deps to 014f44e134a1de387791bffacc32ff9d8db71176 (flutter/engine#50515)
2024-02-12 [email protected] Roll Skia from 4081496b7693 to 867c1e8390e2 (20 revisions) (flutter/engine#50562)
2024-02-12 [email protected] [Impeller] force render pass construction on all backends to ensure image layout is transitioned/render pass state setup. (flutter/engine#50539)
2024-02-12 [email protected] [Windows] Make the view own its EGL surface (flutter/engine#50421)
2024-02-12 [email protected] Roll Dart SDK from 5c9e79e66364 to dca13675e65e (1 revision) (flutter/engine#50559)
2024-02-12 [email protected] [fuchsia] Update Inspect library usage (flutter/engine#50467)
2024-02-12 [email protected] Initializes RBE in the build config runner (flutter/engine#50543)
2024-02-12 [email protected] Rename [ViewFocusDirection.backwards] to [ViewFocusDirection.backward] (flutter/engine#50533)
2024-02-12 [email protected] Refactor NDK helpers some more, add methods for SurfaceControl/Transaction, tests (flutter/engine#50540)
2024-02-12 [email protected] [Impeller] deleted the old blur (flutter/engine#50470)
2024-02-12 [email protected] Update codec defines for Skia (flutter/engine#50554)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from RVHAIKU50ogk to l6mWjvlO1xJg

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],[email protected] on the revert to ensure that a human
is aware of the problem.

...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants