erkanisik/yali-rs
1
0
Fork 0
forked from pisilinux-rs/yali-rs
Code Pull Requests Packages Releases 1 Activity

fix(partition): düzenlemede bağlama noktası seçilemiyordu

Browse Source 
Kullanıcı raporu: "düzenle kısmında bu seferde bağlama noktasını
seçemiyorum."

## Sebep — geçen turda benim eklediğim kod

NTFS için mountpoint'i isteğe bağlı yaparken diyaloga şunu
koymuştum:

    if mp_optional {
        if matches!(add_mountpoint.as_str(), "/" | "/home" | …) {
            add_mountpoint.clear();
        }
    }

egui `show()` HER KARE (~60 Hz) çağrıldığı için bu blok korumasızdı:
kullanıcı açılır listeden `/home` seçtiği anda bir sonraki karede
değer siliniyor, tekrar seçiliyor, tekrar siliniyordu.

Daha kötüsü `mp_optional` FİZİKSEL bölümlerde de doğru olduğu için
(orijinal mountpoint'i olan bir bölümü düzenlerken) `/`, `/home`,
`/var`, `/tmp`, `/swap`, `/boot`, `/boot/efi` listesinin TAMAMI seçilemez
 hale geliyordu.

## Düzeltme — iki koşul birlikte

1. **Yalnızca Ekleme modunda.** Eklemede mountpoint
   `suggest_mountpoint`'in ÜRETTİĞİ bir öneridir ve eski kalıntı
   olabilir. Düzenlemede bölümün GERÇEK değeridir; silmek kullanıcının
   verisini kaybettirirdi.
2. **Yalnızca `add_fstype` DEĞİŞTİĞİ karede.** `ManualState`'e
   `add_fstype_prev: Option<usize>` eklendi; "Yeni Bölüm" ve
   "Düzenle" butonlarında diyalog açılışında kuruluyor ki ilk karede
   yanlışlıkla silinmesin.

(2) olmadan ekleme modunda da aynı sorun yaşanırdı: NTFS seçip
`/home` yazan kullanıcının alanı yine her karede temizlenirdi.

## Aynı sınıf hata: testler ortama bağımlıydı

`is_removable()` gerçek sysfs okuyor. Bu makinede
`/sys/block/sda/removable == 1` olduğu için `/dev/sda` kullanan 3 test
çıkarılabilir sanılıp KIRILDI — o testler **şansa** geçiyormuş.
Şansa geçen test, test olmaktan kötüdür.

Saf katman probu dışarıdan alacak şekilde ayrıldı:
- `sort_disks_with(disks, is_removable)`
- `default_boot_device(root, disks, is_removable, has_partitions)`

Üretim sarmalayıcıları gerçek probu veriyor, testler sabit olanı
(`never_removable`).

## Testler (137 → 138)

- çıkarılabilir aygıt ada göre önde olsa bile sona gider
- dahili diskler yalnızca ada göre sıralanır

cargo check --all-targets : 0 hata, 0 uyarı
cargo test              : 138/138

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Erkan IŞIK
2026-09-27 19:52:58 +03:00
parent bd407adab3
commit cedbb6a741
3 changed files with 179 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
AGENTS.md
+54 -2
View File
@@ -413,6 +413,58 @@ sessiz bir sürpriz olmaz.
> Kural artık ikisinden de iyi; `boot_device_reason` aynı fonksiyonu
> çağırıp UI metnini oradan üretir — **ikinci bir uygulama yok**.
### ⚠️ `show()` HER KARE çalışır — girdi tepkisi KENAR TETİKLİ olmalı
egui `show()` ~60 Hz çağrılır. Bu yüzden kullanıcı girdisine tepki
veren **her durum mutasyonu kenar tetikli (change-detected) olmak
zorunda**; seviye tetikli ("şu an şu koşul doğruysa yaz") yazılan
kod kullanıcının yazdığını **sürekli siler**.
REGRESYON (kullanıcı: *"düzenle kısmında bu seferde bağlama noktasını
seçemiyorum"*):
```rust
// ❌ HER KARE çalışır
if mp_optional {
if matches!(add_mountpoint.as_str(), "/" | "/home" | …) {
add_mountpoint.clear(); // kullanıcı `/home` seçti → silindi
} // tekrar seç → tekrar silindi
}
```
`mp_optional` fiziksel bölümlerde de doğru olduğu için (orijinal
mountpoint'i olan bölümü düzenlerken) liste **tamamen kilitlenmişti**.
İki koşul birlikte gerekiyor:
| Koşul | Neden |
|---|---|
| **yalnızca Ekleme modunda** | Eklemede mountpoint `suggest_mountpoint`'in ÜRETTİĞİ öneridir, eski kalıntı olabilir. Düzenlemede bölümün **GERÇEK** değeridir — silmek kullanıcı verisini kaybettirir. |
| **yalnızca `add_fstype` DEĞİŞTİĞİ karede** (`add_fstype_prev`) | Geçiş değilse kullanıcının yazdığı her karede silinir. Aynı tuzak (b) olmadan ekleme modunda da yaşanırdı. |
> Bu sınız hata **sessizdir**: derleyici uyarı vermez, test de
> yakalamaz (UI kodu). Bu yüzden `ManualState`'e
> `add_fstype_prev: Option<usize>` alanı eklendi ve hem "Yeni Bölüm"
> hem "Düzenle" butonlarında diyalog açılışında kuruldu — ilk karede
> yanlışlıkla silinmesin diye.
### Testler ortamdan bağımsız olmalı
`is_removable()` gerçek `sysfs`'i okur. Bu geliştirme makinesinde
`/sys/block/sda/removable == 1` olduğu için `/dev/sda` kullanan testler
çıkarılabilir sanılıp **kırıldı** — yani o testler **şansa** geçiyormuş.
Şansa geçen test, test olmaktan kötüdür.
Çözüm: saf katman probu dışarıdan alır —
`sort_disks_with(disks, is_removable)` ve
`default_boot_device(root, disks, is_removable, has_partitions)`.
Üretim sarmalayıcıları gerçek probu verir, testler sabit olanı.
> `/dev/sda` gibi **gerçek aygıt adları** test verisi olarak
> kullanılıyorsa, o adın `sysfs`'te çevreye bağlı bir özelliği
> (removable gibi) sorgulanmadığından emin ol. Sorgulanıyorsa probu
> dışarıdan al.
### `selected_disk` hâlâ 3 yerde kullanılıyor
1. Tablo hangi diski gösteriyor + `manual_dual_bars`
2. Diyaloğun varsayılan hedefi
@@ -572,11 +624,11 @@ nedeni yorumla sabitlendi. Log bile "MiB" derken argüman "MB" gönderiyordu.
```
cargo check --all-targets : 0 hata, 0 uyarı
cargo test : 137/137 (test modülü sayısı arttıkça güncel tut)
cargo test : 138/138 (test modülü sayısı arttıkça güncel tut)
cargo clippy : ~77 uyarı, tamamı stil (collapsed_if, replace zinciri)
```
Bu oturumda 1 → 137 test. Test modülü olmayan dosyalar (`users.rs`,
Bu oturumda 1 → 138 test. Test modülü olmayan dosyalar (`users.rs`,
`location.rs`, `rescue.rs`, `funct.rs`, `autoinstall/mod.rs`, `main.rs`)
içine modüller eklendi. Çoklu test modülü paylaşılan yardımcıları
`steps/partition.rs::testutil` altındadır (`disk()`, `part()`).
src/steps/bootloader.rs
+30 -14
View File
@@ -50,17 +50,19 @@ fn disk_partition_count(disk: &str) -> usize {
///
/// Tek diskli kurulumda 3. adım çalışır: başka disk yoktur, davranış
/// kök diskiyle aynıdır.
pub fn default_boot_device<F>(
pub fn default_boot_device<R, F>(
root_disk: Option<&str>,
disks: &[crate::installer::DiskInfo],
is_removable: R,
has_partitions: F,
) -> Option<String>
where
R: Fn(&str) -> bool,
F: Fn(&str) -> bool,
{
// (1) çıkarılabilir aygıtlar ve (2) kök diski ele
disks.iter()
.filter(|d| !crate::steps::partition::is_removable(&d.name))
.filter(|d| !is_removable(&d.name))
.filter(|d| Some(d.name.as_str()) != root_disk)
.find(|d| has_partitions(&d.name))
.map(|d| d.name.clone())
@@ -86,9 +88,12 @@ fn boot_device_reason(state: &GlobalState, current: &str) -> Option<String> {
// Aynı kuralı bir kez daha çalıştırıp sonucun aygıtla örtüştüğünü
// doğruluyoruz. Kural tek kaynakta; burada yalnızca UI metni var.
let expected = default_boot_device(root_disk.as_deref(), &state.available_disks, |d| {
disk_partition_count(d) > 0
})?;
let expected = default_boot_device(
root_disk.as_deref(),
&state.available_disks,
crate::steps::partition::is_removable,
|d| disk_partition_count(d) > 0,
)?;
if expected == current {
Some(t!("bootloader_device_why_existing").to_string())
@@ -126,6 +131,7 @@ impl InstallerStep for BootloaderStep {
let chosen = default_boot_device(
root_disk.as_deref(),
&state.available_disks,
crate::steps::partition::is_removable,
|d| disk_partition_count(d) > 0,
)
.or_else(|| state.selected_disk.clone());
@@ -277,13 +283,23 @@ mod tests {
move |name: &str| v.iter().any(|p| p == name)
}
/// Test ortamında HİÇBİR aygıt çıkarılabilir sayılır.
///
/// ⚠️ Gerçek `is_removable` çağrılırsa testler ortama bağımlı
/// olur: bu geliştirme makinesinde `/sys/block/sda/removable == 1`
/// olduğu için "/dev/sda" kullanan testler çıkarılabilir sanılıp
/// kırılıyordu. Testler **şansa** geçiyordu.
fn never_removable(_name: &str) -> bool {
false
}
/// ⭐ Kullanıcının senaryosu: sda'da Windows, sdb'de PisiLinux
/// kuruluyor. GRUB sda'ya yazılmalı — çünkü firmware açılışta
/// sda'yı okuyor. Kök diski (sdb) seçilirse PisiLinux hiç görünmez.
#[test]
fn picks_the_disk_that_already_has_an_os() {
let disks = vec![d("/dev/sda"), d("/dev/sdb"), d("/dev/sdc")];
let got = default_boot_device(Some("/dev/sdb"), &disks, parts_on(&["/dev/sda"]));
let got = default_boot_device(Some("/dev/sdb"), &disks, never_removable, parts_on(&["/dev/sda"]));
assert_eq!(
got.as_deref(),
Some("/dev/sda"),
@@ -297,7 +313,7 @@ mod tests {
fn single_disk_install_falls_back_to_the_root_disk() {
let disks = vec![d("/dev/sda")];
assert_eq!(
default_boot_device(Some("/dev/sda"), &disks, parts_on(&["/dev/sda"])).as_deref(),
default_boot_device(Some("/dev/sda"), &disks, never_removable, parts_on(&["/dev/sda"])).as_deref(),
Some("/dev/sda")
);
}
@@ -308,7 +324,7 @@ mod tests {
fn no_existing_os_anywhere_falls_back_to_root() {
let disks = vec![d("/dev/sda"), d("/dev/sdb")];
assert_eq!(
default_boot_device(Some("/dev/sdb"), &disks, parts_on(&[])).as_deref(),
default_boot_device(Some("/dev/sdb"), &disks, never_removable, parts_on(&[])).as_deref(),
Some("/dev/sdb")
);
}
@@ -320,7 +336,7 @@ mod tests {
let disks = vec![d("/dev/sda"), d("/dev/sdb")];
// sda'da kurulum, sdb boş → sda
assert_eq!(
default_boot_device(Some("/dev/sda"), &disks, parts_on(&["/dev/sda"])).as_deref(),
default_boot_device(Some("/dev/sda"), &disks, never_removable, parts_on(&["/dev/sda"])).as_deref(),
Some("/dev/sda")
);
}
@@ -331,7 +347,7 @@ mod tests {
fn first_candidate_in_sorted_order_wins() {
let disks = vec![d("/dev/sda"), d("/dev/sdb"), d("/dev/sdc")];
assert_eq!(
default_boot_device(Some("/dev/sdd"), &disks, parts_on(&["/dev/sdb", "/dev/sdc"])).as_deref(),
default_boot_device(Some("/dev/sdd"), &disks, never_removable, parts_on(&["/dev/sdb", "/dev/sdc"])).as_deref(),
Some("/dev/sdb"),
"sıralı listedeki ilk bölümlü disk seçilmeli"
);
@@ -343,10 +359,10 @@ mod tests {
fn unknown_root_disk_is_handled() {
let disks = vec![d("/dev/sda"), d("/dev/sdb")];
assert_eq!(
default_boot_device(None, &disks, parts_on(&["/dev/sda"])).as_deref(),
default_boot_device(None, &disks, never_removable, parts_on(&["/dev/sda"])).as_deref(),
Some("/dev/sda")
);
assert_eq!(default_boot_device(None, &disks, parts_on(&[])), None);
assert_eq!(default_boot_device(None, &disks, never_removable, parts_on(&[])), None);
}
/// Disk listesi boşken bile KÖRK disk biliniyorsa o kullanılır —
@@ -355,7 +371,7 @@ mod tests {
#[test]
fn empty_disk_list_still_uses_the_root_disk() {
assert_eq!(
default_boot_device(Some("/dev/sda"), &[], parts_on(&["/dev/sda"])).as_deref(),
default_boot_device(Some("/dev/sda"), &[], never_removable, parts_on(&["/dev/sda"])).as_deref(),
Some("/dev/sda"),
"liste boş olsa da bilinen kök disk kullanılmalı"
);
@@ -365,6 +381,6 @@ mod tests {
/// `selected_disk`'e düşer, o da yoksa aygıt seçilemez.
#[test]
fn nothing_known_yields_none() {
assert_eq!(default_boot_device(None, &[], parts_on(&[])), None);
assert_eq!(default_boot_device(None, &[], never_removable, parts_on(&[])), None);
}
}
src/steps/partition.rs
+95 -22
View File
@@ -228,6 +228,19 @@ pub fn list_block_devices() -> Vec<crate::installer::DiskInfo> {
/// sezgisi (firmware'in ilk açtığı disk) en azından yanlış bir aygıta
/// denk gelmez.
fn sort_disks(disks: &mut [DiskInfo]) {
sort_disks_with(disks, is_removable);
}
/// Sıralamanın SAF hâli: probu dışarıdan alır.
///
/// `is_removable` gerçek `sysfs`'i okur; testlerde bu makinede
/// `/sys/block/sda/removable == 1` olduğu için "/dev/sda" adını
/// kullanan testler çıkarılabilir sanılıp kırılıyordu. Testler
/// **şansa** geçiyordu — bu katman onları ortamdan bağımsız kılar.
fn sort_disks_with<R>(disks: &mut [DiskInfo], is_removable: R)
where
R: Fn(&str) -> bool,
{
disks.sort_by(|a, b| {
let ra = is_removable(&a.name);
let rb = is_removable(&b.name);
@@ -325,6 +338,17 @@ struct ManualState {
selected_free: bool,
/// Diyalog: seçili dosya sistemi
add_fstype: usize, // FsType listesindeki indeks
/// Diyalogun son çizildiğinde `add_fstype` neydi.
///
/// ⚠️ egui `show()` her karede (~60 Hz) çağrılır. "NTFS'e geçti,
/// mountpoint'i temizle" gibi bir işlem korumasız yapılırsa **her
/// karede** çalışır ve kullanıcının yazdığı değeri anında siler:
/// açılır listeden `/home` seçer, bir sonraki karede `/home`
/// kalıntı sayılıp temizlenir, tekrar seçer, tekrar silinir.
///
/// Bu alan "geçiş oldu mu" sorusunu cevaplar: temizleme yalnızca
/// `add_fstype` DEĞİŞTİĞİ karede yapılır.
add_fstype_prev: Option<usize>,
/// Diyalog: boyut alanı (MB, metin girişi)
add_size_str: String,
/// Diyalog: bağlama noktası
@@ -1757,6 +1781,8 @@ impl PartitionStep {
"/boot/efi" => FS_OPTIONS.iter().position(|(_, t)| *t == FsType::Fat32).unwrap_or(0),
_ => 0,
};
// Öneri ile uyumlu fstype yüklendi; geçiş sayılmasın.
self.manual.add_fstype_prev = Some(self.manual.add_fstype);
// Yeni bölümler varsayılan olarak biçimlendirilir
self.manual.add_skip_format = false;
}
@@ -1795,6 +1821,10 @@ impl PartitionStep {
self.manual.add_fstype = FS_OPTIONS.iter()
.position(|(_, ft)| *ft == part.fstype)
.unwrap_or(0);
// Diyalog açılışında geçiş YOKTUR: mevcut
// mountpoint'i korumak istiyoruz. Referans burada
// kurulur ki ilk karede temizleme çalışmasın.
self.manual.add_fstype_prev = Some(self.manual.add_fstype);
self.manual.add_encrypt = part.encrypt;
self.manual.add_luks_password = part.luks_password.clone();
self.manual.add_skip_format = part.skip_format;
@@ -2083,13 +2113,40 @@ impl PartitionStep {
},
&fstype_now,
);
if mp_optional {
// Önceki öneri ("/boot/efi", "/", "swap"…) geçersiz
// bir kalıntı olurdu; temizleniyor.
if matches!(self.manual.add_mountpoint.as_str(),
"/" | "/boot" | "/boot/efi" | "/home" | "/var" | "/tmp" | "swap") {
// ⚠️ Temizleme YALNIZCA (a) Ekleme modunda VE
// (b) dosya sistemi değiştiği karede yapılır.
//
// REGRESYON: buradaki `if mp_optional { temizle }`
// korumasızdı, yani HER KARE çalışıyordu. Kullanıcı
// açılır listeden `/home` seçtiği anda bir sonraki
// karede değer siliniyordu — "bağlama noktasını
// seçemiyorum" olarak görünüyordu. `mp_optional`
// fiziksel bölümlerde de doğru olduğu için (orijinal
// mountpoint'i olan bir bölümü düzenlerken de) liste
// tamamen kilitleniyordu.
//
// **Düzenleme modunda HİÇBİR ZAMAN temizlenmez.**
// Eklemede mountpoint `suggest_mountpoint` tarafından
// ÜRETİLMİŞ bir öneridir ve eski bir kalıntı olabilir;
// düzenlemede ise bölümün GERÇEK değeridir. Silmek
// kullanıcının verisini kaybettirirdi.
//
// (b) koşulu olmadan ekleme modunda da aynı sorun
// yaşanırdı: kullanıcı NTFS seçip `/home` yazdığında
// alan yine her karede temizlenirdi.
let fstype_changed =
self.manual.add_fstype_prev != Some(self.manual.add_fstype);
if !self.manual.editing && fstype_changed {
if mp_optional
&& matches!(self.manual.add_mountpoint.as_str(),
"/" | "/boot" | "/boot/efi" | "/home" | "/var" | "/tmp" | "swap")
{
self.manual.add_mountpoint.clear();
}
}
// Geçiş işaretini her karede güncelle.
self.manual.add_fstype_prev = Some(self.manual.add_fstype);
if mp_optional {
ui.label(
egui::RichText::new(t!("mp_mountpoint_optional"))
.size(11.0)
@@ -3187,25 +3244,41 @@ mod tests {
/// Bu, "ilk disk" kavramını güvenli kılan asıl nokta: önyükleyici
/// hedefi olarak seçilirse `grub-install` MBR'yi boot medyasına
/// yazar ve sistem açılmaz.
///
/// ⚠️ Probu dışarıdan veriliyor: gerçek `is_removable` bu
/// geliştirme makinesinde `/sys/block/sda/removable == 1` okuduğu
/// için doğrudan çağrılsaydı test ortama bağımlı olurdu.
#[test]
fn removable_media_sorts_last() {
let mut disks = vec![
crate::installer::DiskInfo {
name: "/dev/sda".into(), vendor: String::new(), model: String::new(),
drive_type: "SSD".into(), size_gb: 500,
size_bytes: 500 * 1_073_741_824,
},
crate::installer::DiskInfo {
// USB: removable=1 olduğu için aygıtın ADI ne olursa olsun sona gider
name: "/dev/sdz".into(), vendor: String::new(), model: String::new(),
drive_type: "USB".into(), size_gb: 8,
size_bytes: 8 * 1_073_741_824,
},
];
// is_removable gerçek sysfs'e bakar; testte olmayabilir.
// Sıralamanın "ada göre" kısmını doğrula (removable hepsi false ise).
super::sort_disks(&mut disks);
assert_eq!(disks[0].name, "/dev/sda", "ada göre sda önce gelmeli");
let mk = |name: &str, gtype: &str| crate::installer::DiskInfo {
name: name.into(), vendor: String::new(), model: String::new(),
drive_type: gtype.into(), size_gb: 500,
size_bytes: 500 * 1_073_741_824,
};
let mut disks = vec![mk("/dev/sdz", "USB"), mk("/dev/sda", "SSD")];
super::sort_disks_with(&mut disks, |n| n == "/dev/sdz");
assert_eq!(
disks.iter().map(|d| d.name.as_str()).collect::<Vec<_>>(),
vec!["/dev/sda", "/dev/sdz"],
"çıkarılabilir aygıt ada göre önde olsa bile sona gider"
);
}
/// Hiçbir şey çıkarılabilir değilse sıralama yalnızca ada göre.
#[test]
fn internal_disks_sort_by_name_only() {
let mk = |name: &str| crate::installer::DiskInfo {
name: name.into(), vendor: String::new(), model: String::new(),
drive_type: "SSD".into(), size_gb: 500,
size_bytes: 500 * 1_073_741_824,
};
let mut disks = vec![mk("/dev/sdc"), mk("/dev/sda"), mk("/dev/sdb")];
super::sort_disks_with(&mut disks, |_| false);
assert_eq!(
disks.iter().map(|d| d.name.as_str()).collect::<Vec<_>>(),
vec!["/dev/sda", "/dev/sdb", "/dev/sdc"]
);
}
// ── detect_drive_type ─────────────────────────────────────