Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[various] Adds Android namespace #3791

Merged
merged 10 commits into from
Apr 25, 2023
Merged

[various] Adds Android namespace #3791

merged 10 commits into from
Apr 25, 2023

Conversation

stuartmorgan
Copy link
Contributor

Adds a namespace attribute to the Android build.gradle, for compatibility with Android Gradle Plugin 8.0, and adds tooling to enforce that it's there.

This is necessary for plugins now; for examples this isn't needed yet, but since it will be needed to eventually update the apps to AGP 8.0 anyway, it's easiest to just enforce it everywhere now.

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 relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • 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.

reidbaker
reidbaker reviewed Apr 24, 2023
View reviewed changes
script/tool/lib/src/gradle_check_command.dart Outdated
Comment on lines 50 to 52
final Directory parentDir =
isExample ? androidDir.childDirectory('app') : androidDir;
final File gradleFile = parentDir.childFile('build.gradle');
Copy link
Contributor

Choose a reason for hiding this comment

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

Nonblocking: Can you extract this into a method that gives you the gradle file? parentDir makes sense as a concept when getting a build.gradle file but I think will make this code harder to reason about when we end up doing validations on multiple build.gradle files in a single example app.

It is way more refactoring than I would want to ask from you but I think the right structure of this validation in the long term will be a top line _validateBuildGradle method that passe the contents of build.gradle files into 3 different validators depending on what the project has. One for plugins, one for the root of an application and one for android/app/build.gradle. Then if we need to validate something specific like namespace there is a method for that called in every sub validator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nonblocking: Can you extract this into a method that gives you the gradle file?

Good idea, done.

It is way more refactoring than I would want to ask from you but I think the right structure of this validation [...]

https://xkcd.com/356/ 😁

script/tool/lib/src/gradle_check_command.dart Outdated
namespace 'dev.flutter.foo'
}

The value must match the "package" attribute in AndroidManifest.xml.
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 line is only true while we have both and this error will not make sense if the AndroidManifest does not have a package value which should be the case in the future. For futher guidance maybe we link users to https://developer.android.com/build/publish-library/prep-lib-release#choose-namespace?

FWIW the AGP updater removes the package line from all the manifests that are used.

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 added a "if one is present" caveat, like I had below.

FWIW the AGP updater removes the package line from all the manifests that are used.

That makes sense for apps; we definitely won't want that for plugins since it will presumably break people on old versions of AGP.

Let me know if you also want me to make this match check non-example only.

Copy link
Contributor

Choose a reason for hiding this comment

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

If one is present I think is the correct change here.

script/tool/test/gradle_check_command_test.dart Outdated
@@ -216,4 +254,119 @@ dependencies {
]),
);
});

test('fails when namespace does not match AndroidManifest.xml', () async {
Copy link
Contributor

Choose a reason for hiding this comment

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

see comment above, also I think part of the value android is trying to put forward is to separate the package from the namespace so that tests can have a custom namespace.

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've renamed this to say that it's only testing for the library, and put a TODO on the example app version of the test saying we may want to remove it in the future. (Or I can remove the example test entirely, per my comment above.)

stuartmorgan
stuartmorgan commented Apr 25, 2023
View reviewed changes
script/tool/lib/src/gradle_check_command.dart
RepositoryPackage package, File gradleFile) {
print('${indentation}Validating '
'${getRelativePosixPath(gradleFile, from: package.directory)}.');
// TODO(stuartmorgan): Move the -Xlint validation from lint_android_command
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 have a follow-up PR to address this TODO once this one lands; I started to do it as part of the refactoring, but it added a lot of changes that weren't related to the purpose of this PR so I split it out.

stuartmorgan and others added 2 commits April 25, 2023 10:17
@stuartmorgan @reidbaker
Typo fix
c9a63aa 
Co-authored-by: Reid Baker <hamilton.reid.baker@gmail.com>
@stuartmorgan
Merge branch 'main' into android-gradle-namespace
54c1b86
@stuartmorgan stuartmorgan added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 25, 2023
@auto-submit auto-submit bot merged commit 6284c2d into flutter:main Apr 25, 2023
@stuartmorgan stuartmorgan deleted the android-gradle-namespace branch April 25, 2023 15:44
pro100andrey added a commit to pro100andrey/packages that referenced this pull request Apr 25, 2023
@pro100andrey
Merge branch 'main' into feature/fix
1263b0f 
* main: (144 commits)
  Update test golden images for the latest Skia roll (flutter#3787)
  [various] Adds Android namespace (flutter#3791)
  [shared_preferences] Update gradle/agp in example apps (flutter#3809)
  [go_router] Adds name to TypedGoRoute (flutter#3702)
  [webview_flutter] Adds support to receive permission requests (flutter#3543)
  [google_sign_in] Fix Android Java warnings (flutter#3762)
  [local_auth] Fix enum return on Android (flutter#3780)
  [pigeon] Warn when trying to use enums in collections (flutter#3782)
  [webview_flutter_android] [webview_flutter_wkwebview] Platform implementations for supporting permission requests (flutter#3792)
  [pigeon] Update for compatibility with a future change to the analyzer. (flutter#3789)
  [camera_android] Fix Android lint warnings  (flutter#3716)
  [webview_flutter_platform_interface] Adds method to receive permission requests (flutter#3767)
  [image_picker_ios] In unit test write and read kCGImagePropertyExifUserComment property (flutter#3783)
  [go_router_builder] Fixed the return value of the generated push method (flutter#3650)
  [image_picker] Mention `launchMode: singleInstance` in README (flutter#3759)
  Revert "[pigeon] Add an initial example app" (flutter#3785)
  [google_maps_flutter] Add examples for different iOS versions (flutter#3757)
  [pigeon] Add an initial example app (flutter#3761)
  [google_maps_flutter_web] Allow marker position updates (flutter#3697)
  [Tool] [Code Excerpt] allow excerpts in example readme (flutter#3758)
  ...

# Conflicts:
#	packages/camera/camera/pubspec.yaml
@reidbaker reidbaker mentioned this pull request Apr 25, 2023
Merged
11 tasks
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 1, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 1, 2023
@engine-flutter-autoroll
6284c2d [various] Adds Android namespace (flutter/packages#3791)
0103735
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 1, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 1, 2023
@engine-flutter-autoroll
6284c2d [various] Adds Android namespace (flutter/packages#3791)
d3a511d
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 1, 2023
@engine-flutter-autoroll
Roll Packages from 7e3f5da to de6131d (41 revisions) (#125811)
ff248d2 
Roll Packages from 7e3f5da to de6131d (41 revisions)

flutter/packages@7e3f5da...de6131d

2023-05-01 reidbaker@google.com I122213 update non examples (flutter/packages#3846)
2023-04-29 47866232+chunhtai@users.noreply.github.com [go_router] Cleans up route match API and introduces dart fix (flutter/packages#3819)
2023-04-29 43054281+camsim99@users.noreply.github.com [camerax] Add `LifecycleOwner` Proxy (flutter/packages#3837)
2023-04-29 stuartmorgan@google.com [file_selector] Add getDirectoryPaths implementation for Windows (flutter/packages#3704)
2023-04-29 stuartmorgan@google.com [various] Update Android example min SDKs (flutter/packages#3847)
2023-04-28 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.2.12 to 2.3.2 (flutter/packages#3838)
2023-04-28 43054281+camsim99@users.noreply.github.com [camerax] Implement Image Streaming (flutter/packages#3454)
2023-04-28 reidbaker@google.com [various] update agp and gradle for all examples in packages (flutter/packages#3822)
2023-04-28 54558023+keyonghan@users.noreply.github.com Update xcode to 14c18 (flutter/packages#3774)
2023-04-28 andrewjohncoutts@gmail.com [camera_android] Add NV21 as an image stream format #3277 (flutter/packages#3639)
2023-04-28 gautier.bayzelon@gmail.com [go_router] Remove unused navigator keys (flutter/packages#3708)
2023-04-28 JeroenWeener@users.noreply.github.com [image_picker] Move I/O operations to a separate thread (flutter/packages#3506)
2023-04-28 49699333+dependabot[bot]@users.noreply.github.com [pigeon]: Bump org.jetbrains.kotlin:kotlin-gradle-plugin from 1.8.20 to 1.8.21 in /packages/pigeon/platform_tests/test_plugin/android (flutter/packages#3824)
2023-04-28 stuartmorgan@google.com [pigeon] Reland: Add an initial example app (flutter/packages#3832)
2023-04-28 engine-flutter-autoroll@skia.org Roll Flutter from c9004ff to 66fa4c5 (68 revisions) (flutter/packages#3830)
2023-04-28 stuartmorgan@google.com [various] Conditionalize the namespace in all Android plugins (flutter/packages#3836)
2023-04-27 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump com.android.billingclient:billing from 5.1.0 to 5.2.0 in /packages/in_app_purchase/in_app_purchase_android/android (flutter/packages#3672)
2023-04-27 ychris@google.com [auick_action_ios] Retries multiple times to not fail ci when there is a flake (flutter/packages#3823)
2023-04-26 magder@google.com Swap some iOS package CODEOWNERS (flutter/packages#3793)
2023-04-26 stuartmorgan@google.com [various] Add `targetCompatibility` to build.gradle (flutter/packages#3825)
2023-04-26 stuartmorgan@google.com [various] Set cmake_policy versions (flutter/packages#3828)
2023-04-26 stuartmorgan@google.com [path_provider] Allow `win32` up to version 4.x (flutter/packages#3820)
2023-04-26 stuartmorgan@google.com [tool] Move Android lint checks (flutter/packages#3816)
2023-04-25 evace93@gmail.com [google_maps_flutter_android] Fix Android lint warnings (flutter/packages#3751)
2023-04-25 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump androidx.annotation:annotation from 1.5.0 to 1.6.0 in /packages/in_app_purchase/in_app_purchase_android/android (flutter/packages#3381)
2023-04-25 jason-simmons@users.noreply.github.com Update test golden images for the latest Skia roll (flutter/packages#3787)
2023-04-25 stuartmorgan@google.com [various] Adds Android namespace (flutter/packages#3791)
2023-04-25 reidbaker@google.com [shared_preferences] Update gradle/agp in example apps (flutter/packages#3809)
2023-04-24 43640732+dancamdev@users.noreply.github.com [go_router] Adds name to TypedGoRoute (flutter/packages#3702)
2023-04-22 10687576+bparrishMines@users.noreply.github.com [webview_flutter] Adds support to receive permission requests (flutter/packages#3543)
2023-04-21 stuartmorgan@google.com [google_sign_in] Fix Android Java warnings (flutter/packages#3762)
2023-04-21 stuartmorgan@google.com [local_auth] Fix enum return on Android (flutter/packages#3780)
2023-04-21 stuartmorgan@google.com [pigeon] Warn when trying to use enums in collections (flutter/packages#3782)
2023-04-21 10687576+bparrishMines@users.noreply.github.com [webview_flutter_android] [webview_flutter_wkwebview] Platform implementations for supporting permission requests (flutter/packages#3792)
2023-04-21 scheglov@google.com [pigeon] Update for compatibility with a future change to the analyzer. (flutter/packages#3789)
2023-04-21 10687576+bparrishMines@users.noreply.github.com [camera_android] Fix Android lint warnings  (flutter/packages#3716)
2023-04-21 10687576+bparrishMines@users.noreply.github.com [webview_flutter_platform_interface] Adds method to receive permission requests (flutter/packages#3767)
2023-04-21 magder@google.com [image_picker_ios] In unit test write and read kCGImagePropertyExifUserComment property (flutter/packages#3783)
2023-04-21 widrans@gmail.com [go_router_builder] Fixed the return value of the generated push method (flutter/packages#3650)
2023-04-21 JeroenWeener@users.noreply.github.com [image_picker] Mention `launchMode: singleInstance` in README (flutter/packages#3759)
2023-04-21 stuartmorgan@google.com Revert "[pigeon] Add an initial example app" (flutter/packages#3785)

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 flutter-ecosystem@google.com,rmistry@google.com on the revert to ensure that a human
...
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
@stuartmorgan @nploi
[various] Adds Android namespace (flutter#3791)
68f5426 
Adds a `namespace` attribute to the Android build.gradle, for compatibility with Android Gradle Plugin 8.0, and adds tooling to enforce that it's there.

This is necessary for plugins now; for examples this isn't needed yet, but since it will be needed to eventually update the apps to AGP 8.0 anyway, it's easiest to just enforce it everywhere now.
@davidhsoftware davidhsoftware mentioned this pull request Dec 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reidbaker reidbaker reidbaker approved these changes

@camsim99 camsim99 Awaiting requested review from camsim99 camsim99 is a code owner

@gmackall gmackall Awaiting requested review from gmackall gmackall is a code owner

@bparrishMines bparrishMines Awaiting requested review from bparrishMines bparrishMines is a code owner

@tarrinneal tarrinneal Awaiting requested review from tarrinneal tarrinneal is a code owner

@Hixie Hixie Awaiting requested review from Hixie Hixie is a code owner

@Piinks Piinks Awaiting requested review from Piinks

@goderbauer goderbauer Awaiting requested review from goderbauer goderbauer is a code owner

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: animations p: camera p: dynamic_layouts The dynamic_layouts package p: espresso p: flutter_plugin_android_lifecycle p: google_maps_flutter p: google_sign_in p: image_picker p: in_app_purchase p: local_auth p: path_provider p: pigeon p: quick_actions p: rfw Remote Flutter Widgets p: shared_preferences p: url_launcher p: video_player p: webview_flutter platform-android
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stuartmorgan @reidbaker