Skip to content

Teach the flutter tool how to find android output files if the flavor contains uppercase letters#65768

Closed
knaeckeKami wants to merge 0 commit intoflutter:masterfrom
knaeckeKami:master
Closed

Teach the flutter tool how to find android output files if the flavor contains uppercase letters#65768
knaeckeKami wants to merge 0 commit intoflutter:masterfrom
knaeckeKami:master

Conversation

@knaeckeKami
Copy link
Contributor

@knaeckeKami knaeckeKami commented Sep 14, 2020

Description

This changes the name of the generated output files on android in case a flavor with uppercase letters is used.
Previously, the lowercased flavor name would be used for the apk/aab file. Now, the flavor name is used as-is.

Example: when adding a flavor with the name "aB" to the flutter template project, the name of the generated .aab file is now

app-aB-release.aab instead of
app-ab-release.aab

Because of calling .toLowerCase() on the flavor name, the generated apk/aab files would not be found after the Gradle build for users that use a case sensitive file system (typically Linux, but not Windows or Mac OS), and they would get an error message

Gradle build failed to produce a .apk file. It's likely that this file was generated under /builds/<...>/build, but the tool couldn't find it.

because the flutter tool looks for a file with the real flavor name in its name, not the lowercased flavor name.
See https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/android/gradle.dart#L900

Related Issues

fixes #24106 , particularly #24106 (comment)

Tests

I did not add tests. Since this is a change in the gradle file for the android build process, I don't know how I would add a test for this. If tests are required, I would appreciate a pointer on how and where to add them.

Checklist

Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read [Handling breaking changes].

This should not be a breaking change.
If one does not use uppercase letters in flavor names, nothing changes.

Otherwise, for Windows and Mac, this name of the generated file apk will be different, however, since the file systems of these operating systems are not case sensitive, this should not cause problems.
On Linux, this should also not be a breaking change, since using a flavor name with uppercase letters caused the build to fail anyway previously.

Alternatives

If using upper case letters in the output files is not desired, we could also fix the expected file names in
https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/android/gradle.dart to use the lower case flavor names.

@flutter-dashboard flutter-dashboard bot added tool Affects the "flutter" command-line tool. See also t: labels. work in progress; do not review labels Sep 14, 2020
@knaeckeKami knaeckeKami marked this pull request as ready for review September 14, 2020 17:52
@knaeckeKami knaeckeKami changed the title do not lowercase the flavor name for the android output file name o not lowercase the flavor name for the android output file name Sep 14, 2020
@knaeckeKami knaeckeKami changed the title o not lowercase the flavor name for the android output file name Do not use lowercase for the flavor name for the android output file name Sep 14, 2020
@knaeckeKami
Copy link
Contributor Author

knaeckeKami commented Sep 17, 2020

I think this PR might be ignored because it still has the "work in progress, do not review" label set, which gets set automatically by the bot, but does not get removed automatically (see #63210 ).

Can someone remove that label? @jmagman Maybe?

@jmagman
Copy link
Member

jmagman commented Sep 17, 2020

@knaeckeKami Done, sorry about that. Created #66047.

@jmagman jmagman requested a review from blasten September 17, 2020 18:20
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

@jonahwilliams
Copy link
Contributor

We're going to see if we can repurpose an existing test to cover this

@blasten
Copy link

blasten commented Sep 24, 2020

I think the fix is to apply lowercase in the Dart side, so maybe let's change that as opposed to the .gradle file.

Copy link

@blasten blasten left a comment

Choose a reason for hiding this comment

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

Apply lowercase in the Dart side

@knaeckeKami
Copy link
Contributor Author

Ok, applying this to the dart side is more complex, i hope i caught all the cases

@knaeckeKami knaeckeKami requested a review from blasten September 25, 2020 01:05
@knaeckeKami knaeckeKami changed the title Do not use lowercase for the flavor name for the android output file name Teach the flutter tool how to find android output files if the flavor contains uppercase letters Sep 25, 2020
Copy link

@blasten blasten left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks for the update! I saw some tests failed, maybe rebase to retrigger the checks with the latest changes on master.

@knaeckeKami
Copy link
Contributor Author

Ah I didn't mean to close this PR. Could you reopen it? I don't have the rights

@jonahwilliams
Copy link
Contributor

@knaeckeKami we can't recreate your PR, you need to re-open it through the github UI

@jonahwilliams
Copy link
Contributor

it looks like your force push deleted all of the changes

@knaeckeKami
Copy link
Contributor Author

Ok, strange. See #66687

@knaeckeKami
Copy link
Contributor Author

knaeckeKami commented Sep 25, 2020

Yeah, I messed up since I had other changes in flutter that I forgot about, but the force push should have fixed it. But Github now says that there are no new commits in my branch, but it works fine on the new PR, even if it's against the same branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

On Linux, problems building an Android app that uses flavor dimensions with uppercase characters

5 participants