Magisk-Modules-Alt-Repo / submission

Information regarding the submission of Magisk Modules to the Alt-Repo.
166 stars 8 forks source link

[Module] Compatible Magisk-mirroring #223

Closed yzyhk904 closed 6 months ago

yzyhk904 commented 6 months ago

ID

compatible-magisk-mirroring

Name

Compatible Magisk-mirroring

Description

A Magisk module providing the compatible Magisk-mirroring for installing pure Magisk modules on incompatible Magisk variants and KernelSU.

Module repository link

https://github.com/yzyhk904/compatible-magisk-mirroring

Source code link (for compiled binaries/APKs)

No response

yzyhk904 commented 6 months ago

Sorry. I found a serious bug. I stop the release of this module now. This provides modified system files now (not unmodified ones).

yzyhk904 commented 6 months ago

I fixed the serious bug. Now, this module provides unmodified original system files in a mounted state for installing magisk modules on incompatible Magisk variants. (added "mount --make-private")

I'm sorry for the inconvenience.

Atrate commented 6 months ago

Could you explain more in detail what your module does?

yzyhk904 commented 6 months ago

A typical Magisk module modifying an existing system file on "/vendor/etc", e.g., "/vendor/etc/audio_policy_configuration.xml", needs its original file as intendedly for generating a modified one by editing some parts of the file, and putting the modified one in "${MODPATH}/system/vendor/etc".

Because system files on "/vendor" might be modified already by other Magisk modules and even by the module itself (already installed) when installing or updating the module in the Magisk manager, the module (usually "customize.sh") possibly fails to install or installs wrongly in silence by the interference. For avoiding this situation, experienced developers of a Magisk module use mirrored system files under "$(magisk --path)/.magisk/mirror" that are unmodified original. However some recent Magisk variants (e.g., Magsik alpha, Kitsune Magisk, and KernelSU) don't provide this mirrored system files for developers.

Compatible Magisk-mirroring puts a tiny script in "/data/adb/post-fs-data.d" and patches and forces the Magisk variants to provide the mirrored system files when Magisk module installation and execution , typically for "customize.sh".

Without this module, developers must move some code in "customize.sh" to "post-fs-data.sh" (for handling almost unmodified original ones) involuntarily for supporting such variants. It's very unfortunate, I think.

HuskyDG commented 6 months ago

I think module creator should mount mirrors by theirself while installation if they need original partitions

yzyhk904 commented 6 months ago

How do you justify your opinion? It isn't compatible with Official Magisk's. There isn't any technical difficulty for the compatibility!

And how do you mount mirrors in "customize.sh"? If you mount mirrors in it as usual, the mirrors provide modified system files (not unmodified original ones). System partitions are already mounted in a shared mode, so you must change the mode of them to be private at first. But it is dangerous and critical to recommend this way for module developers.

Atrate commented 6 months ago

I think @yzyhk904 has a point, and this module could be a template for developers on how to do it right.

One question though, is the chmod 777 strictly necessary here?

https://github.com/yzyhk904/compatible-magisk-mirroring/blob/4d7391b60ffd50d703eb886faa0b9f30be28bfa2/customize.sh#L23

yzyhk904 commented 6 months ago

It's a mistake. It should be "chmod 755" (fixed just now). Because there isn't "post-fs-data.d" folder as default on KernelSU, I only need the folder as KernelSU would run scripts in it.

Atrate commented 6 months ago

The module seems okay now, just one more thing.

https://github.com/yzyhk904/compatible-magisk-mirroring/blob/16df1abeca4e0babf4984e29fc21c34aa28f5fed/uninstall.sh#L28

Every time I see a rm -rf with a variable, I get reminded of this beauty of a Steam bug: https://www.youtube.com/watch?v=qzZLvw2AdvM

Could you add a sanity check to that command so that it for sure does not try to remove anything it's not supposed to? I get that with the current code it will work fine, but someday the code may change, there may be a bug with MagiskMirror not being set and boom.

yzyhk904 commented 6 months ago

I changed such code in the module so that more sanity checks are used,

Atrate commented 6 months ago

Approved.

I have created a repository for you at: https://github.com/Magisk-Modules-Alt-Repo/compatible-magisk-mirroring , enjoy!

Check your e-mail inbox for an invite to your new repository and remember to 'Watch' your repository on Github to have new issues in it show up in your feed.

Make sure to change your local repository's push URLs or configure 2 parallel push URLs.