Rework ZFS configuration page and fix GELI encryption with swap#88
Merged
Conversation
- Fix encpass= ordering so it immediately follows the .eli partition line, preventing boot failure when swap is present - Tie swap encryption (SWAP.eli) to disk encryption instead of a separate checkbox - Redesign ZFS page to two-column layout: settings grid on the left, disk list on the right - Rename "Pool Type" to "Pool Layout" and simplify ComboBox entries to stripe, mirror, raidz1, raidz2, raidz3 - Remove "single disk" option (replaced by stripe) - Always show Pool Name label and entry instead of a checkbox toggle - Remove Encrypt Swap and Mirror Swap checkboxes (swap mirror is handled automatically by pc-sysinstall for multi-disk pools) - Remove duplicate password strength functions, import from gbi_common - Remove dead code: unused variables (memory, auto), tree_selection, on_check_poll, on_check_swap_encrypt, on_check_swap_mirror, no-op row increment - Fix self.zfs_four_k initialized as string "True" instead of bool - Replace set_from_stock with set_from_icon_name for password match icon - Convert all %s formatting to f-strings - Update Gtk.Label() calls to use label= keyword - Bump version to 11.1
Reviewer's GuideReworks the ZFS configuration UI into a two‑column layout, simplifies pool layout selection, ties swap encryption to disk encryption with correct encpass ordering, refactors password handling and validation helpers, and performs several small cleanups and version bump. Class diagram for updated ZFS configuration logicclassDiagram
class ZFS {
+list zfs_disk_list
+button3
+vbox1
+store
+check_cell
+mirrorTips
+mirror
+poolType
+pool
+scheme
+zfs_four_k bool
+swap_entry
+disk_encript bool
+passwd_label
+password
+strenght_label
+vpasswd_label
+repassword
+img
+__init__(button3)
+save_selection()
+sheme_selection(combobox)
+mirror_selection(combobox)
+on_check(widget)
+on_check_encrypt(widget)
+get_model()
+load_data()
+check_if_small_disk(size_string)
+col1_toggled_cb(cell, path, model)
+small_disk_warning()
+resset_selection(widget, window)
+digit_only(entry)
+passwdstrength(widget)
+passwdVerification(widget)
}
class GtkComponents {
<<interface>> GtkComponents
+VBox
+HBox
+Grid
+Label
+Entry
+ComboBoxText
+CheckButton
+TreeView
+TreeViewColumn
+ScrolledWindow
+Image
}
ZFS ..> GtkComponents : uses
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The logic that enables/disables
button3based onself.mirrorandlen(self.zfs_disk_list)is still duplicated acrossmirror_selection,on_check_encrypt,col1_toggled_cb, andpasswdVerification; consider extracting this into a single helper (e.g._update_button3_sensitivity()) to keep the rules consistent and easier to maintain. - In
boot_manager.pyyou’ve left a commented-outencryptedcheck and conditional aroundself.refind; either wire this into the behavior or remove the dead code/comment to avoid confusion about the intended logic. - Some of the user-facing mirror layout messages (e.g. in
mirror_selectionfor raidz1/2/3) are slightly awkward or missing spaces around the(select the smallest disk first)suffix; a quick pass to normalize these strings would improve the UI text clarity.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The logic that enables/disables `button3` based on `self.mirror` and `len(self.zfs_disk_list)` is still duplicated across `mirror_selection`, `on_check_encrypt`, `col1_toggled_cb`, and `passwdVerification`; consider extracting this into a single helper (e.g. `_update_button3_sensitivity()`) to keep the rules consistent and easier to maintain.
- In `boot_manager.py` you’ve left a commented-out `encrypted` check and conditional around `self.refind`; either wire this into the behavior or remove the dead code/comment to avoid confusion about the intended logic.
- Some of the user-facing mirror layout messages (e.g. in `mirror_selection` for raidz1/2/3) are slightly awkward or missing spaces around the `(select the smallest disk first)` suffix; a quick pass to normalize these strings would improve the UI text clarity.
## Individual Comments
### Comment 1
<location path="src/boot_manager.py" line_range="68-71" />
<code_context>
read = open(disk_scheme, 'r')
read_scheme = read.read()
scheme = 'GPT' if 'GPT' in read_scheme else 'MBR'
+ # encrypted = 'encpass=' in read_scheme
Title = Gtk.Label('Boot Option', name="Header")
Title.set_property("height-request", 50)
</code_context>
<issue_to_address>
**question (bug_risk):** Commented-out encryption handling suggests incomplete logic around rEFInd availability.
The new `encpass=` detection is only present as a commented line, and the rEFInd condition is still just `scheme == 'GPT' and loader == "UEFI"`. If rEFInd is incompatible with some encrypted setups, this can result in offering rEFInd when it shouldn’t be available. Either fully implement and verify the encryption check around rEFInd, or remove the commented code to avoid ambiguity about the intended behavior.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Rework the ZFS configuration UI and partitioning logic, simplifying pool layout selection and tying swap encryption to disk encryption while updating supporting code and versioning.
New Features:
Bug Fixes:
Enhancements:
Build: