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 //third_party/glfw to //flutter/third_party/glfw #46733

Merged
merged 4 commits into from
Oct 11, 2023

Conversation

cbracken
Copy link
Member

@cbracken cbracken commented Oct 10, 2023

As part of eliminating the Flutter buildroot (#67373), we are moving all third-party dependencies from //third_party to //flutter/third_party.

This is the engine-side follow-up to flutter/buildroot#777, flutter/buildroot#778.

Once all third-party dependencies have been migrated, tooling and config will be moved and the buildroot will be eliminated altogether.

Issue: flutter/flutter#136284

No tests changed because there is no semantic change to this PR. This is simply relocating a dependency.

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.

@cbracken
Copy link
Member Author

The initial commit will fail the licence script; once we get the failures from CI, I'll amend the patch.

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.

FREAKIN LOVE IT

@cbracken cbracken force-pushed the move-glfw branch 2 times, most recently from ad193e5 to 2bac18d Compare October 10, 2023 20:20
Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

lgtm w/ question

.gitignore Outdated Show resolved Hide resolved
@matanlurey
Copy link
Contributor

Latest build error:

error searching for copyright in: ../../../flutter/third_party/glfw/deps/getopt.h
failed to find any license but saw unmatched potential copyright and license statements; first twenty lines

@cbracken see https://raw.githubusercontent.com/flutter/engine/main/ci/licenses_golden/licenses_third_party, but I think we'll need to move these licenses to https://github.com/flutter/engine/blob/main/ci/licenses_golden/licenses_flutter (the names are a bit of a misnomer).

As part of eliminating the Flutter buildroot (#67373), we are moving all
third-party dependencies from //third_party to //flutter/third_party.

This is the engine-side follow-up to flutter/buildroot#777.

Once all third-party dependencies have been migrated, tooling and config
will be moved and the buildroot will be eliminated altogether.

Issue: flutter/flutter#136284
@zanderso
Copy link
Member

It might not fix everything the license script is sad about, but I would suggest starting out by updating the paths here:

https://github.com/flutter/engine/blob/main/tools/licenses/lib/paths.dart#L101

and seeing what the remaining issues are.

What we want is *likely* to break apart licence checking for third-party
deps that are checked into the repo and those that are pulled via DEPS
since they have significantly different characteristics:
* Checked-in third-party code is unlikely to change much in terms of
  license-related content
* Code from deps is outside of our (direct) control and may change more
  frequently, and should likely be treated like the src/third_party deps
  are.

We should probably consider an update to the licence script that treats
these two separately (e.g. via an allowlist).
@cbracken cbracken merged commit e41c646 into flutter:main Oct 11, 2023
26 checks passed
@cbracken cbracken deleted the move-glfw branch October 11, 2023 01:21
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 11, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 11, 2023
…136307)

flutter/engine@ea275c3...8086814

2023-10-11 matanlurey@users.noreply.github.com Remove support for `Paint.enableDithering=false` in `dart:ui`. (flutter/engine#46745)
2023-10-11 chris@bracken.jp Move //third_party/glfw to //flutter/third_party/glfw (flutter/engine#46733)
2023-10-11 skia-flutter-autoroll@skia.org Roll Skia from 06145491fd17 to 4935bed4260d (3 revisions) (flutter/engine#46748)
2023-10-11 bdero@google.com [Impeller] Guard calls to extension proc DebugMessageControlKHR. (flutter/engine#46747)

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 jsimmons@google.com,rmistry@google.com,zra@google.com 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://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
@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 Hixie or stuartmorgan on 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.

harryterkelsen pushed a commit that referenced this pull request Oct 23, 2023
As part of eliminating the Flutter buildroot (#67373), we are moving all
third-party dependencies from //third_party to //flutter/third_party.

This is the engine-side follow-up to flutter/buildroot#777.

Once all third-party dependencies have been migrated, tooling and config
will be moved and the buildroot will be eliminated altogether.

Issue: flutter/flutter#136284

No tests changed because there is no semantic change to this PR. This is
simply relocating a dependency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Archived in project
3 participants