-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[Tool] New tool to download android dependencies #4408
[Tool] New tool to download android dependencies #4408
Conversation
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 on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! If there's a way to share the cache for the whole test run rather than per-plugin that would be even better (or is Gradle actually doing that under the hood?), but either way this is a clear improvement, and should really help us quickly diagnose network and server flake.
So our testing system is complex but gradle lays out the files it relies on here by default all tests on a machine will share the gradle cache unless we configure it differently. Searching I was not able to find a server lifecycle doc to see how often those machines are wiped [1]. Searching for where we might configure gradle home I found a bunch of places where we set GRADLE_USER_HOME and all the the values we set are the same [1] Moma search, exploring https://flutter.googlesource.com/infra/ |
Do you have a build example which downloads gradle dependencies that I can take a look? |
import 'common/plugin_utils.dart'; | ||
import 'common/repository_package.dart'; | ||
|
||
/// Run 'gradlew dependencies' on android. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update this comment now that this command is more generic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I did. If you notice the line below you can see that I document the other things this command could do but this comment is correct with what it does now.
My expectation is that if/when we add dart support the tool comment will be updated to say
/// Run 'gradlew dependencies' on android.
/// Run 'pub get' on dart packages.
/// Run 'pod install' and 'swift package download --all' depending on the configuration.
For now I updated the top line to have a summary then the specifics. Hopefully that addresses your concern.
…tep improve documentaion in fetch-deps command
This is the only pr that should have the new step to download dependencies. That said all existing android test would have gradle dependencies because I think we include kotlin by default. Here is a recent build https://ci.chromium.org/ui/p/flutter/builders/try/Linux_android%20android_build_all_packages%20master/505/overview I am not sure if I understand your comment about the existing infra config all I have found is in https://cs.opensource.google/search?q=GRADLE_USER_HOME%20-f:gradle-wrapper.properties&start=1. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
android_platform_tests.yaml
LGTM
flutter/packages@aa1eace...369ee7e 2023-07-14 [email protected] [Tool] New tool to download android dependencies (flutter/packages#4408) 2023-07-14 [email protected] [video_player] added iOS exception on incorrect asset path (flutter/packages#4318) 2023-07-14 [email protected] [google_maps_flutter_platform_interface] Add support for cloud-based map styling (flutter/packages#4141) 2023-07-14 [email protected] [webview_flutter] Adds support for receiving a url with WebResourceError (flutter/packages#3884) 2023-07-14 [email protected] [xdg_directories] Remove `process` dependency (flutter/packages#4460) 2023-07-13 [email protected] [various] Update Pigeon in Swift plugins (flutter/packages#4461) 2023-07-13 [email protected] Roll Flutter from 544d30d to c40173f (10 revisions) (flutter/packages#4457) 2023-07-13 [email protected] Roll Flutter (stable) from 796c8ef to f468f33 (2 revisions) (flutter/packages#4456) 2023-07-13 [email protected] Fix Router Config Issues #4300 (go_router_builder/example) (flutter/packages#4369) 2023-07-13 [email protected] [webview_flutter_platform_interface] Adds url to `WebResourceError` (flutter/packages#4439) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [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
flutter/packages@aa1eace...369ee7e 2023-07-14 [email protected] [Tool] New tool to download android dependencies (flutter/packages#4408) 2023-07-14 [email protected] [video_player] added iOS exception on incorrect asset path (flutter/packages#4318) 2023-07-14 [email protected] [google_maps_flutter_platform_interface] Add support for cloud-based map styling (flutter/packages#4141) 2023-07-14 [email protected] [webview_flutter] Adds support for receiving a url with WebResourceError (flutter/packages#3884) 2023-07-14 [email protected] [xdg_directories] Remove `process` dependency (flutter/packages#4460) 2023-07-13 [email protected] [various] Update Pigeon in Swift plugins (flutter/packages#4461) 2023-07-13 [email protected] Roll Flutter from 544d30d to c40173f (10 revisions) (flutter/packages#4457) 2023-07-13 [email protected] Roll Flutter (stable) from 796c8ef to f468f33 (2 revisions) (flutter/packages#4456) 2023-07-13 [email protected] Fix Router Config Issues flutter#4300 (go_router_builder/example) (flutter/packages#4369) 2023-07-13 [email protected] [webview_flutter_platform_interface] Adds url to `WebResourceError` (flutter/packages#4439) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [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
This pr is pushed for high level feedback/conversation. I will add tests before serious review.
should be read in conjuction with https://flutter-review.googlesource.com/c/recipes/+/46980
flutter/flutter#120119
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].CHANGELOG.md
to add a description of the change, [following repository CHANGELOG style].///
).