-
Notifications
You must be signed in to change notification settings - Fork 7
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
feature: installer image #603
base: main
Are you sure you want to change the base?
Conversation
b447883
to
592e118
Compare
816fa67
to
2580a8a
Compare
This crash was from the vmware mouse-drivers. Issue has been mitigated by disabling VMware guest integration, which I will probably never need anyway
📝 WalkthroughWalkthroughThe changes update the system’s Nix configuration by adding a debugging flag and a system-specific configuration structure within the flake. Two new applications—one for installing the system (creating disk images, ISOs, and launching QEMU) and another for previewing the Awesome window manager (configuring and launching via Xephyr)—are introduced. Additionally, a new "awesome" entry is added to the npins sources, and package definitions are extended with an awesome override and an installer-iso. Multiple installer modules now configure services for LVM, MOTD, Neovim, network, SDDM, and XTerm. The development shell is enhanced with lua-language-server. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant InstallerScript
participant QEMU
User->>InstallerScript: Start installer
InstallerScript->>InstallerScript: Check for disk image & build ISO
InstallerScript->>QEMU: Launch virtual machine with parameters
QEMU-->>InstallerScript: VM execution complete
InstallerScript->>User: Display ISO info and cleanup actions
sequenceDiagram
participant User
participant AwesomePreview
participant LuaConfig
participant Xephyr
User->>AwesomePreview: Request Awesome WM preview
AwesomePreview->>LuaConfig: Generate awesomerc.lua configuration
AwesomePreview->>Xephyr: Launch nested Awesome WM session
Xephyr-->>User: Render Awesome WM interface
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/installer/motd.nix (1)
4-23
: Informative MOTD with helpful user guidance.The message content provides valuable information about the environment and keybindings for users unfamiliar with the Awesome window manager.
I noticed line 19 has
<s>Rofi is available as well, you can open it by pressing <tt>Mod4+d</tt>.</s>
. The strikethrough suggests this functionality isn't actually available. Consider either:
- Removing this line completely
- Implementing the Rofi binding if it's intended to be available
- Clarifying why this information is struck through
packages/installer/awesome.nix (1)
25-34
: Duplicate awesome package override definition.This awesome package override is identical to the one in packages/default.nix (lines 21-30). Having the same definition in two places creates maintenance challenges if changes are needed in the future.
Consider defining the awesome package override in one location and importing it where needed, or passing it as a parameter between the files.
flake.nix (2)
6-7
: Consider makingdebug
optional or environment-driven.
Enabling debugging unconditionally may produce excessive logs or degrade performance in production scenarios. Allowing a dynamic toggle could be a more flexible approach.Here is an example diff to make
debug
honor an environment variable:- debug = true; + debug = builtins.getEnv "DEBUG" == "1";
58-68
: Consider gracefully handling Xephyr to avoid leftover processes.
Launching Xephyr in the background without cleanup may leave it running if the script exits unexpectedly. Adding atrap
to kill Xephyr or storing its PID could provide a safer termination strategy.A possible snippet:
pkgs.writeShellScriptBin "awesome-preview" '' xephyrPid="$(${pkgs.xorg.xorgserver}/bin/Xephyr :5 &)"; sleep 1 + trap "kill $xephyrPid" EXIT DISPLAY=:5 ${self'.packages.awesome}/bin/awesome --config ${rc_lua} '';
packages/installer/awesomerc.lua (2)
110-128
: Ensure wallpaper scaling for different screen resolutions.
While this approach sets a single wallpaper, it may appear stretched or cropped on varying screen sizes. Consider adding logic to preserve aspect ratio or select different images per screen.
610-612
: Sloppy focus can be disorienting for some users.
Focus follows the mouse by default here, which is a personal preference. You might consider making it optional or documenting it so that users understand this behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (8)
packages/installer/icons/globe.svg
is excluded by!**/*.svg
packages/installer/icons/manual.svg
is excluded by!**/*.svg
packages/installer/icons/nix-flake.svg
is excluded by!**/*.svg
packages/installer/icons/parted.svg
is excluded by!**/*.svg
packages/installer/icons/power-off.svg
is excluded by!**/*.svg
packages/installer/icons/reboot.svg
is excluded by!**/*.svg
packages/installer/icons/terminal.svg
is excluded by!**/*.svg
packages/installer/nix-glow-black.png
is excluded by!**/*.png
📒 Files selected for processing (14)
flake.nix
(2 hunks)npins/sources.json
(1 hunks)packages/default.nix
(2 hunks)packages/installer/awesome.nix
(1 hunks)packages/installer/awesomerc.lua
(1 hunks)packages/installer/base.nix
(1 hunks)packages/installer/default.nix
(1 hunks)packages/installer/lvm.nix
(1 hunks)packages/installer/motd.nix
(1 hunks)packages/installer/neovim.nix
(1 hunks)packages/installer/network.nix
(1 hunks)packages/installer/sddm.nix
(1 hunks)packages/installer/xterm.nix
(1 hunks)parts/auxiliary.nix
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: generate_matrix
- GitHub Check: check_flake
🔇 Additional comments (20)
packages/installer/sddm.nix (1)
1-9
: SDDM configuration looks good for an installer environment.The configuration properly sets up SDDM with auto-login for the "nixos" user, which is appropriate for an installer image where you want to avoid authentication barriers.
packages/installer/neovim.nix (1)
1-9
: Neovim configuration is properly structured.The configuration correctly sets Neovim as the default editor through the EDITOR environment variable and adds it to the system packages. The package reference looks correct.
packages/installer/network.nix (1)
1-8
: Network configuration is properly set up.The configuration enables NetworkManager and correctly sets up the nm-applet to start with the graphical session.
packages/installer/lvm.nix (1)
1-9
: LVM configuration is appropriate for an installer.The configuration enables necessary LVM features (thin provisioning, VDO, and dmeventd) which are useful for an installer that might need to handle various disk configurations.
packages/installer/default.nix (1)
1-22
: Well-structured NixOS configurationThe installer module is well-organized with a clean structure. The module imports all necessary configurations and passes the required specialArgs to child modules.
packages/installer/xterm.nix (1)
1-14
: Clean XTerm configuration with appropriate defaultsThe XTerm configuration provides reasonable defaults with good readability choices (dark background, light foreground, and a monospace font at a readable size). Using systemd tmpfiles for configuration distribution is an elegant approach.
parts/auxiliary.nix (1)
32-32
: Good addition of lua-language-serverAdding lua-language-server is appropriate given the integration of Awesome window manager (which uses Lua for configuration). This will provide a better development experience when working with Awesome configuration files.
packages/installer/base.nix (2)
12-15
: Good ISO configuration choicesThe configuration provides appropriate customization for the ISO image with a custom edition name and efficient compression settings. The zstd compression at level 10 offers a good balance between compression ratio and decompression speed.
17-19
: Helpful workaround for VMware guest issuesGood use of
lib.mkForce
to disable VMware guest tools that were causing issues. The comment explaining the rationale is helpful for future maintenance.npins/sources.json (1)
3-14
: Good addition of the Awesome window manager pin.The new entry for "awesome" follows the established pattern in the file with all necessary components:
- Properly references the GitHub repository and owner
- Uses a specific full SHA revision for reproducibility
- Includes a complete URL to the tarball
- Contains a hash for integrity verification
packages/default.nix (4)
2-4
: Appropriate function signature update.Adding the
npins
parameter to the function signature is necessary to access the pin definitions, which is consistent with how this is being used later in the file.
34-43
: Properly structured installer-iso package definition.The installer-iso package is correctly defined, calling the installer module with appropriate inherited values. The deep attribute path to the isoImage is a standard pattern for NixOS ISO builds.
50-50
: Good inheritance of the awesome package.Inheriting the awesome package for Linux systems makes sense given the nature of the window manager.
21-30
:❓ Verification inconclusive
Well-structured awesome package override.
The overrideAttrs for the awesome package correctly uses the pinned version and source. The postPatch script properly handles the shebang patching for the test file.
Note that you're setting
patches = []
, which could potentially remove important patches from the original package. Was this intentional?
🏁 Script executed:
#!/bin/bash # Check if the original awesome package has any patches nix-instantiate --eval -E '(import <nixpkgs> {}).awesome.patches'Length of output: 173
Verify Empty Patches List in Awesome Package Override
The override for the awesome package is well-structured and correctly uses the pinned version and source. However, setting
patches = []
may unintentionally remove any patches that the original package defines. Our initial automated check (usingnix-instantiate
) did not yield the expected output due to the command being unavailable, so it's unclear whether the original package actually applies any patches.
- File:
packages/default.nix
(lines 21–30)- Concern: Verify if clearing patches (i.e., setting
patches = []
) is intentional, or if any necessary patches from the original awesome package should be preserved.Please manually verify the behavior regarding patches to ensure no critical updates are lost.
packages/installer/motd.nix (1)
25-36
: Well-configured systemd user service for MOTD display.The service is properly set up to:
- Start with the graphical session
- Display the MOTD using zenity for a graphical dialog
- Restart on failure
- Run as a simple service type
packages/installer/awesome.nix (4)
7-14
: Clean configuration generation with proper substitutions.The
rc_lua
generation usessubstitute
to properly inject paths to wallpaper and icons into the configuration file. This approach keeps the configuration template clean while allowing for dynamic path insertion.
16-23
: Well-structured menu entries with essential installer options.The menu provides a good selection of essential tools for an installation environment:
- NixOS Manual for help
- GParted for disk partitioning
- System management options (reboot, power off)
Each entry includes an appropriate icon for visual identification.
38-41
: Proper window manager configuration.The awesome window manager is correctly enabled and configured to use the custom package defined above.
43-46
: Clean approach to user configuration with tmpfiles.Using systemd's tmpfiles to create symlinks in the user's config directory is a clean, declarative approach that:
- Avoids modifying the user's home directory directly
- Ensures configuration is always up-to-date with the system definition
- Follows the principle of reproducibility
packages/installer/awesomerc.lua (1)
64-65
:❓ Verification inconclusive
Verify that
menu_entries
is available at runtime.
This snippet dynamically requires"menu_entries"
, but it's unclear where this file is located or how it's included in the Lua path.
🏁 Script executed:
#!/bin/bash # Description: Search for a file named "menu_entries.lua" or "menu_entries" to ensure it exists. fd -e lua -e '' 'menu_entries' --exec ls -l {}Length of output: 48
Action Required: Confirm the Runtime Availability of
menu_entries
The shell script did not return any matches for a file named
menu_entries.lua
or similar. This leaves open two possibilities:
- The
menu_entries
module is generated dynamically at runtime (or during the build process) and may not appear in the source repository.- The file may be missing from the repository and consequently unavailable at runtime.
Next Steps:
- Manual Verification: Ensure that the environment where this code runs correctly generates or includes the
menu_entries
module.- Clarification in Documentation: If the module is dynamically generated, document the process or dependencies required so that future developers understand how
menu_entries
becomes available.Please validate the availability of this module in your runtime or build process.
Summary by CodeRabbit