[flutter_tools] Add namespace getter in Android project; use namespace as fallback #121416

navaronbracke · 2023-02-24T18:09:32Z

For newer Android projects the package attribute in the Manifest has been replaced with a namespace attribute under the android section of the build.gradle file.

List which issues are fixed by this PR. You must list at least one issue.
Fixes: #121404

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
N/A

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.

…r package attribute in Manifest

flutter-dashboard · 2023-02-24T18:09:36Z

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.

navaronbracke · 2023-02-24T18:11:45Z

@christopherfujino Could you give me a hint for a test? I'd want to have a stub build.gradle with the namespace tag in it and a manifest without a package attribute. Then I'd try to build an apk and assert on the package identifier?

I just don't know how to do that exactly. Edit: I think I got it, except for a remaining expect()

navaronbracke · 2023-02-24T20:37:26Z

packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart

+
+      expect(processManager, hasNoRemainingExpectations);
+
+      // TODO: verify the packageId of the built APK equals the namespace value


So this test verifies that building an APK using the namespace works, but I'd like to assert that the namespace value is also correct. Any tips on how to do that? Or is that not required (as the test already validates that the APK is built without errors)

I don't think this test validates your change--I reverted your lib/src changes and this test still passes.

Can you test AndroidApk.fromAndroidProject()?

navaronbracke · 2023-03-08T16:10:11Z

packages/flutter_tools/lib/src/project.dart

@@ -486,9 +487,15 @@ class AndroidProject extends FlutterProjectPlatform {
  }

  File get appManifestFile {
-    return isUsingGradle


@christopherfujino I had mismatching results when originally running this test.
The fileSystem created in the test would report that the appManifestFile exists, but the globals.fs file system in this getter reported that it did not exist.

According to a doc comment I can override globals.fs in tests. I don't see the setUp((){}) doing something like that. What would be the right thing to do here?

A) Override globals.fs in the test
B) Fix the getter to use hostAppGradleRoot, which does use the fileSystem that was set up in the specific test (which is what I did here)

Good find. Looks like whatever test you copy-pasted this from was also leaking the real file system. The whole purpose of the testUsingContext() function you're calling is that it allows you to provide overrides to the Zone/context getters (globals.fs is essentially calling `context.get();).

Here is an example of constructing a FileSystem in setUp and then providing it as a context override for a testUsingContext invocation: https://github.com/flutter/flutter/blob/master/packages/flutter_tools/test/commands.shard/hermetic/build_test.dart#L60

although if it makes sense, what you did here is probably better (it looks cleaner)

Thanks for the tip, ~~I'll update the test with an override for the file system.~~ I'll keep that in mind for the next time I need to write tests like this.

packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart

navaronbracke · 2023-03-09T11:53:16Z

@christopherfujino fixed the test (forgot the activity tag, woops) and now it no longer hangs when running the tests.
I wonder why it hung in the first place, perhaps the huge amount of tests in flutter_tools has something to do with it?

christopherfujino

LGTM! Thanks for sticking through, I know this was tricky to test.

eliasyishak

👍

…namespace as fallback (flutter/flutter#121416)

…e as fallback (flutter#121416) [flutter_tools] Add namespace getter in Android project; use namespace as fallback

…namespace as fallback (flutter/flutter#121416)

add namespace getter in Android project; use namespace as fallback fo…

9fe145b

…r package attribute in Manifest

flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Feb 24, 2023

navaronbracke added 2 commits February 24, 2023 19:48

Merge branch 'master' into android_gradle_namespace_fix

2593dd0

add test

b467fab

navaronbracke commented Feb 24, 2023

View reviewed changes

navaronbracke added 6 commits February 24, 2023 23:06

Merge branch 'master' into android_gradle_namespace_fix

39bbc14

Merge branch 'master' into android_gradle_namespace_fix

f186207

Merge branch 'master' into android_gradle_namespace_fix

d90fd58

Merge branch 'master' into android_gradle_namespace_fix

2f949ae

use hostAppGradleRoot file system in appManifestFile getter

ccc6930

test AndroidApk.fromAndroidProject() instead

7254a0f

navaronbracke commented Mar 8, 2023

View reviewed changes

packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart Show resolved Hide resolved

christopherfujino reviewed Mar 8, 2023

View reviewed changes

packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart Outdated Show resolved Hide resolved

navaronbracke added 3 commits March 9, 2023 08:00

Merge branch 'master' into android_gradle_namespace_fix

162e2aa

remove import

0580a5b

add missing activity tag

9b6af20

navaronbracke requested a review from christopherfujino March 9, 2023 11:53

remove unnecessary override in test

f9edfde

christopherfujino approved these changes Mar 9, 2023

View reviewed changes

christopherfujino requested a review from eliasyishak March 9, 2023 20:14

eliasyishak approved these changes Mar 9, 2023

View reviewed changes

eliasyishak added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 9, 2023

auto-submit bot merged commit 57171a3 into flutter:master Mar 9, 2023

navaronbracke deleted the android_gradle_namespace_fix branch March 10, 2023 06:55

engine-flutter-autoroll mentioned this pull request Mar 10, 2023

Roll Flutter from 5a279edc239f to 1f426123230b (53 revisions) flutter/packages#3437

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 10, 2023

57171a3 [flutter_tools] Add namespace getter in Android project; use …

682ad8e

…namespace as fallback (flutter/flutter#121416)

engine-flutter-autoroll mentioned this pull request Mar 10, 2023

Roll Flutter from 5a279edc239f to e98c53751ef9 (55 revisions) flutter/packages#3440

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 10, 2023

57171a3 [flutter_tools] Add namespace getter in Android project; use …

0f54645

…namespace as fallback (flutter/flutter#121416)

engine-flutter-autoroll mentioned this pull request Mar 10, 2023

Roll Flutter from 5a279edc239f to 3f2cca8eebfd (56 revisions) flutter/packages#3441

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 10, 2023

57171a3 [flutter_tools] Add namespace getter in Android project; use …

f6a5c5c

…namespace as fallback (flutter/flutter#121416)

hangyujin pushed a commit to hangyujin/flutter that referenced this pull request Mar 11, 2023

[flutter_tools] Add namespace getter in Android project; use namespac…

cfea130

…e as fallback (flutter#121416) [flutter_tools] Add namespace getter in Android project; use namespace as fallback

engine-flutter-autoroll mentioned this pull request Mar 11, 2023

Roll Flutter from 5a279edc239f to f9ad42a32d7e (69 revisions) flutter/packages#3443

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 11, 2023

57171a3 [flutter_tools] Add namespace getter in Android project; use …

18146cd

…namespace as fallback (flutter/flutter#121416)

engine-flutter-autoroll mentioned this pull request Mar 12, 2023

Roll Flutter from 5a279edc239f to 9ba0d08ebc07 (72 revisions) flutter/packages#3445

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 12, 2023

57171a3 [flutter_tools] Add namespace getter in Android project; use …

6c3bbc2

…namespace as fallback (flutter/flutter#121416)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

57171a3 [flutter_tools] Add namespace getter in Android project; use …

ca241aa

…namespace as fallback (flutter/flutter#121416)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023

57171a3 [flutter_tools] Add namespace getter in Android project; use …

96f3369

…namespace as fallback (flutter/flutter#121416)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[flutter_tools] Add namespace getter in Android project; use namespace as fallback #121416

[flutter_tools] Add namespace getter in Android project; use namespace as fallback #121416

navaronbracke commented Feb 24, 2023 •

edited

Loading

flutter-dashboard bot commented Feb 24, 2023

navaronbracke commented Feb 24, 2023 •

edited

Loading

navaronbracke Feb 24, 2023

christopherfujino Mar 3, 2023

navaronbracke Mar 8, 2023

christopherfujino Mar 8, 2023

christopherfujino Mar 8, 2023

navaronbracke Mar 8, 2023 •

edited

Loading

navaronbracke commented Mar 9, 2023 •

edited

Loading

christopherfujino left a comment

eliasyishak left a comment


		expect(processManager, hasNoRemainingExpectations);

		// TODO: verify the packageId of the built APK equals the namespace value

[flutter_tools] Add namespace getter in Android project; use namespace as fallback #121416

[flutter_tools] Add namespace getter in Android project; use namespace as fallback #121416

Conversation

navaronbracke commented Feb 24, 2023 • edited Loading

Pre-launch Checklist

flutter-dashboard bot commented Feb 24, 2023

navaronbracke commented Feb 24, 2023 • edited Loading

navaronbracke Feb 24, 2023

Choose a reason for hiding this comment

christopherfujino Mar 3, 2023

Choose a reason for hiding this comment

navaronbracke Mar 8, 2023

Choose a reason for hiding this comment

christopherfujino Mar 8, 2023

Choose a reason for hiding this comment

christopherfujino Mar 8, 2023

Choose a reason for hiding this comment

navaronbracke Mar 8, 2023 • edited Loading

Choose a reason for hiding this comment

navaronbracke commented Mar 9, 2023 • edited Loading

christopherfujino left a comment

Choose a reason for hiding this comment

eliasyishak left a comment

Choose a reason for hiding this comment

navaronbracke commented Feb 24, 2023 •

edited

Loading

navaronbracke commented Feb 24, 2023 •

edited

Loading

navaronbracke Mar 8, 2023 •

edited

Loading

navaronbracke commented Mar 9, 2023 •

edited

Loading