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

Magisk detection and working with systemless ACC #58

Closed
s-kk opened this issue Aug 12, 2019 · 10 comments
Closed

Magisk detection and working with systemless ACC #58

s-kk opened this issue Aug 12, 2019 · 10 comments
Labels
acc/kernel https://github.com/VR-25/acc/issues/ discussion enhancement New feature or request

Comments

@s-kk
Copy link

s-kk commented Aug 12, 2019

Dear developer!
I love ACC and ACCA. But here's a small problem. In previous versions, ACCA frontend works perfectly with systemless ACC backend. But after recent updates, ACCA no longer works with systemless ACC backend.
New ACCA seems to uninstalling systemless ACC and replace it with integrated ACC, and because of which, ACCA is altering the originally RO partition.

And my suggestion is provide support for systemless ACC backend or maybe give an option in ACCA settings to switch between systemless or integrated version.

Thanks!

@MatteCarra
Copy link
Owner

Thanks for the suggestion.
I want to make it clear that the current installation system of the integrated acc is made by vr-25, the creator of acc.

An integrated version of acc has been adopted for the following reasons:

  • it ensures that acca is compatible with acc
  • it ensures that acc is installed when using acca
  • it allows to automatically uninstall acc when acca is uninstalled. This particularly important because we want to publish this app on F-Droid and casual users wouldn't like a two step uninstall process.

What would be the advantages of a decoupled systemless installation of acc for you?
I could add an option if you need it.

@MatteCarra MatteCarra added the enhancement New feature or request label Aug 12, 2019
@s-kk
Copy link
Author

s-kk commented Aug 12, 2019

Thanks for your reply.
So here are some advantages of separate installation.

  • Magisk has its own update detection for uploaded modules.
  • If ACC is somehow wrongly scripted in an update and may cause harm to device, turning Magisk off will be an ideal choice to stop ACC failure instead of leaving wrong code in device.
  • The change on /data partition is handled by Android OS and since we're using Magisk, we should leave it untouched and perform the change in Magisk way, which is by installing modules. And that way, we could avoid potential problems. I've digged into the integrated ACC and found the script set with root ownership in /data and that does not really look good.

And also, suggestions.

  • Could you please implement a Magisk detection on the start of MainActivity? For example, a popup showing Magisk is installed and offering choice if you would like to proceed to installing Magisk version of ACC or just install integrated one.
  • In terms of version issues, if Magisk version of ACC detected, ACCA can check the version of currently installed ACC and see if it's compatible with ACCA. If not, display an alert.

That's all.
Thanks again for your effort!

@s-kk
Copy link
Author

s-kk commented Aug 12, 2019

And in terms of ACC upstream, I could propose an issue or make this issue as an reference on ACC project page.

@MatteCarra
Copy link
Owner

Yes, post it there: https://github.com/VR-25/acc/issues

@MatteCarra MatteCarra self-assigned this Aug 13, 2019
@VR-25
Copy link
Collaborator

VR-25 commented Aug 14, 2019

ACCA is altering the originally RO partition

@s-kk, that's unheard of.
What partition are you referring to?

Note that ACC is still systemless - whether or not it's integrated into AccA.

For updates, there's the command acc --upgrade - which in conjunction with the scheduling features being implemented into AccA - is way more efficient than Magisk's module update mechanism.

The integrated ACC is always safer than its Magisk module counterpart - especially in regards to bootloops. It's easier to uninstall, too.

The only potential issue with integrating ACC into AccA is that advanced security restrictions may forbid the placement of files with elevated privileges (root) in /data/data/mattecarra.accapp/files/acc/. This is under investigation.
VR-25/acc#12

@VR-25 VR-25 added acc/kernel https://github.com/VR-25/acc/issues/ discussion labels Aug 14, 2019
@MatteCarra MatteCarra removed their assignment Aug 14, 2019
@s-kk
Copy link
Author

s-kk commented Aug 14, 2019

@VR-25 Thanks for reply.

The only potential issue with integrating ACC into AccA is that advanced security restrictions may forbid the placement of files with elevated privileges (root) in /data/data/mattecarra.accapp/files/acc/. This is under investigation.

Yea this is one of the concerns I'm talking about.

ACCA is altering the originally RO partition.

Sorry, this expression is not really accurate. I mean that files on /data are handled by Android OS, and placing files with elevated owner and permissions may cause potential issues.

And I still insist on providing a choice for users.

@VR-25
Copy link
Collaborator

VR-25 commented Aug 14, 2019

@s-kk, the elevated privileges concern is out of the way now. All ACC data in /data/data/mattecarra.accapp/files will be owned by the app, as opposed to root.

As for the other concern, ACC/AccA never touch the root of /data nor any other folder besides the aforementioned and /data/adb/acc-data/.

Lastly, supporting AccA working alongside ACC Magisk module goes against what we already accomplished with the app. A major design philosophy was Magisk independency - and we reached that goal. Now we're aiming at even higher levels. Ultimately, the AccA/ACC combo will have better upgrade mechanisms and be pushed to F-Droid and Play Store.

We appreciate your feedback.
If you can come up with a solid reason to have ACC Magisk module working alongside AccA, the idea will be reconsidered.

Thank you

@VR-25 VR-25 closed this as completed Aug 16, 2019
@ovizii
Copy link

ovizii commented Oct 17, 2019

Just wanted to add that the decoupling of ACCA and Magisk does not seem complete. Magisk is telling me to update ACC but when I try I get told that I should do it via acc --update or via ACCA but the update to ACC keeps still showing up in Magisk.

Could this updatenotification be removed or avoided?

@VR-25
Copy link
Collaborator

VR-25 commented Oct 18, 2019

@ovizii, it is complete.
There was an error on your end.
You're not even supposed to see ACC in Magisk Manager > modules when using AccA.
Run su -c rm -rf /data/adb/modules*/acc/ to remove that zombie module.

Install this ACC version from Magisk Manager, and upgrade to the latest with acc -u dev.

@ovizii
Copy link

ovizii commented Oct 18, 2019

Thanks for the help, all your info seems to have worked and fixed the problem except this command needed to be prefixed with "su" in my case: su acc -u dev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acc/kernel https://github.com/VR-25/acc/issues/ discussion enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants