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

Automatically delete unused dart deps from flutter/DEPS file #245

Merged
aam merged 2 commits into from
May 1, 2019
Merged

Automatically delete unused dart deps from flutter/DEPS file #245

aam merged 2 commits into from
May 1, 2019

Conversation

@aam
Copy link
Member

@aam aam commented Apr 30, 2019

This requires introducing markers to flutter DEPS file as follows:

diff --git a/DEPS b/DEPS
index d17dfbc35..ff704a697 100644
--- a/DEPS
+++ b/DEPS
@@ -167,15 +167,17 @@ deps = {
   'src/third_party/icu':
    Var('chromium_git') + '/chromium/deps/icu.git' + '@' + 'c56c671998902fcc4fc9ace88c83daa99f980793',
 
-  'src/third_party/dart':
-   Var('dart_git') + '/sdk.git' + '@' + Var('dart_revision'),
-
   'src/third_party/boringssl':
    Var('github_git') + '/dart-lang/boringssl_gen.git' + '@' + Var('dart_boringssl_gen_rev'),
 
   'src/third_party/boringssl/src':
    'https://boringssl.googlesource.com/boringssl.git' + '@' + Var('dart_boringssl_rev'),
 
+  'src/third_party/dart':
+   Var('dart_git') + '/sdk.git' + '@' + Var('dart_revision'),
+
+  # WARNING: Unused Dart dependencies in the list below till "WARNING:" marker are removed automatically - see create_updated_flutter_deps.py.
+
   'src/third_party/dart/third_party/observatory_pub_packages':
    Var('dart_git') + '/observatory_pub_packages.git' + '@' + Var('dart_observatory_pub_packages_rev'),
 
@@ -356,6 +358,8 @@ deps = {
   'src/third_party/dart/third_party/pkg/yaml':
    Var('dart_git') + '/yaml.git' + '@' + Var('dart_yaml_tag'),
 
+  # WARNING: end of dart dependencies list that is cleaned up automatically - see create_updated_flutter_deps.py.
+
   'src/third_party/colorama/src':
    Var('chromium_git') + '/external/colorama.git' + '@' + '799604a1041e9b3bc5d2789ecbd7e8db2e18e6b8',

@aam
Copy link
Member Author

aam commented Apr 30, 2019

cc @bkonyi

bkonyi
bkonyi reviewed Apr 30, 2019
View reviewed changes
tools/dart/create_updated_flutter_deps.py Outdated
for (dart_k, dart_v) in (new_deps.iteritems()):
dart_k_suffix = dart_k[len('sdk/') if dart_k.startswith('sdk/') else 0:]
if (k.endswith(dart_k_suffix)):
updated_value = dart_v
Copy link
Contributor

@bkonyi bkonyi Apr 30, 2019

Choose a reason for hiding this comment

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

Does this do anything? We overwrite updated_value in the next line.

Copy link
Member Author

@aam aam Apr 30, 2019

Choose a reason for hiding this comment

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

Ah, right!

@bkonyi bkonyi self-requested a review April 30, 2019 23:19
bkonyi
bkonyi approved these changes Apr 30, 2019
View reviewed changes
Copy link
Contributor

@bkonyi bkonyi left a comment

Choose a reason for hiding this comment

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

Overall, LGTM.

@aam
Copy link
Member Author

aam commented Apr 30, 2019

Okay, so then we need to land dart sdk with updated quiver var name, then update flutter/DEPS so it has markers and run the script once so it just reformats everything and stays unchanged in next runs(unless dependencies do change), then land this CL with the script into buildroot.

@bkonyi
Copy link
Contributor

bkonyi commented May 1, 2019

Okay, so then we need to land dart sdk with updated quiver var name, then update flutter/DEPS so it has markers and run the script once so it just reformats everything and stays unchanged in next runs(unless dependencies do change), then land this CL with the script into buildroot.

I'll let you know when the next SDK roll makes its way in so we can do the necessary updates. Should hopefully be in tonight.

@bkonyi
Copy link
Contributor

bkonyi commented May 1, 2019

The latest SDK roll has landed with the updated Quiver var name: flutter/engine#8796 Feel free to make the necessary changes to the DEPS file now.

@aam aam mentioned this pull request May 1, 2019
Merged
@aam
Copy link
Member Author

aam commented May 1, 2019

Feel free to make the necessary changes to the DEPS file now.

flutter/engine#8800 with the changes

@bkonyi
Copy link
Contributor

bkonyi commented May 1, 2019

We should also roll the buildroot version forward once this change lands.

@aam
Copy link
Member Author

aam commented May 1, 2019

We should also roll the buildroot version forward once this change lands.

Right!

@aam aam merged commit 35b76cf into flutter:master May 1, 2019
@aam aam deleted the maintain-dart-deps branch May 1, 2019 18:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@bkonyi bkonyi bkonyi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aam @bkonyi