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.

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

Allow numbers and uppercase letters in app package id #4326

Merged
merged 1 commit into from
Sep 27, 2020

Conversation

Stypox
Copy link
Member

@Stypox Stypox commented Sep 26, 2020

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

When I work on PRs from other people I am used to calling the local branches prNUMBER, i.e. pr3178. But all such names resulted the same app package id, since numbers were stripped completely, leading to conflicting APKs (e.g. #4259 (comment) ). This PR fixes the issue by only stripping the numbers at the beginning of the git branch, as required by Android, but not all of the others. While I was at it I also removed toLowercase() since package ids needn't be only lowercase, according to the same SO.

Agreement

@avently
Copy link
Contributor

avently commented Sep 27, 2020

Since you are changing build.gradle could you please consider adding info I asked a couple of weeks ago in #4141 ?

Something like that is enough:

pr {
           multiDexEnabled true

           // suffix the app id and the app name with git branch name
           def workingBranch = getGitWorkingBranch()
           def normalizedWorkingBranch = workingBranch.replaceFirst("^[^A-Za-z]+", "").replaceAll("[^0-9A-Za-z]+", "")
           if (normalizedWorkingBranch.isEmpty() || workingBranch == "master" || workingBranch == "dev") {
               // default values when branch name could not be determined or is master or dev
               applicationIdSuffix ".debug"
               resValue "string", "app_name", "NewPipe Debug"
           } else {
               applicationIdSuffix "." + normalizedWorkingBranch
               resValue "string", "app_name", "NewPipe " + workingBranch
               archivesBaseName = 'NewPipe_' + normalizedWorkingBranch
           }
       }

Here: https://github.com/TeamNewPipe/NewPipe/blob/dev/app/build.gradle#L47

It will allow to build non-debuggable apk (i.e. fast) with predefined suffix and app name by just invoking:
./gradlew assemblePr

Of couse with the changes you do for debug build in this PR

@TobiGr
Copy link
Member

TobiGr commented Sep 27, 2020

@Stypox your change looks good.
@avently I think naming release builds "Debug" is misleading.
I'd suggest to do something like this:

    pr {
           multiDexEnabled true

           // suffix the app id and the app name with git branch name
           def workingBranch = getGitWorkingBranch()
           def normalizedWorkingBranch = workingBranch.replaceFirst("^[^A-Za-z]+", "").replaceAll("[^0-9A-Za-z]+", "")
           if (normalizedWorkingBranch.isEmpty() || workingBranch == "master" || workingBranch == "dev") {
               // default values when branch name could not be determined or is master or dev
               applicationIdSuffix ".releaseTest"
               resValue "string", "app_name", "NewPipe Test"
           } else {
               applicationIdSuffix "." + normalizedWorkingBranch
               resValue "string", "app_name", "NewPipe " + workingBranch
               archivesBaseName = 'NewPipe_' + normalizedWorkingBranch
           }
       }

Apart from that, we also need to change

public static final boolean DEBUG = !BuildConfig.BUILD_TYPE.equals("release");

I am not sure if there are more changes necessary. This needs to be tested properly and I'd therefore suggest to do it in another PR.

@B0pol
Copy link
Member

B0pol commented Sep 27, 2020

yes, it should be done in another PR

@B0pol B0pol merged commit a9fafe9 into TeamNewPipe:dev Sep 27, 2020
This was referenced Sep 27, 2020
@TobiGr TobiGr added the meta Related to the project but not strictly to code label Sep 29, 2020
@Stypox Stypox deleted the appid-numbers branch August 4, 2022 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Related to the project but not strictly to code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants