feat: move flatpak lists to brewflles #97
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the flatpak installation process to use Homebrew's brew bundle with Brewfiles, which is a positive change. However, the implementation introduces some significant issues in the justfile recipes. The toggle-devmode recipe contains flawed logic for detecting and disabling developer mode, which could fail for certain image names. More critically, the install-system-flatpaks recipe is broken and will not install development flatpaks when switching to developer mode. I have provided two review comments with high and critical severity to address these bugs, including code suggestions to resolve them.
| if grep -q -E -e "dx$" <<< "${IMAGE_NAME}" ; then | ||
| gum confirm "Would you like to disable developer mode?" || exit 0 | ||
| echo "Rebasing to a non developer image" | ||
| NEW_IMAGE="$(sed "s/\-dx//" <<< $CURRENT_IMAGE)" | ||
| pkexec bootc switch --enforce-container-sigpolicy "${NEW_IMAGE}" | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
The logic for detecting and disabling developer mode is flawed. The check grep -q -E -e "dx$" is too strict and will fail for image names where -dx is not the final suffix (e.g., aurora-dx-nvidia). This is a regression from the previous implementation. Additionally, using sed "s/\-dx//" to create the new image name is fragile and can produce incorrect names.
A more robust approach is to check for the presence of -dx anywhere in IMAGE_NAME and then perform a targeted replacement on IMAGE_NAME to generate the new base name, which is then used to construct the full NEW_IMAGE reference.
if [[ "${IMAGE_NAME}" == *"-dx"* ]] ; then
gum confirm "Would you like to disable developer mode?" || exit 0
echo "Rebasing to a non developer image"
NEW_IMAGE_NAME="$(echo "${IMAGE_NAME}" | sed 's/-dx//')"
NEW_IMAGE="$(sed "s/${IMAGE_NAME}/${NEW_IMAGE_NAME}/" <<< "${CURRENT_IMAGE}")"
pkexec bootc switch --enforce-container-sigpolicy "${NEW_IMAGE}"
exit 0
fi
inffy
requested review from
NiHaiden,
hanthor,
ledif and
renner0e
as code owners
|
Tested locally and the brewfiles are in the correct place and the install-system-flatpak commands seem to work. Would prefer another eyes and testing round for this though. |
|
What about ISOs? |
ISOs are still at ublue-os/aurora and this should make them ublue-os/aurora#1604 But we don't have testing ISO repo at the moment and we can't build new ISOs because of the bootc bug afaik |
|
think the flatpak installation is ok now. but moving nvidia DX is completely broken |
2 participants
Closes #84
From projectbluefin/common@ac474a8
Also needed for ISOs ublue-os/aurora#1604