Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade Android dependencies #820

Merged
merged 7 commits into from
Jul 21, 2023
Merged

Upgrade Android dependencies #820

merged 7 commits into from
Jul 21, 2023

Conversation

HollowMan6
Copy link
Collaborator

@HollowMan6 HollowMan6 commented Jul 7, 2023

This PR upgrades Android dependencies as newest as possible. The compileSdkVersion is now 33 because of the dependency needs. Some of the dependencies are not upgraded to the latest due to the following build errors (WARNING: I still haven’t got my VR device, so I didn’t tried to test the functionality. I can only ensure that it builds successfully):

We are also not ready to land on android_gradle_plugin 8 because some of the deprecated method used in com.huawei.agconnect:agcp.

Also I fixed the CMake version to 3.10.2 because of the nature of the CMakeList.txt in project.

@HollowMan6 HollowMan6 marked this pull request as draft July 7, 2023 06:47
@HollowMan6
Copy link
Collaborator Author

Looks like there are some strange errors when building SDKs (noAPI should be fine) https://github.com/Igalia/wolvic/actions/runs/5483543167/jobs/9989998898?pr=820

@HollowMan6
Copy link
Collaborator Author

HvrArm64Gecko build is OK on my computer, here the error is ERROR:R8: java.lang.OutOfMemoryError: Java heap space.

@HollowMan6
Copy link
Collaborator Author

Get it fixed by increasing the Java maximum size of the memory allocation pool -Xmx to 4g. Now this PR is ready for review.

Copy link
Member

@svillar svillar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the most amazing changesets I've reviewed lately. Keep up the good job! I'm actually quite surprised that everything builds fine after those changes.

We should be supercareful and enhance QA anyway, because there might be breaking changes/regressions.

I just have some minor nits

.github/workflows/ci.yml Show resolved Hide resolved
gradle.properties Show resolved Hide resolved
versions.gradle Show resolved Hide resolved
versions.gradle Show resolved Hide resolved
versions.gradle Show resolved Hide resolved
@HollowMan6 HollowMan6 force-pushed the deprecated branch 3 times, most recently from fbe3ec0 to 21f187d Compare July 11, 2023 19:41
@HollowMan6 HollowMan6 marked this pull request as draft July 12, 2023 15:08
@HollowMan6 HollowMan6 force-pushed the deprecated branch 2 times, most recently from 8819496 to 046b6ba Compare July 12, 2023 17:48
@HollowMan6
Copy link
Collaborator Author

The upgrade is harder than I previously anticipated. Looks like a lot of things have been broken with this PR (bookmarks, history, extensions ...) Will try to fix those as well. This needs more time.

@HollowMan6 HollowMan6 force-pushed the deprecated branch 2 times, most recently from 79f1ada to 80b4d89 Compare July 13, 2023 18:19
@HollowMan6 HollowMan6 marked this pull request as ready for review July 13, 2023 18:19
@HollowMan6
Copy link
Collaborator Author

Looks like many breaking changes were made during Gecko/Android Component 105->106 upgrades, as I can fix those bookmarks, history, and extensions ... breaks by downgrading to 105

@HollowMan6 HollowMan6 force-pushed the deprecated branch 3 times, most recently from 92d4dec to b6529aa Compare July 14, 2023 06:11
@HollowMan6 HollowMan6 requested a review from svillar July 14, 2023 06:11
@svillar
Copy link
Member

svillar commented Jul 15, 2023

The upgrade is harder than I previously anticipated. Looks like a lot of things have been broken with this PR (bookmarks, history, extensions ...) Will try to fix those as well. This needs more time.

Ah OK, that makes more sense :). We tried this upgrade in the past and it was not trivial at all. That's why I was amazed because I thought the upgrade didn't bring regressions. Yeah, let's go step by step as we are not in a hurry

@HollowMan6
Copy link
Collaborator Author

I think we are now ready. Looks like the upgrade doesn't bring regressions now.

Ah OK, that makes more sense :). We tried this upgrade in the past and it was not trivial at all. That's why I was amazed because I thought the upgrade didn't bring regressions. Yeah, let's go step by step as we are not in a hurry

Copy link
Member

@svillar svillar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. I just have a few nits here and there.

app/src/noapi/java/com/igalia/wolvic/PlatformActivity.java Outdated Show resolved Hide resolved
versions.gradle Outdated Show resolved Hide resolved
versions.gradle Show resolved Hide resolved
@HollowMan6 HollowMan6 force-pushed the deprecated branch 2 times, most recently from 9b426de to 533a40f Compare July 20, 2023 21:34
@HollowMan6 HollowMan6 requested a review from svillar July 20, 2023 21:45
- Android Component to 105.0.8
- We are not ready to land on android_gradle_plugin
  8 because some of the deprecated method used in
  com.huawei.agconnect:agcp.
- Increase the Java maximum size of the memory
  allocation pool to 4g, so that HvrArm64Gecko build
  failure with ERROR:R8: java.lang.OutOfMemoryError:
  Java heap space in CI can be fixed
- Use JDK 17 and Gradle 7.5 in CI

Signed-off-by: Songlin Jiang <sjiang@igalia.com>
Signed-off-by: Songlin Jiang <sjiang@igalia.com>
So that we can focus on specific API failures in CI

Signed-off-by: Songlin Jiang <sjiang@igalia.com>
Signed-off-by: Songlin Jiang <sjiang@igalia.com>
- Fragment in android.app was deprecated in API level 28:
  Use the Jetpack Fragment Library Fragment for consistent behavior
  across all devices and access to Lifecycle.
- FragmentTransaction in android.app was deprecated in API level 28.
  Use the Support Library FragmentTransaction.
- getFragmentManager() has been removed in favor of
  getParentFragmentManager() which throws an IllegalStateException
  if the FragmentManager is null.
- BaseBundle.get was deprecated in API level 33. Use the type-safe
  specific APIs depending on the type of the item to be retrieved

Signed-off-by: Songlin Jiang <sjiang@igalia.com>
Partially fixes #802

Signed-off-by: Songlin Jiang <sjiang@igalia.com>
Signed-off-by: Songlin Jiang <sjiang@igalia.com>
Copy link
Member

@svillar svillar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a big leap. Thanks so much

@svillar svillar merged commit cf3a504 into main Jul 21, 2023
7 checks passed
@svillar svillar deleted the deprecated branch July 21, 2023 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants