detached 3ee469f

refactor(handlers): address CodeRabbit findings from first-boot fixes

commit
3ee469f8b48f6f153e0322058c1e91d97f9016a6
author
brice · · 2026-03-02 06:13 UTC
parent
e32ab76
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
5 files · +13 -7
Unified
UnifiedSplit
M install/mandatory/post-boot/all.sh +3 -3
@@ -82,11 +82,11 @@ fi
8282
8383 # Install modern CLI toolkit
8484 if [ -f /usr/local/share/archup/post-boot/cli-tools.sh ]; then
85- echo "Installing modern CLI toolkit..." | tee -a "$LOG_FILE"
85+ echo "Installing modern CLI toolkit..." >> "$LOG_FILE"
8686 if bash /usr/local/share/archup/post-boot/cli-tools.sh >> "$LOG_FILE" 2>&1; then
87- echo "[OK] CLI tools installed" | tee -a "$LOG_FILE"
87+ echo "[OK] CLI tools installed" >> "$LOG_FILE"
8888 else
89- echo "[KO] CLI tools installation failed (non-critical)" | tee -a "$LOG_FILE"
89+ echo "[KO] CLI tools installation failed (non-critical)" >> "$LOG_FILE"
9090 fi
9191 fi
9292
@@ -82,11 +82,11 @@ fi
82 82
83 # Install modern CLI toolkit83 # Install modern CLI toolkit
84 if [ -f /usr/local/share/archup/post-boot/cli-tools.sh ]; then84 if [ -f /usr/local/share/archup/post-boot/cli-tools.sh ]; then
85- echo "Installing modern CLI toolkit..." | tee -a "$LOG_FILE"85+ echo "Installing modern CLI toolkit..." >> "$LOG_FILE"
86 if bash /usr/local/share/archup/post-boot/cli-tools.sh >> "$LOG_FILE" 2>&1; then86 if bash /usr/local/share/archup/post-boot/cli-tools.sh >> "$LOG_FILE" 2>&1; then
87- echo "[OK] CLI tools installed" | tee -a "$LOG_FILE"87+ echo "[OK] CLI tools installed" >> "$LOG_FILE"
88 else88 else
89- echo "[KO] CLI tools installation failed (non-critical)" | tee -a "$LOG_FILE"89+ echo "[KO] CLI tools installation failed (non-critical)" >> "$LOG_FILE"
90 fi90 fi
91 fi91 fi
92 92
M internal/application/handlers/bootloader_handler.go +2 -1
@@ -2,6 +2,7 @@ package handlers
22
33 import (
44 "context"
5+ "errors"
56 "fmt"
67 "os"
78 "path/filepath"
@@ -232,7 +233,7 @@ func (h *BootloaderHandler) configureLimine(ctx context.Context, cmd commands.In
232233 "\n //%s-fallback\n protocol: linux\n path: boot():/vmlinuz-%s\n cmdline: %s\n module_path: boot():/initramfs-%s-fallback.img\n",
233234 kernelName, kernelName, kernelParams, kernelName,
234235 )
235- } else if !os.IsNotExist(err) {
236+ } else if !errors.Is(err, os.ErrNotExist) {
236237 h.logger.Warn("Could not stat fallback initramfs, omitting fallback entry", "path", fallbackImgPath, "error", err)
237238 }
238239 limineConfig = strings.ReplaceAll(limineConfig, "{{FALLBACK_ENTRY}}", fallbackEntry)
@@ -2,6 +2,7 @@ package handlers
2 2
3 import (3 import (
4 "context"4 "context"
5+ "errors"
5 "fmt"6 "fmt"
6 "os"7 "os"
7 "path/filepath"8 "path/filepath"
@@ -232,7 +233,7 @@ func (h *BootloaderHandler) configureLimine(ctx context.Context, cmd commands.In
232 "\n //%s-fallback\n protocol: linux\n path: boot():/vmlinuz-%s\n cmdline: %s\n module_path: boot():/initramfs-%s-fallback.img\n",233 "\n //%s-fallback\n protocol: linux\n path: boot():/vmlinuz-%s\n cmdline: %s\n module_path: boot():/initramfs-%s-fallback.img\n",
233 kernelName, kernelName, kernelParams, kernelName,234 kernelName, kernelName, kernelParams, kernelName,
234 )235 )
235- } else if !os.IsNotExist(err) {236+ } else if !errors.Is(err, os.ErrNotExist) {
236 h.logger.Warn("Could not stat fallback initramfs, omitting fallback entry", "path", fallbackImgPath, "error", err)237 h.logger.Warn("Could not stat fallback initramfs, omitting fallback entry", "path", fallbackImgPath, "error", err)
237 }238 }
238 limineConfig = strings.ReplaceAll(limineConfig, "{{FALLBACK_ENTRY}}", fallbackEntry)239 limineConfig = strings.ReplaceAll(limineConfig, "{{FALLBACK_ENTRY}}", fallbackEntry)
M internal/application/handlers/bootloader_handler_test.go +4 -3
@@ -46,7 +46,8 @@ func setupCommonMocks(mockFS *mocks.MockFileSystem, mockExec *mocks.MockCommandE
4646 mockExec.EXPECT().Execute(gomock.Any(), "blkid", "-s", "UUID", "-o", "value", gomock.Any()).Return([]byte("uuid"), nil).AnyTimes()
4747 mockExec.EXPECT().Execute(gomock.Any(), "cp", gomock.Any(), gomock.Any()).Return([]byte{}, nil).AnyTimes()
4848 mockChrExec.EXPECT().ExecuteInChroot(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{}, nil).AnyTimes()
49- mockChrExec.EXPECT().ExecuteInChroot(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{}, nil).AnyTimes()
49+ // efibootmgr and other multi-arg chroot commands: match any number of trailing args
50+ mockChrExec.EXPECT().ExecuteInChroot(gomock.Any(), gomock.Any(), "efibootmgr", gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{}, nil).AnyTimes()
5051 }
5152
5253 func TestBootloaderHandler_Handle_Limine(t *testing.T) {
@@ -136,7 +137,7 @@ func TestConfigureLimine_FallbackAbsent(t *testing.T) {
136137 // Capture the written config
137138 var writtenConfig string
138139 mockFS.EXPECT().WriteFile(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
139- func(path string, data []byte, perm interface{}) error {
140+ func(path string, data []byte, perm os.FileMode) error {
140141 if strings.HasSuffix(path, "limine.conf") {
141142 writtenConfig = string(data)
142143 }
@@ -200,7 +201,7 @@ func TestConfigureLimine_FallbackPresent(t *testing.T) {
200201 // Capture the written config
201202 var writtenConfig string
202203 mockFS.EXPECT().WriteFile(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
203- func(path string, data []byte, perm interface{}) error {
204+ func(path string, data []byte, perm os.FileMode) error {
204205 if strings.HasSuffix(path, "limine.conf") {
205206 writtenConfig = string(data)
206207 }
@@ -46,7 +46,8 @@ func setupCommonMocks(mockFS *mocks.MockFileSystem, mockExec *mocks.MockCommandE
46 mockExec.EXPECT().Execute(gomock.Any(), "blkid", "-s", "UUID", "-o", "value", gomock.Any()).Return([]byte("uuid"), nil).AnyTimes()46 mockExec.EXPECT().Execute(gomock.Any(), "blkid", "-s", "UUID", "-o", "value", gomock.Any()).Return([]byte("uuid"), nil).AnyTimes()
47 mockExec.EXPECT().Execute(gomock.Any(), "cp", gomock.Any(), gomock.Any()).Return([]byte{}, nil).AnyTimes()47 mockExec.EXPECT().Execute(gomock.Any(), "cp", gomock.Any(), gomock.Any()).Return([]byte{}, nil).AnyTimes()
48 mockChrExec.EXPECT().ExecuteInChroot(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{}, nil).AnyTimes()48 mockChrExec.EXPECT().ExecuteInChroot(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{}, nil).AnyTimes()
49- mockChrExec.EXPECT().ExecuteInChroot(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{}, nil).AnyTimes()49+ // efibootmgr and other multi-arg chroot commands: match any number of trailing args
50+ mockChrExec.EXPECT().ExecuteInChroot(gomock.Any(), gomock.Any(), "efibootmgr", gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{}, nil).AnyTimes()
50 }51 }
51 52
52 func TestBootloaderHandler_Handle_Limine(t *testing.T) {53 func TestBootloaderHandler_Handle_Limine(t *testing.T) {
@@ -136,7 +137,7 @@ func TestConfigureLimine_FallbackAbsent(t *testing.T) {
136 // Capture the written config137 // Capture the written config
137 var writtenConfig string138 var writtenConfig string
138 mockFS.EXPECT().WriteFile(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(139 mockFS.EXPECT().WriteFile(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
139- func(path string, data []byte, perm interface{}) error {140+ func(path string, data []byte, perm os.FileMode) error {
140 if strings.HasSuffix(path, "limine.conf") {141 if strings.HasSuffix(path, "limine.conf") {
141 writtenConfig = string(data)142 writtenConfig = string(data)
142 }143 }
@@ -200,7 +201,7 @@ func TestConfigureLimine_FallbackPresent(t *testing.T) {
200 // Capture the written config201 // Capture the written config
201 var writtenConfig string202 var writtenConfig string
202 mockFS.EXPECT().WriteFile(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(203 mockFS.EXPECT().WriteFile(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(
203- func(path string, data []byte, perm interface{}) error {204+ func(path string, data []byte, perm os.FileMode) error {
204 if strings.HasSuffix(path, "limine.conf") {205 if strings.HasSuffix(path, "limine.conf") {
205 writtenConfig = string(data)206 writtenConfig = string(data)
206 }207 }
M internal/application/handlers/post_install_handler.go +3 -0
@@ -255,6 +255,9 @@ func (h *PostInstallHandler) sanitizeLimineSnapperSyncConfig(mountPoint string)
255255 const limineDiskPlaceholder = "DISK_PLACEHOLDER"
256256
257257 func (h *PostInstallHandler) installLimineHook(mountPoint, targetDisk string) error {
258+ if targetDisk == "" {
259+ return fmt.Errorf("target disk is required for limine hook installation")
260+ }
258261 hooksDir := filepath.Join(mountPoint, "etc", "pacman.d", "hooks")
259262 if err := h.fs.MkdirAll(hooksDir, 0755); err != nil {
260263 return fmt.Errorf("failed to create hooks dir: %w", err)
@@ -255,6 +255,9 @@ func (h *PostInstallHandler) sanitizeLimineSnapperSyncConfig(mountPoint string)
255 const limineDiskPlaceholder = "DISK_PLACEHOLDER"255 const limineDiskPlaceholder = "DISK_PLACEHOLDER"
256 256
257 func (h *PostInstallHandler) installLimineHook(mountPoint, targetDisk string) error {257 func (h *PostInstallHandler) installLimineHook(mountPoint, targetDisk string) error {
258+ if targetDisk == "" {
259+ return fmt.Errorf("target disk is required for limine hook installation")
260+ }
258 hooksDir := filepath.Join(mountPoint, "etc", "pacman.d", "hooks")261 hooksDir := filepath.Join(mountPoint, "etc", "pacman.d", "hooks")
259 if err := h.fs.MkdirAll(hooksDir, 0755); err != nil {262 if err := h.fs.MkdirAll(hooksDir, 0755); err != nil {
260 return fmt.Errorf("failed to create hooks dir: %w", err)263 return fmt.Errorf("failed to create hooks dir: %w", err)
M internal/application/services/installation_service_test.go +1 -0
@@ -35,6 +35,7 @@ func createTestService(ctrl *gomock.Controller) *InstallationService {
3535 mockLogger.EXPECT().Info(gomock.Any()).AnyTimes()
3636 mockLogger.EXPECT().Info(gomock.Any(), gomock.Any()).AnyTimes()
3737 mockLogger.EXPECT().Warn(gomock.Any()).AnyTimes()
38+ mockLogger.EXPECT().Warn(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
3839 mockLogger.EXPECT().Error(gomock.Any(), gomock.Any()).AnyTimes()
3940 mockLogger.EXPECT().LogPath().Return("/var/log/archup-install.log").AnyTimes()
4041 mockExec.EXPECT().Execute(gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{}, nil).AnyTimes()
@@ -35,6 +35,7 @@ func createTestService(ctrl *gomock.Controller) *InstallationService {
35 mockLogger.EXPECT().Info(gomock.Any()).AnyTimes()35 mockLogger.EXPECT().Info(gomock.Any()).AnyTimes()
36 mockLogger.EXPECT().Info(gomock.Any(), gomock.Any()).AnyTimes()36 mockLogger.EXPECT().Info(gomock.Any(), gomock.Any()).AnyTimes()
37 mockLogger.EXPECT().Warn(gomock.Any()).AnyTimes()37 mockLogger.EXPECT().Warn(gomock.Any()).AnyTimes()
38+ mockLogger.EXPECT().Warn(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
38 mockLogger.EXPECT().Error(gomock.Any(), gomock.Any()).AnyTimes()39 mockLogger.EXPECT().Error(gomock.Any(), gomock.Any()).AnyTimes()
39 mockLogger.EXPECT().LogPath().Return("/var/log/archup-install.log").AnyTimes()40 mockLogger.EXPECT().LogPath().Return("/var/log/archup-install.log").AnyTimes()
40 mockExec.EXPECT().Execute(gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{}, nil).AnyTimes()41 mockExec.EXPECT().Execute(gomock.Any(), gomock.Any(), gomock.Any()).Return([]byte{}, nil).AnyTimes()

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 1

Tags 0

    ↑↓ moveenter openesc close