Skip to content

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
Contributor

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