Fix path name to discover debug apk on add2app builds #117999

GaryQian · 2023-01-04T20:32:42Z

This issue was introduced by #113622

Android Add2app builds output the debug APK in build/host/outputs/apk/debug/app-debug.apk but we were searching in the directory without /debug/

This doesn't occur with the other build modes (profile, release)

flutter-dashboard · 2023-01-04T20:32:45Z

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.

christopherfujino · 2023-01-05T21:37:22Z

packages/flutter_tools/lib/src/android/application_package.dart

@@ -116,7 +116,11 @@ class AndroidApk extends ApplicationPackage implements PrebuiltApplicationPackag
    }

    if (androidProject.isUsingGradle && androidProject.isSupportedVersion) {
-      apkFile = getApkDirectory(androidProject.parent).childFile(filename);
+      Directory apkDirectory = getApkDirectory(androidProject.parent);
+      if (androidProject.parent.isModule && buildInfo?.mode == BuildMode.debug) {


What was the PR that changed this? And can you explain why we only do this extra namespace for debug modules?

This issue was caused by #113622, which stopped copying the generated apk to a fixed file name. Module/add2app projects were always outputting the apk in a buildmode subdirectory, eg <buildmode>/app-<buildmode>.apk. Previously, the copy to app.apk masked this <buildmode>/app-<buildmode>.apk output by placing the apk in app.apk without the subdir. When the copy was removed, the search path is now missing a <buildmode> subdir only for module projects.

Full flutter apps don't experience this issue due to the output dir not including the buildmode subdir.

We should also do this for the other buildmodes.

christopherfujino · 2023-01-10T21:23:41Z

@GaryQian you still working on this one?

GaryQian · 2023-01-10T22:04:31Z

Yeah, still working on it 😀

christopherfujino · 2023-01-12T21:44:27Z

Yeah, still working on it grinning

awesome

GaryQian · 2023-01-18T17:26:46Z

cc @reidbaker

christopherfujino

LGTM

christopherfujino · 2023-01-24T21:18:53Z

Merging on yellow FRoB because of an ongoing issue

…ter/flutter#117999)

…r/flutter#117999)

* 373523184 Cleanup old Dart SDK layout compatibility (flutter/flutter#118819) * 4d250302a Add leak_tracker as dev_dependency. (flutter/flutter#118952) * e3c51a2f2 Add Windows unit tests to plugin template (flutter/flutter#118638) * d20dd9e4b Roll Flutter Engine from 7d3233d26d09 to 71ee5f19bc16 (15 revisions) (flutter/flutter#119081) * 5dabe102a Fix path name to discover debug apk on add2app builds (flutter/flutter#117999) * 50ed8a34b Enable `unnecessary_null_comparison` check (flutter/flutter#118849) * 455e6aca5 Test integration test apps' runner files against current template app (flutter/flutter#118646) * a788e1b31 Roll Flutter Engine from 71ee5f19bc16 to 59ea78bfabda (2 revisions) (flutter/flutter#119087) * c35370cf0 Roll Flutter Engine from 59ea78bfabda to 2499a5d9fca7 (2 revisions) (flutter/flutter#119089) * 2f0dd5673 Refactor highlight handling in FocusManager (flutter/flutter#119075) * 2759f3f0b Roll Flutter Engine from 2499a5d9fca7 to d98926c32ee7 (2 revisions) (flutter/flutter#119090) * 760fb2115 Roll Flutter Engine from d98926c32ee7 to bec40654a5d7 (2 revisions) (flutter/flutter#119093) * bbca694ef Roll Flutter Engine from bec40654a5d7 to 5405f2c26e85 (2 revisions) (flutter/flutter#119095) * 6414c3604 f1464b49c Manually roll ANGLE, vulkan-deps, SwiftShader (flutter/engine#38650) (flutter/flutter#119097) * 426cdd90c 55bb8deaf [Impeller] Linear sample atlas glyphs when the CTM isn't translation/scale only (flutter/engine#39112) (flutter/flutter#119098) * 83c3a61e3 Only emit image painting events in debug & profile modes. (flutter/flutter#118872) * b113df2dc bffb98352 Roll Skia from b72fececbdcc to 8ffd5c20d634 (3 revisions) (flutter/engine#39114) (flutter/flutter#119099) * 351466aea Add Decoding Flutter videos to API docs (flutter/flutter#116454) * 318f8758b Pass through magnifierConfiguration (flutter/flutter#118270) * eced23eab d39ab638b Roll Fuchsia Mac SDK from MUvFS0baOnigVUIND... to _H53AyDxR9Pm2TbwN... (flutter/engine#39122) (flutter/flutter#119126) * 29ab437e2 Add Material 3 `CheckboxListTile` example and update existing examples (flutter/flutter#118792) * a815ee634 8efc7183b Roll Skia from 8ffd5c20d634 to da5034f9d117 (4 revisions) (flutter/engine#39123) (flutter/flutter#119129)

* 3735231 Cleanup old Dart SDK layout compatibility (flutter/flutter#118819) * 4d25030 Add leak_tracker as dev_dependency. (flutter/flutter#118952) * e3c51a2 Add Windows unit tests to plugin template (flutter/flutter#118638) * d20dd9e Roll Flutter Engine from 7d3233d26d09 to 71ee5f19bc16 (15 revisions) (flutter/flutter#119081) * 5dabe10 Fix path name to discover debug apk on add2app builds (flutter/flutter#117999) * 50ed8a3 Enable `unnecessary_null_comparison` check (flutter/flutter#118849) * 455e6ac Test integration test apps' runner files against current template app (flutter/flutter#118646) * a788e1b Roll Flutter Engine from 71ee5f19bc16 to 59ea78bfabda (2 revisions) (flutter/flutter#119087) * c35370c Roll Flutter Engine from 59ea78bfabda to 2499a5d9fca7 (2 revisions) (flutter/flutter#119089) * 2f0dd56 Refactor highlight handling in FocusManager (flutter/flutter#119075) * 2759f3f Roll Flutter Engine from 2499a5d9fca7 to d98926c32ee7 (2 revisions) (flutter/flutter#119090) * 760fb21 Roll Flutter Engine from d98926c32ee7 to bec40654a5d7 (2 revisions) (flutter/flutter#119093) * bbca694 Roll Flutter Engine from bec40654a5d7 to 5405f2c26e85 (2 revisions) (flutter/flutter#119095) * 6414c36 f1464b49c Manually roll ANGLE, vulkan-deps, SwiftShader (flutter/engine#38650) (flutter/flutter#119097) * 426cdd9 55bb8deaf [Impeller] Linear sample atlas glyphs when the CTM isn't translation/scale only (flutter/engine#39112) (flutter/flutter#119098) * 83c3a61 Only emit image painting events in debug & profile modes. (flutter/flutter#118872) * b113df2 bffb98352 Roll Skia from b72fececbdcc to 8ffd5c20d634 (3 revisions) (flutter/engine#39114) (flutter/flutter#119099) * 351466a Add Decoding Flutter videos to API docs (flutter/flutter#116454) * 318f875 Pass through magnifierConfiguration (flutter/flutter#118270) * eced23e d39ab638b Roll Fuchsia Mac SDK from MUvFS0baOnigVUIND... to _H53AyDxR9Pm2TbwN... (flutter/engine#39122) (flutter/flutter#119126) * 29ab437 Add Material 3 `CheckboxListTile` example and update existing examples (flutter/flutter#118792) * a815ee6 8efc7183b Roll Skia from 8ffd5c20d634 to da5034f9d117 (4 revisions) (flutter/engine#39123) (flutter/flutter#119129)

…r#3089) * 373523184 Cleanup old Dart SDK layout compatibility (flutter/flutter#118819) * 4d250302a Add leak_tracker as dev_dependency. (flutter/flutter#118952) * e3c51a2f2 Add Windows unit tests to plugin template (flutter/flutter#118638) * d20dd9e4b Roll Flutter Engine from 7d3233d26d09 to 71ee5f19bc16 (15 revisions) (flutter/flutter#119081) * 5dabe102a Fix path name to discover debug apk on add2app builds (flutter/flutter#117999) * 50ed8a34b Enable `unnecessary_null_comparison` check (flutter/flutter#118849) * 455e6aca5 Test integration test apps' runner files against current template app (flutter/flutter#118646) * a788e1b31 Roll Flutter Engine from 71ee5f19bc16 to 59ea78bfabda (2 revisions) (flutter/flutter#119087) * c35370cf0 Roll Flutter Engine from 59ea78bfabda to 2499a5d9fca7 (2 revisions) (flutter/flutter#119089) * 2f0dd5673 Refactor highlight handling in FocusManager (flutter/flutter#119075) * 2759f3f0b Roll Flutter Engine from 2499a5d9fca7 to d98926c32ee7 (2 revisions) (flutter/flutter#119090) * 760fb2115 Roll Flutter Engine from d98926c32ee7 to bec40654a5d7 (2 revisions) (flutter/flutter#119093) * bbca694ef Roll Flutter Engine from bec40654a5d7 to 5405f2c26e85 (2 revisions) (flutter/flutter#119095) * 6414c3604 f1464b49c Manually roll ANGLE, vulkan-deps, SwiftShader (flutter/engine#38650) (flutter/flutter#119097) * 426cdd90c 55bb8deaf [Impeller] Linear sample atlas glyphs when the CTM isn't translation/scale only (flutter/engine#39112) (flutter/flutter#119098) * 83c3a61e3 Only emit image painting events in debug & profile modes. (flutter/flutter#118872) * b113df2dc bffb98352 Roll Skia from b72fececbdcc to 8ffd5c20d634 (3 revisions) (flutter/engine#39114) (flutter/flutter#119099) * 351466aea Add Decoding Flutter videos to API docs (flutter/flutter#116454) * 318f8758b Pass through magnifierConfiguration (flutter/flutter#118270) * eced23eab d39ab638b Roll Fuchsia Mac SDK from MUvFS0baOnigVUIND... to _H53AyDxR9Pm2TbwN... (flutter/engine#39122) (flutter/flutter#119126) * 29ab437e2 Add Material 3 `CheckboxListTile` example and update existing examples (flutter/flutter#118792) * a815ee634 8efc7183b Roll Skia from 8ffd5c20d634 to da5034f9d117 (4 revisions) (flutter/engine#39123) (flutter/flutter#119129)

) Co-authored-by: Gary Qian <garyq@google.com>

AlexV525 · 2023-03-09T00:30:01Z

@GaryQian Can't we unify different behaviors about copying built artifacts?

gnprice · 2023-03-10T03:07:40Z

@AlexV525 A new issue would probably be a more effective place to discuss ideas for further changes, rather than this old merged PR. You can always link to the PR from the issue for reference.

Fix path name to discover apk on add2app builds

2d54109

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

GaryQian added 2 commits January 4, 2023 18:13

Add test:

2b0fa6a

Fix analyzer

80c6ca7

GaryQian requested review from reidbaker, gmackall, camsim99 and christopherfujino January 4, 2023 23:17

GaryQian added 2 commits January 4, 2023 23:58

Analyzer

6f4e7a8

Newline

ae87b1d

christopherfujino reviewed Jan 5, 2023

View reviewed changes

GaryQian added 2 commits January 18, 2023 02:09

Do for all build modes

d93b28b

Comments

17c9d43

GaryQian requested a review from christopherfujino January 18, 2023 07:20

christopherfujino approved these changes Jan 18, 2023

View reviewed changes

GaryQian added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 18, 2023

reidbaker approved these changes Jan 23, 2023

View reviewed changes

christopherfujino merged commit 5dabe10 into flutter:master Jan 24, 2023

This was referenced Jan 25, 2023

Roll Flutter from c35efdaa6854 to a815ee634202 (22 revisions) flutter/packages#3089

Merged

Roll Flutter from c35efdaa6854 to a815ee634202 (22 revisions) flutter/plugins#7025

Merged

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

5dabe102a Fix path name to discover debug apk on add2app builds (flut…

8e15f5c

…ter/flutter#117999)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jan 25, 2023

5dabe10 Fix path name to discover debug apk on add2app builds (flutte…

688d64b

…r/flutter#117999)

burnermanx pushed a commit to burnermanx/flutter that referenced this pull request Feb 22, 2023

Fix path name to discover debug apk on add2app builds (flutter#117999)

d276b2a

burnermanx pushed a commit to burnermanx/flutter that referenced this pull request Mar 3, 2023

Fix path name to discover debug apk on add2app builds (flutter#117999)

a1816e2

burnermanx mentioned this pull request Mar 3, 2023

Fix path name to discover debug apk on add2app builds (#117999) #121881

Merged

8 tasks

itsjustkevin pushed a commit that referenced this pull request Mar 8, 2023

Fix path name to discover debug apk on add2app builds (#117999) (#121881

33ab7b5

) Co-authored-by: Gary Qian <garyq@google.com>

This was referenced Mar 10, 2023

🔥 Remove the build mode name from outputs of Flutter Modules #122373

Closed

[tool] Produce similar paths for built artifacts when building APK with Flutter Modules #122802

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix path name to discover debug apk on add2app builds #117999

Fix path name to discover debug apk on add2app builds #117999

GaryQian commented Jan 4, 2023 •

edited

flutter-dashboard bot commented Jan 4, 2023

christopherfujino Jan 5, 2023

GaryQian Jan 18, 2023

christopherfujino commented Jan 10, 2023

GaryQian commented Jan 10, 2023

christopherfujino commented Jan 12, 2023

GaryQian commented Jan 18, 2023

christopherfujino left a comment

christopherfujino commented Jan 24, 2023

AlexV525 commented Mar 9, 2023

gnprice commented Mar 10, 2023

Fix path name to discover debug apk on add2app builds #117999

Fix path name to discover debug apk on add2app builds #117999

Conversation

GaryQian commented Jan 4, 2023 • edited

flutter-dashboard bot commented Jan 4, 2023

christopherfujino Jan 5, 2023

Choose a reason for hiding this comment

GaryQian Jan 18, 2023

Choose a reason for hiding this comment

christopherfujino commented Jan 10, 2023

GaryQian commented Jan 10, 2023

christopherfujino commented Jan 12, 2023

GaryQian commented Jan 18, 2023

christopherfujino left a comment

Choose a reason for hiding this comment

christopherfujino commented Jan 24, 2023

AlexV525 commented Mar 9, 2023

gnprice commented Mar 10, 2023

GaryQian commented Jan 4, 2023 •

edited