[Build] Enable Non-Transitive Resources #2766
Conversation
This is long overdue and shouldn't cause any issues to WPAndroid and/or WCAndroid whatsoever. For more info see WPAndroid PR below, and more specifically the discussion about 'FluxC' and the below final comment on that: Drop jetifier #15675: - wordpress-mobile/WordPress-Android#15675 FluxC was never part of the plan: - https://github.com/wordpress-mobile/WordPress-Android/pull/ 15675#issuecomment-1057840724 ------------------------------------------------------------------------ Also, as part of this change, this 'android.jetifier.ignorelist' Gradle property got removed as well. This Gradle property was added to fix Robolectric tests in CI (df56fb1), then updated to newer equivalent (232ce17), but I am not sure if this is still needed, especially now that Jetifier is disabled.
This fixes the below 'AndroidX' related warning with 'android.enableJetifier=true'. This behavior will not be allowed in Android Gradle plugin 8.0. Another way to do it, which might be an even better and more safe way of doing it, is to update 'wellsql', then create and use a newer version, one that removes any 'com.android.support' references from it. However, mind the fact that, in order to do such an update to 'wellsql', lots need to be done to get this project up to that point, it being out-dated and unmaintained for so long (last '1.7.0' release about 1,5 years ago). wellsql: https://github.com/wordpress-mobile/wellsql ------------------------------------------------------------------------ > Configure project :plugins:woocommerce ... This behavior will not be allowed in Android Gradle plugin 8.0. Please use only AndroidX dependencies or set `android.enableJetifier=true` in the `gradle.properties` file to migrate your project to AndroidX (see https://developer.android.com/jetpack/ androidx/migrate for more info). The following legacy support libraries are detected: :fluxc:debugRuntimeClasspath -> org.wordpress:wellsql:1.7.0 -> com.android.support:support-annotations:28.0.0 WARNING:Your project has set `android.useAndroidX=true`, but configuration `:example:debugRuntimeClasspath` still contains legacy support libraries, which may cause runtime issues. This behavior will not be allowed in Android Gradle plugin 8.0. Please use only AndroidX dependencies or set `android.enableJetifier=true` in the `gradle.properties` file to migrate your project to AndroidX (see https://developer.android.com/jetpack/ androidx/migrate for more info). The following legacy support libraries are detected: :example:debugRuntimeClasspath -> project :plugins:woocommerce -> org.wordpress:wellsql:1.7.0 -> com.android.support: support-annotations:28.0.0 > Task :plugins:woocommerce:generateDebugRFile UP-TO-DATE WARNING:Your project has set `android.useAndroidX=true`, but configuration `:plugins:woocommerce:debugRuntimeClasspath` still contains legacy support libraries, which may cause runtime issues. This behavior will not be allowed in Android Gradle plugin 8.0. Please use only AndroidX dependencies or set `android.enableJetifier=true` in the `gradle.properties` file to migrate your project to AndroidX (see https://developer.android.com/jetpack/ androidx/migrate for more info). The following legacy support libraries are detected: :plugins:woocommerce:debugRuntimeClasspath -> org.wordpress:wellsql:1.7.0 -> com.android.support: support-annotations:28.0.0 ------------------------------------------------------------------------
This commit doesn't updates the encrypted 'gradle.properties.enc' file to point to that pinned hash which has the 'android.nonTransitiveRClass=true' property added into the 'gradle.properties' file for the FluxC project. FYI: It seems that this is not necessary for the CI to pick-up this specific change and download the correct version of 'gradle.properties' to then build the project with non transitive resources enabled. Instead, the CI is just copy-pasting the existing version of the 'gradle.properties-example' file, into the 'gradle.properties' file, and then works itself from there. PS: Maybe the 'gradle.properties.enc' file should be deleted from this project in order to avoid further confusion on that matter going forward.
UPDATE: I went ahead and created this |
There was a problem hiding this comment.
Please also notice this 8fff0be and its description. This commit doesn't updates the encrypted gradle.properties.enc file as it seems that this kind of encrypted gradle.properties file isn't available for this project.
This library, similar to stories-android, is using the configure Gradle plugin. You should be able to update the encrypted file with ./gradlew updateConfiguration. Without the secrets, the instrumented tests wouldn't work.
UPDATE: I went ahead and created this wellsql related PR anyway, I think it is worth the trouble, that is, for the amount of time it took me to deal with it today: wordpress-mobile/wellsql#24 😊
Thank you for taking care of this! I've approved that PR. If all the work for that repo is done, could you merge the PR, tag it and update this PR with the new version? I assume we'll no longer need to exclude com.android.support with that update. 🤞 I would have been happy to take care of all this, but didn't want to intervene without your explicit approval 😅
FYI: This change was done for testing purposes and until the below 'Android Dependency Catalog' #19 PR gets merged to 'trunk'. When that's done, the 'catalogVersion' will be updated to '1.11.0' instead. PR: Automattic/android-dependency-catalog#19
|
👋 @oguzkocer !
Ah, probably I didn't make myself clear, apologies Oguz. 😊 What I was trying to say is that there is no Am I am missing something obvious... 😊
Done: f9c899d FYI: Updating |
Ah, sorry about that. Yes, I think we can remove the unused encrypted file. |
As per 8fff0be this file is not necessary for the CI to pick-up this specific change and download the correct version of 'gradle.properties' to then build the project.
Awesome, thanks the confirmation on that @oguzkocer ! 🙇 Done: 618bce0 |
|
👋 @oguzkocer ! FYI: Catalog was updated to latest 1.11.0, and with that, this PR is ready for another review: e2ae59c |
Catalog: https://github.com/Automattic/android-dependency-catalog/ releases/tag/1.11.0 WellSQL: https://github.com/wordpress-mobile/wellsql/ releases/tag/1.8.0 ------------------------------------------------------------------------ FYI: f9c899d
ParaskP7
force-pushed
the
build/enable-non-transitive-resources
branch
from
e12ff67
to
e2ae59c
Compare
There was a problem hiding this comment.
Thanks for taking care of this @ParaskP7! I've tested it with WPAndroid and it seems to be working as expected.
I am going to merge this PR so that I can include it in the upcoming code freeze, so that these changes are tested before we upgrade AGP.
Also note that I couldn't directly test it with WCAndroid because this branch is behind trunk and there is a breaking change. However, I'll test it with the merge commit afterwards and make sure to follow up if I find any issues with it.
|
Awesome, thank you for reviewing, testing and merging this @oguzkocer , you rock! 🙇 ❤️ 🚀
Noted! 👍 |
This PR is a prerequisite for the
Gradle 8.1.1 & AGP 8.0.2 Upgrade for WPAndroid, WCAndroid & Related Libsproject.Platform Request:
pdnsEh-13V-p2Project Thread:
paaHJt-57Z-p2This PR enables non-transitive resources (
android.nonTransitiveRClass) for the project.FYI: This behavior becomes the default in AGP
8.0and higher. As such, this becomes a prerequisite for the AGP8.0.2upgrade, that is of course, unlessandroid.nonTransitiveRClassis explicitly set tofalse.PS: This PR also disables Jetifier (87139a6), something that was already discussed back then (1,5 years ago), but we never got around to applying and testing this change.
To test:
FYI: Due to the above 87139a6 and 251b1a5 changes, I am also considering updating the wellsql project and level-it-up with all other repos that we are currently supporting. Maybe then, we can drop the
com.android.supportgroup dependency exclusion from thewellsqldependency altogether, and this way, to avoid any future bottlenecks when working with wellsql. Wdyt @oguzkocer ?Please also notice this 8fff0be and its description. This commit doesn't updates the encrypted
gradle.properties.encfile as it seems that this kind of encryptedgradle.propertiesfile isn't available for this project.Can you please confirm the above? 🤔
And if that's the case, let's take this opportunity to remove any such encryption related functionality from this repo.