这是indexloc提供的服务,不要输入任何密码
Skip to content

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

Merged
merged 2 commits into from
Jun 7, 2023

Conversation

chunhtai
Copy link
Contributor

@chunhtai chunhtai commented May 3, 2023

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

  • 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, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label May 3, 2023
@chunhtai chunhtai force-pushed the issues/120408-applink branch 5 times, most recently from 8c30d81 to bd14573 Compare May 9, 2023 18:14
@chunhtai chunhtai marked this pull request as ready for review May 9, 2023 18:22
@@ -904,7 +976,11 @@ class FlutterPlugin implements Plugin<Project> {
validateDeferredComponentsValue = project.property('validate-deferred-components').toBoolean()
}
addTaskForJavaVersion(project)
addTaskForPrintBuildVariants(project)
if(isFlutterAppProject()) {
Copy link
Contributor Author

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

Copy link
Contributor

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?

Copy link
Contributor Author

@chunhtai chunhtai May 23, 2023

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.

@eliasyishak
Copy link
Contributor

Hi, @chunhtai is this still a work in progress? Seems like you have some tests failing

@chunhtai chunhtai force-pushed the issues/120408-applink branch 2 times, most recently from ecb584f to 27f5f46 Compare May 11, 2023 20:36
@chunhtai
Copy link
Contributor Author

These tests failures seem to be related to #126557

@chunhtai chunhtai force-pushed the issues/120408-applink branch from e16b622 to fff1c6c Compare May 16, 2023 17:27
@reidbaker reidbaker requested a review from camsim99 May 16, 2023 20:20
@@ -904,7 +976,11 @@ class FlutterPlugin implements Plugin<Project> {
validateDeferredComponentsValue = project.property('validate-deferred-components').toBoolean()
}
addTaskForJavaVersion(project)
addTaskForPrintBuildVariants(project)
if(isFlutterAppProject()) {
Copy link
Contributor

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?

@chunhtai chunhtai force-pushed the issues/120408-applink branch from fff1c6c to 19feb3e Compare May 23, 2023 23:06
@chunhtai chunhtai requested a review from reidbaker May 25, 2023 16:21
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") {
Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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.

@chunhtai chunhtai force-pushed the issues/120408-applink branch from 19feb3e to dd4c8a7 Compare June 6, 2023 16:43
@chunhtai chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 6, 2023
@auto-submit auto-submit bot merged commit 5328bd9 into flutter:master Jun 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 8, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 8, 2023
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jun 8, 2023
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
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Deeplink Automation] Implements flutter tool to retrieve app settings from Android project
3 participants