Revert "[CP-beta]Revert "Resolve resolve native Flutter dependencies in Android Studio (#167332)"" #177601
Conversation
…in Andro…" This reverts commit 6c0f1cf.
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
There was a problem hiding this comment.
Code Review
This pull request reverts a previous revert, re-introducing the logic to resolve native Flutter dependencies in Android Studio. This is achieved by adding compileOnly dependencies for debug builds when the Gradle task is invoked from the IDE. The changes look reasonable, but I have a couple of suggestions for improvement regarding maintainability and error handling.
| } catch (e: TaskInstantiationException) { | ||
| // ignored | ||
| } |
There was a problem hiding this comment.
Silently ignoring exceptions is generally not a good practice. If this TaskInstantiationException is caught, it would be beneficial to log it to inform developers about what happened. This would be helpful for debugging purposes, for instance, if the task registration is skipped when it wasn't intended to be.
Consider adding a log message, for example, at the info level.
| } catch (e: TaskInstantiationException) { | |
| // ignored | |
| } | |
| } catch (e: TaskInstantiationException) { | |
| rootProject.logger.info("The 'generateLockfiles' task is already registered. Skipping.", e) | |
| } |
| dependencies.add("compileOnly", "io.flutter:flutter_embedding_debug:$engineVersion") | ||
| dependencies.add("compileOnly", "io.flutter:armeabi_v7a_debug:$engineVersion") | ||
| dependencies.add("compileOnly", "io.flutter:arm64_v8a_debug:$engineVersion") | ||
| dependencies.add("compileOnly", "io.flutter:x86_64_debug:$engineVersion") |
There was a problem hiding this comment.
To improve maintainability and avoid hardcoding the list of architectures, you can iterate over FlutterPluginConstants.DEFAULT_PLATFORMS to add these dependencies. This would be more consistent with how other parts of the codebase handle platform-specific dependencies and would make it easier to add or remove architectures in the future.
dependencies.add("compileOnly", "io.flutter:flutter_embedding_debug:$engineVersion")
FlutterPluginConstants.DEFAULT_PLATFORMS.forEach { platform ->
val arch = FlutterPluginConstants.PLATFORM_ARCH_MAP[platform]?.replace("-", "_")
dependencies.add("compileOnly", "io.flutter:${arch}_debug:$engineVersion")
}
jtmcdole
left a comment
There was a problem hiding this comment.
I approve the revert of the revert. Long live the first one.
Reverts #177255. This pull request was to the wrong branch. Should have been flutter-3.38-candidate.0.