Create multiple build flavors in Bazel #2432
Assignees
Labels
Impact: Low
Low perceived user impact (e.g. edge cases).
Z-ibt
Temporary label for Ben to keep track of issues he's triaged.
Comments
|
I can try making the builds. Could you point me to any references/ resources which can be helpful? |
I think this is blocked on some issue |
|
This requires some design. It could be done now, but part of this issue is defining the flavors and and how they should be built/what they should contain. It might be best to wait until feature gating is done before starting this. |
8 tasks
8 tasks
|
bazelbuild/bazel#11497 (comment) is promising. We could try this out soon and see if bundles can build. |
BenHenning
changed the title
6 tasks
BenHenning
added a commit
that referenced
this issue
Sep 10, 2021
…, Proguard, and build flavors (#3750) * Add support for AABs, build flavors, and proguard. There are a lot of details to cover here--see the PR for the complete context. * Lint & codeowner fixes. * Fix failures. - Add missing codeowner - Add support for configuring base branch reference - Update CI for dev/alpha AAB builds to use 'develop' since there's no origin configured by default in the workflows * Different attempt to fix bad develop reference in CI. * Initial commit. This is needed to open a PR on GitHub. This commit is being made so that the PR can start off in a broken Actions state. This also initially disables most non-Bazel workflows to make workflow iteration faster and less impacting on other team members. * Introduce infrastructure for batching. This introduces a new mechanism for passing lists of tests to sharded test targets in CI, and hooks it up. No actual sharding is occurring yet. This led to some simplifications in the CI workflow since the script can be more dynamic in computing the full list of targets (which also works around a previous bug with instrumentation tests being run). Java proto lite also needed to be upgraded for the scripts to be able to use it. More testing/documentation needed as this functionality continues to expand. * Add bucketing strategy. This simply partitions bucketed groups of targets into chunks of 10 for each run. Only 3 buckets are currently retained to test sharding in CI before introducing full support. * Fix caching & stabilize builds. Fixes some caching bucket and output bugs. Also, introduces while loop & keep_going to introduce resilience against app test build failures (or just test failures in general). * Increase sharding & add randomization. Also, enable other workflows. Note that CI shouldn't fully pass yet since some documentation and testing needs to be added yet, but this is meant to be a more realistic test of the CI environment before the PR is finished. * Improving partitionin & readability. Adds a human-readable prefix to make the shards look a bit nicer. Also, adds more fine-tuned partitioning to bucket & reduce shard counts to improve overall timing. Will need to be tested in CI. * Add new tests & fix static analysis errors. * Fix script. A newly computed variable wasn't updated to be used in an earlier change. * Fix broken tests & test configuration. Add docstrings for proto. * Fix mistake from earlier commit. * Try 10 max parallel actions instead. See #3757 (comment) for context. * Fix another error from an earlier commit. * Fix mv command so it works on Linux & OSX. Neither 'mv -t' nor piping to mv work on OSX so we needed to find an alternative (in this case just trying to move everything). This actually works a bit better since it's doing a per-file move rather than accommodating for files that shouldn't be moved (which isn't an issue since the destination directory is different than the one containing the AAB file).
|
The issue is reopened because of the following unresolved TODOs: Line 691 in 92ce46b |
|
Ah. We may need to change that TODO or resolve it as part of your PR @anandwana001. |
Broppia
added
issue_type_infrastructure
Impact: Low
6 tasks
BenHenning
added a commit
that referenced
this issue
Aug 19, 2022
…omponent (#4414) ## Explanation Fixes part of #4410, #4300, and #2432. This PR refactors the existing approach for managing application-level build dependencies by splitting up OppiaApplication ApplicationComponent into two: one for the developer build, and one for alpha. The existing OppiaApplication was retrofitted to be a base abstract class for both of the flavor-specific applications, and also for the e2e test TestApplication class (which subsequently simplified that). This has overall resulted in a decrease in complexity since it now allows us to select specific dependencies per flavor of the app which should significantly simplify ongoing release automation work, plus other flavor-specific gating that we may want to do in the future without having to rely on a human to manually enable/disable certain modules during release time. A few other small things of note: - ApplicationContext was removed since it seems to be barely used, and is confusing compared to the regular Context binding that's used almost everywhere - This PR has led to the app module application package having its own Bazel BUILD file (which is a nice help toward continued modularization work) - Most of this PR is tested by observing that the corresponding builds still work as expected, other than the changes to the manifest transformation utility - A bunch of the new classes have been exempted for requiring tests since they (mostly) cannot be effectively tested, or are maintaining the status quo for testing these classes ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only N/A -- This is an infrastructure-only change. Commits: * Create dedicated alpha application component. This simplifies application component management significantly and allows individual build flavors to have their own unique module lists. * Fix broken test per earlier changes. * Update TransformAndroidManifestTest.kt Correct typos.
BenHenning
changed the title
Merged
6 tasks
BenHenning
added a commit
that referenced
this issue
Sep 8, 2022
…s and build flavors (#4417) ## Explanation Fixes #4300 Fixes #2432 This PR introduces support for showing notices when users change from certain flavors of the app to the beta and/or GA (general availability) flavors (both of which are newly introduced in this PR). The user behavior is as follows: - Changing from any flavor _to_ beta will result in the beta notice showing for the user the *first* time they open the app. - Changing from alpha or beta flavors _to_ the GA flavor will result in the GA notice being shown. - The user will always see the notice after the first open following one of these transitions, and in no other cases (including the first open for beta). - The user will not see one or both notices if they select the "never show again" checkbox(es) prior to dismissing the notice(s). - The app deprecation notice will always take logical precedence over other notices (i.e. it will always show in cases when the app is deprecated regardless of flavor transitions). In addition to the above, text was added to the splash screen for the developer, alpha, and beta flavors of the app to make it a bit more explicit to the user. The alpha and beta splash experiences have been configured to wait 2 seconds so that the user has a moment to see the actual text before the app finishes loading. Note also that existing alpha users will not see the beta notice after installing the first beta version (since no flavor has been recorded). While this is technically fixable, it doesn't seem worthwhile given the current small user audience that would be affected. Subsequent transitions from alpha to beta will work as expected (and lead to the beta notice being shown). As a side note: #2432 is fixed as of this PR since the final planned build flavors are being introduced here. While more flavors may be added in the future, it seems sufficient to consider "build flavors are introduced" to be done now that there are developer, alpha, beta, and GA flavors of the app. Technical things of note: - The splash screen now has a contingency fallback of 5 seconds in case the startup state doesn't ever load/the app gets stuck (it will fall back to a default state). - The transitions required a new BuildFlavor proto to be included and bound to Dagger. For tests and the e2e test application component, a new testing-specific flavor has been introduced (though it's never expected to correspond to a distributable build of the app). - The existing 'deprecation' package under the app module has been repurposed to be the location for all notices in the app (including the two new ones). We'll likely add more here in the future, as well (such as the GA deprecation notices). - The new beta and GA flavors of the app have been added to the build_tests CI workflow--you can see the artifacts of the workflow in this PR or on develop after the PR is merged for downloads to the corresponding built AABs. - The Kenya-specific alpha version of the app has the same 'alpha' build flavor as the default alpha version of the app since there doesn't seem to be any reason why they should differ here. - Some places were updated to use Application instead of Context to make things a bit tighter in expectation for *what* type of context is available to use (for cases when casting to injector providers is used). This helps to avoid a particularly challenging bug that I ran into during test development wherein the wrong context was being used after creating a new application (since that application's application context was pointing to the original application). This means that ``Application.getApplicationContext()`` may actually not return itself in instrumentation cases. The changes help avoid this leading to actual issues in tests (but they should have no impact on the app's behavior in production environments). - PersistentCacheStore had a few significant changes to better facilitate the needs of this PR's domain changes: - Changes in the store are only published to its in-memory cache (and subsequently notified to subscribers of the store) if the cache store actually changes. - The store's existing two priming functions were combined into a single new one with more flexibility as it can be used to tweak both changing the on-disk cache and publishing the in-memory one. The documentation around priming has also been significantly improved in terms of detail as I now think it's a legitimate use case that we'll want to use in more places in the future. - InMemoryBlockingCache was updated to include publishing before-and-after values to its registered observer callback to avoid over-notifying for alterations to the cache that don't logically change it (this can actually lead to subtle differences in behavior that may not always be expected, so the change should improve robustness of the cache). Details regarding test changes & exemptions: - SplashActivityTest and AppStartupStateControllerTest were updated to enable parameterized testing since they have to test many different cases of build flavor transitions. - The only test exemptions were: renames (deprecation notice classes), files that can't be tested (applications, application components, and listeners), and files that are not conventionally tested (test activities and fragment presenters). - A new BUILD file was added for PersistentCacheStoreTest (which made development a bit easier when verifying some of the changes that ended up breaking the test suite). - One of SplashActivityTest's error case tests were removed since it now seems impossible to trigger a situation that could actually cause it. Similarly, the pending case with 5-second timeout also seems impossible to test, though it's theoretically possible to occur if something catastrophically goes wrong in a production environment with a real clock. ## Essential Checklist - [x] The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".) - [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation. - [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide). - [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)). - [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop". - [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)). ## For UI-specific PRs only This PR absolutely should include screenshots & a demo video, but for the sake of optimizing time this will be done post-merge. Commits: * Create dedicated alpha application component. This simplifies application component management significantly and allows individual build flavors to have their own unique module lists. * Add beta & GA update notices. This also introduces dedicated beta & GA build flavors which is a necessary prerequisite. It also introduces an extra beta, alpha, and dev mode labels for the splash screen (the latter 2 were extra) with 2 second minimum wait timers for beta and alpha to ensure they are seen. A 5-second safety timer was added to ensure the splash screen can always be passed even if something goes wrong at the domain level (since there are now quite a few moving pieces to determine the user's current onboarding state). * Add build tests for the new beta & GA flavors. * Fix broken test per earlier changes. * Fix general broken tests & builds. Tests broken due to changes to the app startup experience haven't yet been fixed. * Lint fixes. * First part of adding tests for GA notices. There's a bunch left to do here, this is mainly needed so that I can transfer changes to a different machine. * Update TransformAndroidManifestTest.kt Correct typos. * Fix tests & static checks. This also removes temporary debug code and TODOs, and finishes the tests for SplashActivity. * Post-merge fixes. * Test fixes. * Fix Gradle test. * Follow-up adjustments after self-review.
BenHenning
added
the
Z-ibt
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
While we don't yet have compile-time feature gating, we should be introducing different flavors of the app in Bazel builds. Specifically, we should have:
Longer-term, we will likely want alpha/beta/ga versions of the prod build. Ideally, these would also be Android app bundles rather than APKs, but that's blocked on bazelbuild/bazel#11497.
The text was updated successfully, but these errors were encountered: