-
-
Notifications
You must be signed in to change notification settings - Fork 39
Add text-based boot splash #231
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
Reviewer's GuideThis PR introduces a text-based boot splash system integrated into the GhostBSD build and live-boot flow, adds ANSI-colored, NO_COLOR/FORCE_COLOR–aware logging, enhances ZFS tuning and build prerequisites, and updates desktop and autologin scripts to invoke new splash routines. Sequence diagram for splash script invocation during live bootsequenceDiagram
participant BootLoader
participant InitScript
participant ExtendedBootScript
participant DesktopEnv
actor User
BootLoader->>User: Display PCX splash (if available)
BootLoader->>InitScript: Start system initialization
InitScript->>User: Show ASCII logo and progress bar (0-95%)
InitScript->>ExtendedBootScript: Invoke for desktop finalization (95-100%)
ExtendedBootScript->>User: Show ASCII logo and progress bar (final phase)
ExtendedBootScript->>DesktopEnv: Start desktop environment
User->>InitScript: Press ESC to reveal verbose messages (optional)
User->>ExtendedBootScript: Press ESC to reveal verbose messages (optional)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- This PR bundles a huge set of unrelated changes (color logging, ZFS tuning, splash screen, autologin tweaks, etc.); please split it into smaller, focused PRs to make review and testing more manageable.
- The new common_config/splash-setup.sh script is very large and complex—consider refactoring it into smaller, well-named modules or functions to improve readability and maintainability.
- The gitpkg.sh fallback logic references hard-coded local paths (e.g.
/usr/home/ericbsd/...
); make these developer-specific paths configurable or remove them so the script works on other machines.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- This PR bundles a huge set of unrelated changes (color logging, ZFS tuning, splash screen, autologin tweaks, etc.); please split it into smaller, focused PRs to make review and testing more manageable.
- The new common_config/splash-setup.sh script is very large and complex—consider refactoring it into smaller, well-named modules or functions to improve readability and maintainability.
- The gitpkg.sh fallback logic references hard-coded local paths (e.g. `/usr/home/ericbsd/...`); make these developer-specific paths configurable or remove them so the script works on other machines.
## Individual Comments
### Comment 1
<location> `common_config/gitpkg.sh:60-62` </location>
<code_context>
+ log "INFO: Cloning with ${timeout}s timeout..."
+
+ # Run git clone with timeout
+ if timeout "$timeout" sh -c "$git_cmd" 2>&1 | while IFS= read -r line; do
+ # Show progress for verbose output
+ case "$line" in
+ *"Cloning into"*|*"remote: Counting"*|*"Receiving objects"*)
+ echo " $line"
</code_context>
<issue_to_address>
**issue (bug_risk):** The use of 'timeout' with 'sh -c' and piped output may not propagate the exit code of 'git clone' reliably.
Piping the output in this way can obscure the actual exit code from 'git clone', which may lead to incorrect error handling. To avoid this, capture the exit code immediately after running the command or use a subshell to preserve it.
</issue_to_address>
### Comment 2
<location> `common_config/gitpkg.sh:100-102` </location>
<code_context>
- cp -R /usr/home/ericbsd/projects/ghostbsd/pc-sysinstall "${release}/pc-sysinstall"
+
+ # Check network connectivity
+ if ! check_network; then
+ log "WARNING: No network connectivity, checking for local copy"
+ if [ -d "/usr/home/ericbsd/projects/ghostbsd/pc-sysinstall" ]; then
+ log "INFO: Using local copy"
+ cp -R /usr/home/ericbsd/projects/ghostbsd/pc-sysinstall "${release}/pc-sysinstall"
</code_context>
<issue_to_address>
**suggestion:** Hardcoded fallback paths may not be portable or configurable for other environments.
This path should be configurable or clearly documented to avoid issues in other environments.
</issue_to_address>
### Comment 3
<location> `common_config/autologin.sh:203-140` </location>
<code_context>
+ printf "set tty (tty)
</code_context>
<issue_to_address>
**issue (bug_risk):** The fish config for autologin runs 'startx' twice in some cases.
Both branches of the conditional run 'startx', and it is also called after the conditional, leading to two executions. Please update the logic so 'startx' runs only once per session.
</issue_to_address>
### Comment 4
<location> `common_config/finalize.sh:24-25` </location>
<code_context>
+ echo 'kern.log_console_output=0' >> "${release}/etc/sysctl.conf"
+
+ # Add quiet boot options to loader.conf if not already present
+ if ! grep -q "boot_verbose" "${release}/boot/loader.conf" 2>/dev/null; then
+ cat >> "${release}/boot/loader.conf" << 'EOF'
+
+# Quiet boot options
</code_context>
<issue_to_address>
**issue (bug_risk):** Appending to loader.conf without checking for existing keys may cause duplicate entries.
If keys like 'boot_verbose' or 'autoboot_delay' are already present, appending may create duplicates or conflicts. Please update existing keys or remove duplicates to maintain consistent boot settings.
</issue_to_address>
### Comment 5
<location> `common_config/splash-setup.sh:25-26` </location>
<code_context>
+
+setup_interactive_splash()
+{
+ echo "Current working directory: $(pwd)"
+ echo "cwd variable: ${cwd}"
+
+ # Splash configuration - no bootloader logo modifications
</code_context>
<issue_to_address>
**nitpick:** Debugging output in setup_interactive_splash may be left over from development.
If these echo statements are not needed in production, please remove them to keep logs and output clean.
</issue_to_address>
### Comment 6
<location> `README.md:11` </location>
<code_context>
## Features
* Build GhostBSD from packages
-* Multiple desktop environments (Mate, XFCE, Gershwin, Plasma, and more)
+* Multiple desktop environments (Mate, XFCE, Gershwin, and Kde Plasma 6)
* Hybrid DVD/USB image
* Configurable ZFS memory management for build optimization
</code_context>
<issue_to_address>
**suggestion (typo):** Correct 'Kde' to 'KDE' for consistency.
Use 'KDE Plasma 6' to align with standard naming conventions.
```suggestion
* Multiple desktop environments (Mate, XFCE, Gershwin, and KDE Plasma 6)
```
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Add double quotes as correction
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
Add text-based boot splash screen system
Implement a comprehensive splash screen system that provides visual
feedback during GhostBSD live system initialization and desktop loading.
Features:
Changes:
system analysis, ZFS pool creation, and image decompression phases
console logo, and service monitoring
desktop variants (MATE, XFCE, Plasma, Gershwin)
The splash system gracefully degrades to verbose mode when:
Tested on: MATE, XFCE, Gershwin desktop variants
Summary by Sourcery
Add a comprehensive text-based boot splash subsystem and enhance the build workflow with colored logging, smarter resource checks, and improved error handling
New Features:
Bug Fixes:
Enhancements:
Documentation: