-
Notifications
You must be signed in to change notification settings - Fork 28.9k
Adds vmservices to retrieve android applink settings #125998
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
Conversation
8c30d81
to
bd14573
Compare
@@ -904,7 +976,11 @@ class FlutterPlugin implements Plugin<Project> { | |||
validateDeferredComponentsValue = project.property('validate-deferred-components').toBoolean() | |||
} | |||
addTaskForJavaVersion(project) | |||
addTaskForPrintBuildVariants(project) | |||
if(isFlutterAppProject()) { |
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 need this check to avoid crash when running gradle in flutter modules that are use in add to app scenario
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 this is because you are depending on steps that only exist for apps. But I would expect add to app to work and potentially problems with libraries.
Does this need to be cherry picked into either beta or stable?
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.
for add to app, these tasks can only be added in the main android project.
I think in the future I should refactor this out into a separate gradle plugin to support add-to-app, but I would like to make the decision later when we have more concrete design for add-to-app support in deeplink validation tool.
This doesn't need to be cherry pick as there isn't a client using these tasks yet.
Hi, @chunhtai is this still a work in progress? Seems like you have some tests failing |
ecb584f
to
27f5f46
Compare
These tests failures seem to be related to #126557 |
e16b622
to
fff1c6c
Compare
@@ -904,7 +976,11 @@ class FlutterPlugin implements Plugin<Project> { | |||
validateDeferredComponentsValue = project.property('validate-deferred-components').toBoolean() | |||
} | |||
addTaskForJavaVersion(project) | |||
addTaskForPrintBuildVariants(project) | |||
if(isFlutterAppProject()) { |
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 this is because you are depending on steps that only exist for apps. But I would expect add to app to work and potentially problems with libraries.
Does this need to be cherry picked into either beta or stable?
fff1c6c
to
19feb3e
Compare
private static void addTasksForPrintApplicationId(Project project) { | ||
project.android.applicationVariants.all { variant -> | ||
// Warning: The name of this task is used by `AndroidProject.getApplicationIdForVariant`. | ||
project.tasks.register("print${variant.name.capitalize()}ApplicationId") { |
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 according to https://developer.android.com/reference/tools/gradle-api/7.4/com/android/build/api/variant/ApplicationVariant#applicationId() this depends on some task running that will merge the manifests. That means this task depends on another task (possibly as simple as assembleVariant). Given this I think you should add a dependsOn block as defined here. https://docs.gradle.org/current/userguide/tutorial_using_tasks.html#sec:task_dependencies
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 you might actually want variant.outputs.each
see example here https://developer.android.com/build/gradle-tips#configure-dynamic-version-codes to print the applicationId.
But I am not sure how task registration interacts differently if you use outputs.
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.
FWIW I am trying to give good guidance here if you explore these things and they dont work you can ignore my comments.
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.
It looks like it is not depending on any task, I ran it after a clean, and it still works. Also the application id is per applicationvariant I think? i didn't see a property on variantoutput for applicationID.
19feb3e
to
dd4c8a7
Compare
flutter/flutter@8a5c22e...6e254a3 2023-06-08 chillers@google.com [labeler] Set sync labels to false to stop removing labels (flutter/flutter#128446) 2023-06-08 jacksongardner@google.com Update Chrome version for testing (flutter/flutter#128447) 2023-06-08 zanderso@users.noreply.github.com Revert "Redo make inspector weakly referencing the inspected objects." (flutter/flutter#128506) 2023-06-08 sstrickl@google.com Use `--target-os` for appropriate precompiled targets. (flutter/flutter#127567) 2023-06-08 polinach@google.com Redo make inspector weakly referencing the inspected objects. (flutter/flutter#128471) 2023-06-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1089ce6874cf to a5f7d5d75ff2 (11 revisions) (flutter/flutter#128473) 2023-06-07 gspencergoog@users.noreply.github.com Disable context menu (flutter/flutter#128365) 2023-06-07 47866232+chunhtai@users.noreply.github.com Adds vmservices to retrieve android applink settings (flutter/flutter#125998) 2023-06-07 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4f4486b00be2 to 1089ce6874cf (20 revisions) (flutter/flutter#128460) 2023-06-07 leroux_bruno@yahoo.fr Fix typos 'wether' -> 'whether' (flutter/flutter#128392) 2023-06-07 aam@google.com Roll engine, patch expression evaluation (flutter/flutter#128255) 2023-06-07 engine-flutter-autoroll@skia.org Roll Packages from da72219 to a84b2c2 (1 revision) (flutter/flutter#128444) 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 Please CC rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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
fixes #120408
Added two gradle tasks, one for grabing the application id, one for grabbing app link domains.
Added a new vmservices to call these two gradle tasks and return the result.
The expected work flow is that the devtool will first call a vmservices to grab all avaliable build variants. It will then choose one of the build variant and call this new services to get application id and app link domains.
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.