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

Proper addon.d support for the /vendor and /product partitions for A/B devices #872

Open
nezorflame opened this issue Aug 20, 2020 · 10 comments · Fixed by #879
Open

Proper addon.d support for the /vendor and /product partitions for A/B devices #872

nezorflame opened this issue Aug 20, 2020 · 10 comments · Fixed by #879

Comments

@nezorflame
Copy link
Contributor

nezorflame commented Aug 20, 2020

Thanks to @merothh users who have custom /product partition are now able to properly remove stock apps during the installation process: #868

The addon.d script though requires more work and stil needs to be fixed. Let's track this issue here.

@osm0sis
Copy link
Member

osm0sis commented Aug 20, 2020

/vendor will also need some consideration.. for addon.d (v1) they can be mounted to /vendor and /product, but for addon.d-v2 it'll need to be /postinstall/vendor and /postinstall/product, but only if e.g. /postinstall/product is a directory and /postinstall/system/product is a symlink.

Then the normal addon.d lists should work fine, since they're all based in /system, and things are symlinked.

Main problem is I'm not sure that's possible when booted without root. That'll require some investigation.

@nezorflame nezorflame changed the title addon.d support for the custom /product partition proper addon.d support for the /vendor and /product partitions Aug 20, 2020
@nezorflame nezorflame changed the title proper addon.d support for the /vendor and /product partitions Proper addon.d support for the /vendor and /product partitions Aug 20, 2020
@merothh
Copy link
Contributor

merothh commented Aug 27, 2020

merothh@334628a

I have just verified the above commit to work on an A only dynamic device. If it's looks fine, i shall extend the mount support in the function for vendor as well, and verify it on my A only non dynamic device too.

If so, we could gurantee proper addon functionality on A devices and leave A/B as pending.

Looking forward to you guys opinion on this.

@osm0sis
Copy link
Member

osm0sis commented Aug 27, 2020

Nice work!!

A couple minor notes:

  1. Please make the mount_product function mount_generic so that we can just supply the word product or vendor and it'll do the setup.
  2. It will need to mount to /postinstall/product or /postinstall/vendor for dynamic and/or addon.d-v2.

If you could fix those up and submit as PR I think we're good! 🙂

@merothh
Copy link
Contributor

merothh commented Aug 27, 2020

Nice work!!

A couple minor notes:

  1. Please make the mount_product function mount_generic so that we can just supply the word product or vendor and it'll do the setup.
  2. It will need to mount to /postinstall/product or /postinstall/vendor for dynamic and/or addon.d-v2.

If you could fix those up and submit as PR I think we're good! 🙂

Awesome.

As for:

1: That's what I wanted to do. Just wanted to see if this initial approach was feasible. I'll do it as you suggested.

2: I thought I'd leave that since I don't have an A/B device to test on. Hence I just left checks to skip it on A/B.

I'll see to both and open a PR then.

@osm0sis
Copy link
Member

osm0sis commented Aug 28, 2020

Hmm there might still be a complication with A/B.. I think addon.d using $SYS or /postinstall/system as a base will cause issues, since the /system/vendor and /system/product symlinks aren't relative.. so would point to actual /vendor and /product, and mounting over those would be kinda haphazard to do in a global mount namespace on a booted device.. 😕

@merothh
Copy link
Contributor

merothh commented Aug 28, 2020

Hmm there might still be a complication with A/B.. I think addon.d using $SYS or /postinstall/system as a base will cause issues, since the /system/vendor and /system/product symlinks aren't relative.. so would point to actual /vendor and /product, and mounting over those would be kinda haphazard to do in a global mount namespace on a booted device..

There is also the question of mounts happening while booted, without root (as you mentioned earlier above). Even then /postinstall is ro.

merothh@ed94684

I have made the mount function generic and added vendor to the mounts as well. Tested on A only Dynamic ( Poco X2 ) / A only Non Dynamic ( Redmi Note 7 Pro ). Both have a separate product partition.

However as we dont have a clear methodology figured out for A/B (Dynamic / Non Dynamic), i still left the A/B checks in the mount_generic function, so that it skips the mounts and retains the current behaviour of not mounting product, vendor on A/B.

Keeping that in mind. I would say that the current commit is good to go ?

@nezorflame
Copy link
Contributor Author

I'm OK with fixing only the A devices for now, that's better than nothing anyway and will help some of the users.

@merothh
Copy link
Contributor

merothh commented Sep 5, 2020

We should still be tracking the issue for A/B devices.

@nezorflame nezorflame changed the title Proper addon.d support for the /vendor and /product partitions Proper addon.d support for the /vendor and /product partitions for A/B devices Sep 5, 2020
@nezorflame nezorflame reopened this Sep 5, 2020
@osm0sis
Copy link
Member

osm0sis commented Jan 13, 2021

Seems like this could be resolved by addon.d-v3 once it's merged and widely rolls out to other ROMs: https://review.lineageos.org/c/LineageOS/android_vendor_lineage/+/299059

@DrymarchonShaun
Copy link

DrymarchonShaun commented Apr 6, 2022

Has there been any update to this? I'm sitting here with a oneplus 8t that has a 500MB system partition, and a 450MB product partition that opengapps doesn't seem to be using, which sucks because it mean i can install half as many apps, essentially. In fact, looking at it, it seems flashing the biggest package i could (modified stock) filled up system so it has 16MB left, and it deleted the stock apps that were in product, so now it has 719MB that i can't use.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants