-
-
Notifications
You must be signed in to change notification settings - Fork 3k
manager: simplify find boot image #2901
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the boot partition detection logic to simplify the API by removing the OTA parameter from several functions and consolidating partition selection logic. The main changes include:
- Refactored
choose_boot_devicetochoose_boot_partitionto return partition names instead of full device paths - Removed OTA toggle parameters from CLI commands
default-partitionandavailable-partitions - Simplified partition selection logic by moving slot suffix handling to callers
- Removed the
default-deviceCLI command and theget_default_partition_namefunction
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| userspace/ksud/src/cli.rs | Removed OTA parameters from BootInfo enum variants and simplified CLI command handlers |
| userspace/ksud/src/boot_patch.rs | Refactored partition selection logic, removed OTA parameter from several functions, changed return types |
| manager/app/src/main/java/me/weishu/kernelsu/ui/util/KsuCli.kt | Consolidated partition detection functions and removed OTA parameters from function signatures |
| manager/app/src/main/java/me/weishu/kernelsu/ui/screen/Install.kt | Updated UI code to use simplified partition detection APIs |
Comments suppressed due to low confidence (2)
userspace/ksud/src/boot_patch.rs:730
- The non-Android stub function
choose_boot_devicestill exists but the Android version has been renamed tochoose_boot_partition. This creates an API inconsistency between platforms. Either:
- Add a non-Android stub for
choose_boot_partitionwith the same signature - Or remove the unused
choose_boot_devicestub
Since choose_boot_partition is now being called in find_boot_image (line 672), and find_boot_image is callable on non-Android platforms (it has cross-platform logic), there should be a non-Android stub for choose_boot_partition.
#[cfg(not(target_os = "android"))]
pub fn choose_boot_device(
_kmi: &str,
_ota: bool,
_is_replace_kernel: bool,
_partition: &Option<String>,
) -> Result<String> {
bail!("Current OS is not android, refusing auto bootdevice detection")
}
userspace/ksud/src/boot_patch.rs:758
- The refactored
list_available_partitionsfunction has removed the ordering logic that previously prioritized partitions based onskip_init_bootand partition existence. The old implementation would return partitions in an order like["init_boot", "boot", "vendor_boot"]or["vendor_boot", "boot", "init_boot"]based on the logic, but now it always returns them in the fixed order["boot", "init_boot", "vendor_boot"](filtered by existence). If the partition order was meaningful for the UI or other consumers, this could be a breaking change.
pub fn list_available_partitions() -> Vec<String> {
let slot_suffix = get_slot_suffix(false);
let candidates = vec!["boot", "init_boot", "vendor_boot"];
candidates
.into_iter()
.filter(|name| Path::new(&format!("/dev/block/by-name/{}{}", name, slot_suffix)).exists())
.map(|s| s.to_string())
.collect()
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
manager/app/src/main/java/me/weishu/kernelsu/ui/screen/Install.kt
Outdated
Show resolved
Hide resolved
manager/app/src/main/java/me/weishu/kernelsu/ui/screen/Install.kt
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Jehado1991
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: YuKongA <[email protected]>
Co-authored-by: YuKongA <[email protected]> Signed-off-by: u9521 <[email protected]>
Co-authored-by: YuKongA <[email protected]> Signed-off-by: u9521 <[email protected]>
Co-authored-by: YuKongA <[email protected]> Signed-off-by: u9521 <[email protected]>
No description provided.