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

Install script uses wrong device path for A/B device, silently succeeds #5

Closed
infinity0 opened this issue Nov 17, 2022 · 18 comments · Fixed by #7
Closed

Install script uses wrong device path for A/B device, silently succeeds #5

infinity0 opened this issue Nov 17, 2022 · 18 comments · Fixed by #7
Assignees

Comments

@infinity0
Copy link
Contributor

Device: OnePlus 8T (KB2003), LineageOS 19.1 with MicroG, same LineageOS recovery.

getprop ro.treble.enabled shows true.

Androidacy/WebviewManager-Module does work for me.


Firstly you want to set -e at the top of your install script, so it actually fails if one of the steps fails.

Then to my actual problem: these lines don't work:

mount -o rw /dev/block/by-name/system /picodroid_system
mount -o rw /dev/block/by-name/vendor /picodroid_vendor

On my system, they should instead be:

mount -o rw /dev/block/mapper/system_a /picodroid_system
mount -o rw /dev/block/mapper/vendor_a /picodroid_vendor

You'd also need to write logic to detect what the active slot is (either A or B). On my recovery ROM at least, {system_b,vendor_b} doesn't exist when the active slot is A. I have no idea if this holds true in the general case.

Then there is a further problem, which is that cp creates empty files on both /system and /vendor for me, showing no error message nor failure exit code. This is the case even if I boot into my normal system, mount read-write, and try it. It is possible to create files in those paths, but not to put content in those empty files. I haven't figured out how to fix this yet...

@arovlad arovlad self-assigned this Nov 17, 2022
@arovlad
Copy link
Owner

arovlad commented Nov 17, 2022

Thank you for your thorough description. I wrote the update binary while only targeting LineageOS and only testing it on my device, so it's no surprise that there are some edge cases where it fails. I will research a solution and I will update the script accordingly.

@infinity0
Copy link
Contributor Author

infinity0 commented Nov 17, 2022

A further observation: /dev/block/mapper doesn't exist when booting into recovery, but exists by the time I have finished running adb sideload and update-binary. (Unclear if it's available during the running of update-binary; adb shell is disabled during the sideload process.) Alternatively, one can make it exist before running adb sideload, by selecting Advanced -> mount/umount system which mounts /dev/block/mapper/system_[ab] on /mnt/system.

My device also has *-cow partitions. This suggests it might be using Virtual A/B which might be the reason why /system and /vendor appear as "disk full" (which is why I can't write new files to them, even after mounting rw). I haven't been able to figure out yet from the documentation how to actually write to them, it's a complex stack of virtual devices, and the services it mentions for accessing them aren't available during recovery. :(

@infinity0
Copy link
Contributor Author

(Magisk itself installs fine via adb sideload, so you might be able to re-use whatever technique it uses.)

@PatrykMis
Copy link

Same problem with OnePlus 6, LineageOS v19.1.

@infinity0
Copy link
Contributor Author

Magisk does this:

https://github.com/topjohnwu/Magisk/blob/master/scripts/util_functions.sh
https://github.com/topjohnwu/Magisk/blob/master/scripts/flash_script.sh

Specifically

I will have a play with it manually tomorrow.

@infinity0
Copy link
Contributor Author

It turns out my own /system/addon.d/99-magisk.sh is in fact empty, so it seems the Magisk installer itself is not working entirely 100%. The mystery deepens...

@infinity0
Copy link
Contributor Author

infinity0 commented Nov 19, 2022

@PatrykMis can you check if your own /system/addon.d/99-magisk.sh is empty or not? And are you using MicroG or not?

@infinity0
Copy link
Contributor Author

LineageOS devs tell me the reason is because MicroG sets WITH_GMS = true leading to lack of extra space in the system partition, it means no installer (Magisk or MindTheGapps or anything else) will be able to write extra stuff to /system or /vendor or anything, so this is not a problem with your script. I'll follow this issue up separately with them later.

The other stuff I mentioned still applies though, about slot detection and the right device. The MindTheGapps install script is a bit simpler than Magisk's so perhaps you could use that instead:

https://gitlab.com/MindTheGapps/vendor_gapps/-/blob/tau/build/meta/com/google/android/update-binary#L169

@PatrykMis
Copy link

PatrykMis commented Nov 19, 2022

Not nessesarily. On Redmi 7 (Onclite), Lineage 19.1 with MicroG it works well; but this model doesn't have A/B partitioning scheme.

@arovlad
Copy link
Owner

arovlad commented Nov 26, 2022

I updated the installation script to detect the partitions programmatically by searching the /dev/block. @infinity0 @PatrykMis Can you test this on your devices (by unpacking the .zip first because of #39)?

@PatrykMis
Copy link

PatrykMis commented Nov 28, 2022

Unfortunately, it does not work too, but in my case the problem might be somewhere else. I've attached full recovery.log

Output of ls in /dev/block:


OnePlus6:/dev/block # ls
bootdevice  loop12  loop4  platform  ram13  ram5  sda1   sda15  sda5  sdb1  sdd1   sde11  sde17  sde22  sde28  sde33  sde39  sde44  sde5   sde55  sde60  sde66  sde71  sdf2
by-name     loop13  loop5  ram0      ram14  ram6  sda10  sda16  sda6  sdb2  sdd2   sde12  sde18  sde23  sde29  sde34  sde4   sde45  sde50  sde56  sde61  sde67  sde72  sdf3
loop0       loop14  loop6  ram1      ram15  ram7  sda11  sda17  sda7  sdc   sdd3   sde13  sde19  sde24  sde3   sde35  sde40  sde46  sde51  sde57  sde62  sde68  sde8   sdf4
loop1       loop15  loop7  ram10     ram2   ram8  sda12  sda2   sda8  sdc1  sde    sde14  sde2   sde25  sde30  sde36  sde41  sde47  sde52  sde58  sde63  sde69  sde9   sdf5
loop10      loop2   loop8  ram11     ram3   ram9  sda13  sda3   sda9  sdc2  sde1   sde15  sde20  sde26  sde31  sde37  sde42  sde48  sde53  sde59  sde64  sde7   sdf    volmgr
loop11      loop3   loop9  ram12     ram4   sda   sda14  sda4   sdb   sdd   sde10  sde16  sde21  sde27  sde32  sde38  sde43  sde49  sde54  sde6   sde65  sde70  sdf1   zram0

Looking at the recovery log, the only mention of system partition is here: /dev/block/bootdevice/by-name/system_a.

@infinity0
Copy link
Contributor Author

Looking at the recovery log, the only mention of system partition is here: /dev/block/bootdevice/by-name/system_a.

[ 0.003874] 1 / ext4 /dev/block/bootdevice/by-name/system_a 0

Not sure, but I think this is just the system partition for the recovery OS not your actual main system OS.

As I mentioned in my second post, the OS system devices don't exist until certain setup things have been run. I don't know the specific details of this, but I'd imagine copying whatever MindTheGapps does will work.

@arovlad
Copy link
Owner

arovlad commented Nov 29, 2022

@PatrykMis The recovery log seems to indicate that the script fails because of the missing addon.d folder (line 545), not because the partition is not detected correctly. This is reinforced by the fact that your active partition slot is A (line 304) which is the same as the one in the filesystem table (line 4). Could you please check if the folder exists on your system? If you have LineageOS installed it should exist.

@arovlad
Copy link
Owner

arovlad commented Nov 29, 2022

Not sure, but I think this is just the system partition for the recovery OS not your actual main system OS.

@infinity0 You might be right, but the update script you referenced also seems to install everything on the active slot.

@infinity0
Copy link
Contributor Author

@arovlad I was just guessing about the failure without looking at the details, but your explanation of the failure due to addons.d is likely the real explanation.

I don't think your current strategy is the best though. find can return things in a different order, e.g. put system-ext before system, and later versions of Android might have other names, so someone that naively uses this 5-6 years from now (after you stop paying attention to this repo) might do something bad to their system. IMO you should just copy the 10-20 lines of shell that MindTheGapps uses, they test on a lot of devices and a lot of thought has gone into those 10-20 lines.

@arovlad
Copy link
Owner

arovlad commented Dec 4, 2022

find can return things in a different order, e.g. put system-ext before system

This is a valid point and I will look into potential fixes but there's hardly anything that can be done besides blacklisting a few names or mounting every partition that we find and checking for the required folders. This is a case where we cannot escape hard-coding a string.

later versions of Android might have other names

It is highly unlikely that future versions of Android will ever rename the system partition to something else. Even if it were to happen, the partition will probably be symlinked to the new name so there will be no issues whatsoever.

someone that naively uses this 5-6 years from now (after you stop paying attention to this repo) might do something bad to their system.

All pieces of software will eventually become obsolete at some point if they fall out of use or are not properly maintained, so this is hardly an argument. Even so, if the system partition will hypothetically be replaced with something else the script will simply fail as it cannot find it.

IMO you should just copy the 10-20 lines of shell that MindTheGapps uses, they test on a lot of devices and a lot of thought has gone into those 10-20 lines.

I strongly disagree. In my humble opinion, the way they mount the partitions is the same as mine but in an over-complicated fashion. They check the fstab for the mount points which complicates things even further as it involves even more hard-coding and thus even more ways in which it can fail.

@infinity0 Thanks for your input.

@arovlad arovlad reopened this Dec 4, 2022
@infinity0
Copy link
Contributor Author

infinity0 commented Dec 4, 2022

later versions of Android might have other names

It is highly unlikely that future versions of Android will ever rename the system partition to something else.

What I meant is that other partitions matching your search pattern system* might exist. In fact one already does, namely system-ext_*. A few years ago one could also have easily said "it's highly unlikely that future versions of Android will have two system partitions" but this assumption is broken by A/B. We just can't predict the future.

As I understand, their complexity is based on Android docs, so it is just checking for all the concrete options, and not trying to guess unknown future scenarios. I can empathise with wanting to be future-proof, I'm just not sure it can be done safely.

@arovlad
Copy link
Owner

arovlad commented Dec 11, 2022

I changed the script to hard-code system and then append the active partition prefix if one exists, which is basically what MindTheGapps does. An additional check can be written to detect if the partition is present in fstab but if it isn't the script will fail anyway since the mount command will throw an error. The discussion remains open, but the issue itself is fixed and closed.

@arovlad arovlad closed this as completed Dec 11, 2022
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 a pull request may close this issue.

3 participants