Fix xlibre build option from not finishing and other various things #235
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR streamlines the build process by removing obsolete logging and fixing build-type option parsing in build.sh, and updates package manifests to add XLibre ATI Radeon driver, integrate Ly login manager for theming, and replace VLC with MPV for better 60 fps support. Entity relationship diagram for updated package manifestserDiagram
COMMON_PACKAGES {
string name
string version
string description
}
DRIVERS_PACKAGES {
string name
string version
string description
}
COMMON_PACKAGES ||--o| DRIVERS_PACKAGES : includes
MPV {
string type
}
VLC {
string type
}
Ly_LOGIN_MANAGER {
string type
}
XLIBRE_ATI_RADEON_DRIVER {
string type
}
COMMON_PACKAGES ||--o| MPV : has
COMMON_PACKAGES ||--o| VLC : has
COMMON_PACKAGES ||--o| Ly_LOGIN_MANAGER : has
DRIVERS_PACKAGES ||--o| XLIBRE_ATI_RADEON_DRIVER : has
Class diagram for updated build.sh functions and structureclassDiagram
class build.sh {
- logging function removed
+ improved build-type option parsing
+ streamlined package URL selection
}
build.sh : set_ghostbsd_version()
build.sh : fetch_x_drivers_packages()
build.sh : main()
File-Level Changes
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 there - I've reviewed your changes - here's some feedback:
- The case for build_type "testing" was removed from fetch_x_drivers_packages, so testing builds will now fall back to the unstable URL—please reintroduce or explicitly handle testing if it’s still supported.
- You’ve removed the log() function entirely but the build script still uses log calls elsewhere—either replace them with echo or remove those calls to avoid undefined function errors.
- The PR description mentions adding XLibre driver, Ly login manager, and MPV, but the diff only shows logging and build-type changes—please include the actual package changes or update the description to match.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The case for build_type "testing" was removed from fetch_x_drivers_packages, so testing builds will now fall back to the unstable URL—please reintroduce or explicitly handle testing if it’s still supported.
- You’ve removed the log() function entirely but the build script still uses log calls elsewhere—either replace them with echo or remove those calls to avoid undefined function errors.
- The PR description mentions adding XLibre driver, Ly login manager, and MPV, but the diff only shows logging and build-type changes—please include the actual package changes or update the description to match.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
This file is fine in master. Please don't delete the code from it.
There was a problem hiding this comment.
i was on a vm and my laptop and it fails to build inside of ghostbsd unless i remove the log commands. i needed this to build ISO files for my setup. you can verify in testing. It should work but it is missing from the distro contained on the ISO you would have to add the pkg to the list then it works.
| wifi-firmware-rtw88-kmod | ||
| xlibre-xf86-input-evdev | ||
| xlibre-xf86-input-joystick | ||
| xlibre-xf86-input-keyboard | ||
| xlibre-xf86-input-mouse | ||
| xlibre-xf86-input-synaptics | ||
| xlibre-xf86-input-vmmouse | ||
| xlibre-xf86-video-scfb | ||
| xlibre-xf86-video-vesa | ||
| xlibre-xf86-video-ati |
There was a problem hiding this comment.
There is an xlibre branch for Xlibre.
There was a problem hiding this comment.
could xf86-video-ati be added for my laptop and others to stay working? older AMD fare needs this or it crashes to prompt also adding some more rt88 or old wifi by their name would be excellent for further compat. amdgpu loads all radeonkms modules i found out but it still needs that to be added for that to work.
also is it possible to open the jenkins on the unstable xlibre branch instead of stable it works great so far and that branch has the latest KDE and XFCE for playing steam games
There was a problem hiding this comment.
wifi-firmware-rtw88-kmod is fine to add, and probably xf86-video-ati.
| xorg-minimal | ||
| zip | ||
| ly | ||
| mpv |
There was a problem hiding this comment.
Changing to MPV should be a request ticket for the community to vote on.
I do not want Ly to be part of the default installation.
There was a problem hiding this comment.
ly is so pretty with colormix is it possible to add a build option to use ly if i do all the work? obviously not everyone wants textmode login like that but it should be an option for when drivers arent working for fun.
There was a problem hiding this comment.
To be clear, the more packages that get added, the more RAM the system needs to run in live.
github-project-automation
bot
moved this from In Review
to In Progress
in Development Management
2 participants
While doing the first option with my build.sh. I also took it upon myself to add XLibre ati radeon driver and Ly login manager for better theming. I replaced VLC with MPV to better support the SVP 4 project vlc doesnt work great with 60fps anime
Summary by Sourcery
Fix the xlibre build script to prevent it from hanging, streamline its logic, and update default packages by adding ATI Radeon driver support, integrating the ly login manager, and replacing VLC with MPV for better performance.
New Features:
Bug Fixes:
Enhancements: