fix package names and add support for incremental compilation

[tolot27]

This PR affects a lot of files because it changes package names to all lowercase (see Naming a Package). The app works still very well.

If we would upgrade the Android Gradle plugin directly to 3.6.x or higher and enable android.useAndroidX=true and android.enableJetifier=true in gradle.properties, we would not have to apply these changes. IMHO, it is a good idea to fix the package names.

[tolot27]

Oops, this PR should be against lib_reorg.

[tolot27]

Oops, this PR should be against lib_reorg.

It is now against current master. Hence, it can be merged independendly.

BTW: Package names in comments were not renamed to keep the commit small.

[tzachi-dar]

Changing all this files will make it very hard to other pull requests and branches.
Is this really necessary to do all this changes (that is package names to small letters)?

[tolot27]

Is this really necessary to do all this changes (that is package names to small letters)?

If we don't want to go directly to AndroidX support using jettifyier (I've another branch in preparation for it), we have to rename at least all packages using annotations. This maybe only affect a smaller subset of files. I'll change this PR later.

Anyway, renaming packages to fulfill the package naming convention does not break the business logic. Maybe we should consider this after narrowing down the number of open PRs and do this in smaller PRs.

[tolot27]

Is this really necessary to do all this changes (that is package names to small letters)?

If we don't want to go directly to AndroidX support using jetifier (I've another branch in preparation for it), we have to rename at least all packages using annotations. This maybe only affect a smaller subset of files. I'll change this PR later.

There is no way around fixing the package names. After migration to AndroidX, the same errors occur:

import com.eveningoutpost.dexdrip.Models;
                                 ^
  symbol:   class Models
  location: package com.eveningoutpost.dexdrip

Tomorrow, I'll try to just fix the relevant packages: com.eveningoutpost.dexdrip.Models and com.eveningoutpost.dexdrip.UtilityModels.

[tolot27]

The new PR #1598 is much smaller and does not rename the packages, which is still necessary in the future.

[jamorham]

Instead of changing 1200+ files and creating merge conflicts with every feature branch out there, can we not try any of these alternative approaches:

1. Fix the bug in android data binding. I did some investigations with 3.4.3 and from what I remember it was the generated import statements that didn't understand mixed case package names. Which is odd because this appeared to be a test case within the databinding unit tests. There may be more to it than that but as this is a regression between v1 and v2 databinding. Couldn't we fix that?

2. Use something like ShadowJar's relocate feature to rename packages during compilation stage.

3. Use proguard applymapping feature to rename the classes which suffer the issue. This might not occur early enough in the build process.

[tolot27]

Instead of changing 1200+ files and creating merge conflicts with every feature branch out there, can we not try any of these alternative approaches:

1. Fix the bug in android data binding. I did some investigations with 3.4.3 and from what I remember it was the generated import statements that didn't understand mixed case package names. Which is odd because this appeared to be a test case within the databinding unit tests. There may be more to it than that but as this is a regression between v1 and v2 databinding. Couldn't we fix that?

Please can you provide a link to the unit tests you found? At the other side, we may also have to patch AGP and any future version of the databinding library, if we need other features in the future or security fixes. IMHO, not a good idea.

The main cause of the problem is the incorrect package naming. Hence, I would not call it a regression. Instead, it was an unsupported feature in v1.

2. Use something like ShadowJar's relocate feature to rename packages during compilation stage.

A shadow jar is one big jar containing all packages/classes unpacked from their original library jars. Hence, not available during databinding annotation.

3. Use proguard applymapping feature to rename the classes which suffer the issue. This might not occur early enough in the build process.

Yes, this mapping is mainly useful for incremental obfuscation and not available during databinding/annotation.

I tried a 4th approach: creating lowercase package naming folders and migrating the POJO classes incrementally. I've not finished this yet but it worked for a few classes, even in Windows build environment. But it looks not very nice having to directories with the same name but different casing.

If I remember correctly, I even had to fix the package names after migration to androidx. I highly recommend combining both steps and maybe combine it further with the extraction of all hard-coded strings into resource strings.

Regarding to potential merge conflicts with feature branches: The features can either be merged before or fixed afterwards at the time they get integrated. Many features (PRs) laying around for months or years and maybe never get merged.

AND: I'm willing to patch these feature branch (all or those CI does not complain) with a script to fix potential merge conflicts, if that finally advances migration and further development.

[jamorham]

Here is the mixed case data-binding test I saw: https://android.googlesource.com/platform/frameworks/data-binding/+/897cf5a29761b556e392382086a29355a2841022/integration-tests/ViewBindingTestApp/app/src/androidTest/java/com/example/viewbinding/WeirdCasingTest.java

As far as I know mixed case package names are legal in java but very rarely used as the convention is to always lower-case them.

So there is a 5th way to solve this which I have put together and looks like it works. It uses a gradle pre-processor to remap the package names to lower case in the java source and xml files during compilation. This allows android.databinding.enableV2=true to work and I've tested with android gradle tools up to 3.5.4. I've done some limited testing of the resultant app and the activities which use data binding looked okay. Changes to the source are not required with this method.

The pre-processing takes about 1 second but I still need to do some more work to make sure it properly injects in to the build process with regards to caching, ide ui etc. The AndroidManifest also needs slightly different handling. I'll push something up soon once I've refined it a bit further.

[tolot27]

So there is a 5th way to solve this which I have put together and looks like it works. It uses a gradle pre-processor to remap the package names to lower case in the java source and xml files during compilation.

Cool. I've searched for a long time for such a solution.

The pre-processing takes about 1 second but I still need to do some more work to make sure it properly injects in to the build process with regards to caching, ide ui etc. The AndroidManifest also needs slightly different handling. I'll push something up soon once I've refined it a bit further.

That would be really interesting. It would be great if I can be involved in testing.

[jamorham]

@tolot27 Sure, feel free to test it out: #1621

[tolot27]

Most of these changes are implemented by recent commits (i. e. 9da21f0). Hence, maybe only 87b2205 will be necessary.