Skip to content

Conversation

@xihan123
Copy link

This pull request includes several changes to improve the functionality and stability of the LSPatch application. The most important changes include modifications to the AndroidManifest.xml file, updates to the LSPApplication and Patcher classes, and enhancements to the NewPatchScreen and AppManagePage user interfaces.

Permissions and Providers:

  • Added the REQUEST_INSTALL_PACKAGES permission to the AndroidManifest.xml file.
  • Added a FileProvider to the AndroidManifest.xml to support file sharing.

Application and Patching Logic:

  • Introduced a new targetApkFile property in LSPApplication to handle APK files.
  • Updated the Patcher class to copy the APK to the external cache directory and assign it to targetApkFile.

User Interface Enhancements:

  • Modified NewPatchScreen to include a new InstallDialog2 for handling APK installations with additional uninstall logic.
  • Added functions to Utils.kt to check if an APK is fixed by LSP, install APKs, and uninstall APKs by package name.

Clean-Up and Utility Functions:

  • Added a method to clean the external temporary APK directory in LSPPackageManager.
  • Created a new XML file file_paths.xml to define paths for file sharing.

@JingMatrix
Copy link
Owner

I will begin reviewing when I finish my PhD thesis (soon), please feel free to improve current PR.

@JingMatrix
Copy link
Owner

For the local mode, i.e., when useManager = False, you choose to embed the loader.
I am not sure whether this is a good design, I may thus choose to skip the third commit.

Do you find any advantage of doing so?

@xihan123
Copy link
Author

xihan123 commented Aug 3, 2025

This can be convenient to a certain extent. If there are no changes for a long time, the user experience will be better. However, many users are now reporting that this causes the program to crash.

According to the log, the crash is caused by the core not being loaded.

@JingMatrix
Copy link
Owner

JingMatrix commented Aug 3, 2025

I see, I'd prefer to see the user bug reports. We can keep it as another possible pull-request in the future. You may keep it in another branch of your repo.

Let us focus on the native installation. Currently, most installed application from play store are using split APK delivery.
However, the simple Intent method is no longer sufficient to handle multiple APKs.

It is recommended to use PackageInstaller for split APKs. Could you please improve / check this point?
To test it out, you can try patching Spotify installed from Google Play as an example.

@xihan123
Copy link
Author

xihan123 commented Aug 3, 2025

use PackageInstaller for split APKs?

@JingMatrix
Copy link
Owner

I meant, currently, only PackageInstaller can handle split APKs installation.
You need to create a PackageInstaller session and so on.

@xihan123
Copy link
Author

xihan123 commented Aug 3, 2025

I don't understand

@JingMatrix
Copy link
Owner

Okay, here is what I mean.

Problem in you code

You code cannot handle split APK installation, which thus fails to patch most applications installed from Google Play store, such as Spotify. Note that in this case, LSPatch modfies each split APK, and these modified APKs need to be installed at the same time.

My suggestion to solve this

I suggest you to read the official doc of the PackageInstaller API, since this is the official way to handle monolithic APK and split APKs. After reading its doc or maybe some other resources, you should be able to finally solve this problem.

Futher details of this API

This API has a concept of session, by handling all split APKs during a created session, you will manage to correctly install them into the device.

@xihan123
Copy link
Author

xihan123 commented Aug 3, 2025

OK, now I understand. I will take a look at it when I have time.

@JingMatrix
Copy link
Owner

JingMatrix commented Aug 7, 2025

Is the code written by AI, and you were talking with it in Chinese?

@xihan123
Copy link
Author

xihan123 commented Aug 8, 2025

Is the code written by AI, and you were talking with it in Chinese?

The annotations were written by the AI, while the overall logic was completed by myself.

@JingMatrix
Copy link
Owner

AI generated code are really low, you should double check.
At least, please log in English, and use the TAG properly. No stupid comments from AI.

@xihan123
Copy link
Author

xihan123 commented Aug 9, 2025

Recently, I have been in the habit of using AI to generate comments to organize the logic after writing the code. However, when submitting, I forgot to delete them.

val copyError = stringResource(R.string.copy_error)
var installing by remember { mutableStateOf(false) }
if (installing) InstallDialog(viewModel.patchApp) { status, message ->
var installing by remember { mutableStateOf(0) }
Copy link
Owner

@JingMatrix JingMatrix Aug 9, 2025

Choose a reason for hiding this comment

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

It would be better, if you use an enum to define installing an rename it to installation.

@JingMatrix
Copy link
Owner

Please rebase on the master branch, remove the two commits, 8dba962 and feea434.

You can keep them in another branch of your repo.

@xihan123 xihan123 closed this by deleting the head repository Aug 9, 2025
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.

[Feature Request] Using package installer when shizuku is not connected

2 participants