pocket-casts-android icon indicating copy to clipboard operation
pocket-casts-android copied to clipboard

set up free/play build flavors and move CastOptionsProvider to play

Open eighthave opened this issue 2 years ago • 15 comments

Description

The sets up the standard pattern for building free releases of apps that also use proprietary libraries. This draws on patterns used by Nextcloud, Element and other apps. There are two build flavors:

  • free: this should ultimately end up building a free open source APK.
  • gplay: this builds the current version, with all the existing proprietary dependencies.

This includes one fix as an example of how build flavors work for this kind of thing. The CastOptionsProvider comes from the proprietary Chromecast library. This moves all that to the gplay flavor.

first step towards #424

Testing Instructions

./gradlew assemble
./gradlew assembleFreeDebug
./gradlew assembleGplayDebug
  • Check whether Chromecast works in the gplay APK.

Checklist

  • [ ] If this is a user-facing change, I have added an entry in CHANGELOG.md
  • [x] Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
  • [x] I have considered whether it makes sense to add tests for my changes
  • [x] All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
  • [x] Any jetpack compose components I added or changed are covered by compose previews

eighthave avatar Jul 25 '23 17:07 eighthave

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jul 25 '23 17:07 CLAassistant

  1. Chromecasting is failing

Oops, sorry about that. I used the wrong name for the flavor folder ("play" when it should have been "gplay". I don't have an easy way to test with Chromecast. So I used diffoscope to do a before/after diff on the APKs. I can see before the class and manifest entry were missing, now they are there.

  1. CI is failing

Should be fixed.

  1. [Nice to have] Automotive and Wear app modules and the free flavor

Sounds good to me. I worked with Gradle Build Flavors a fair amount, and I can't think of how to do this. Do you have a suggestion? Given the structure with base.gradle being shared across modules, I think it would require a bigger change to the Gradle setup.

eighthave avatar Aug 04 '23 18:08 eighthave

Thanks for the updates!

For removing the build variants, I think something along these lines in the app and wear modules works to exclude the free variants.

android {

    //...

    variantFilter { variant ->
        if (variant.flavors*.name.contains("free")) {
            // Automotive app should not have a free variant
            setIgnore(true)
        }
    }
}

Unrelated to this, CI is still failing (but it's a new failure at least). I haven't found the fix yet, but it looks like the useGoogleServicesDebugFile task is not getting run anymore, and this is causing the build to fail due to the missing google-services.json file. If I execute ./gradlew :app:preBuild --info from the main branch, the useGoogleServicesDebugFile task runs, but it gets skipped with the changes on this PR (it gets skipped because "task onlyIf 'Task is enabled' is false"). 🤔

mchowning avatar Aug 04 '23 21:08 mchowning

I'll try adding the code that removes the free tasks from wear and automotive.

I also saw that build failure intermittently on my machine. I can't actually see the build logs, it seems I need an Automattic account to see them on buildkite.com? Other CI services just let me click and see them.

I would solve it by removing that task, and just committing the google-services.json file in the debug flavor folder, e.g.:

git mv app/google-services.json_debug-only app/src/debug/

I don't understand what exactly that task is doing beyond just putting the file somewhere where the build will pick it up.

eighthave avatar Aug 05 '23 07:08 eighthave

I'll be AFK for vacation, I'll come back to this when I'm back.

eighthave avatar Aug 05 '23 22:08 eighthave

I'd like to continue to work on this. Any feedback on https://github.com/Automattic/pocket-casts-android/pull/1206#issuecomment-1666427743 ?

eighthave avatar Sep 29 '23 19:09 eighthave

I would solve it by removing that task, and just committing the google-services.json file in the debug flavor folder, e.g.:

git mv app/google-services.json_debug-only app/src/debug/

I don't understand what exactly that task is doing beyond just putting the file somewhere where the build will pick it up.

Seems like that should work—at least I can't think of any reason that wouldn't work now. It would be nice to simplify the build process. 👍

I'm assuming we won't need to update .configure since release builds would still fall back to the file at the root. We'll probably need to update .gitleaksignore though to ignore the new google-services.json files in the debug folders (and we can remove the currently ignored files if those get removed).

Since this change is directly dealing with how we handle secrets, let's make this change in a separate PR so it is easier to review and debug any issues that might crop up.

mchowning avatar Oct 02 '23 13:10 mchowning

Since this change is directly dealing with how we handle secrets, let's make this change in a separate PR so it is easier to review and debug any issues that might crop up.

I posted https://github.com/Automattic/pocket-casts-android/pull/1434

eighthave avatar Oct 03 '23 13:10 eighthave

I rebased this on #1434 and added https://github.com/Automattic/pocket-casts-android/pull/1206#issuecomment-1666200671 to remove the free flavors from Automotive and Wear. The builds/tests pass locally

eighthave avatar Oct 06 '23 07:10 eighthave

I rebased this now that #1434 is merged, so this should be ready to go. Everything works locally for me. The one thing I couldn't figure out was how to use the new version catalogs syntax in the productFlavor block. The old syntax works there still.

eighthave avatar Oct 23 '23 10:10 eighthave

Let me know when you're close to reviewing this and I can rebase and fix the merge conflicts.

eighthave avatar Nov 13 '23 19:11 eighthave

I'm ready to pick this up again, just let me know when you can review it, and I'll rebase it on master.

eighthave avatar Jan 17 '24 07:01 eighthave

I'm switching away from being a Pocket Casts core contributor, so someone else from @Automattic/pocket-casts-android will need to review this going forward.

mchowning avatar Jan 19 '24 15:01 mchowning

Could someone else from @Automattic/pocket-casts-android review this? It looks like this was well on its way to being a useful step towards F-Droid eligibility #424 ...

@mchowning wrote:

I'm switching away from being a Pocket Casts core contributor, so someone else from @Automattic/pocket-casts-android will need to review this going forward.

julianfoad avatar Jun 21 '24 05:06 julianfoad

If I know that it'll be reviewed, I'll happily fix the merge conflicts.

eighthave avatar Jun 21 '24 06:06 eighthave