diff --git a/.github/scripts/tests/test_pbs_cli_localization.py b/.github/scripts/tests/test_pbs_cli_localization.py index bda1064b..618395e2 100644 --- a/.github/scripts/tests/test_pbs_cli_localization.py +++ b/.github/scripts/tests/test_pbs_cli_localization.py @@ -167,7 +167,7 @@ class UpdateCliTests(unittest.TestCase): with tempfile.TemporaryDirectory(prefix='pbs-update-i18n-') as folder: body=function('scripts/global/update-pbs.sh','update_pbs') for old,new in [('/var/log/',folder+'/log/'),('/tmp/proxmenux_screen_capture_',folder+'/tmp/proxmenux_screen_capture_'),('/var/run/',folder+'/run/'),('/boot/',folder+'/boot/')]: body=body.replace(old,new) - script=UPDATE_HARNESS+function('scripts/utils.sh','translate')+body+'\nupdate_pbs\n' + script=UPDATE_HARNESS+function('scripts/utils.sh','translate')+function('scripts/global/common-functions.sh','pmx_confirm_update')+body+'\nupdate_pbs\n' result=subprocess.run(['bash','-c',script],capture_output=True,text=True,timeout=15, env={**os.environ,'REPO':str(ROOT),'FIXTURE':folder,'CASE':case,'LANGUAGE':language,'SUITE':suite}) files={str(p.relative_to(folder)):p.read_text() for p in Path(folder).rglob('*') if p.is_file()} diff --git a/lang/de.json b/lang/de.json index dd7c06f0..d0862bcb 100644 --- a/lang/de.json +++ b/lang/de.json @@ -8601,6 +8601,7 @@ "missing": "fehlt", "mkfs.btrfs not found. Install btrfs-progs and retry.": "mkfs.btrfs nicht gefunden. Installieren Sie btrfs-progs und versuchen Sie es erneut.", "more": "mehr", + "more packages; the full list is in the update log": "weitere Pakete; die vollständige Liste steht im Update-Protokoll", "motionEye web interface": "motionEye Web-Schnittstelle", "mount.cifs command not found after installation.": "Der Befehl mount.cifs wurde nach der Installation nicht gefunden.", "mount.nfs command not found after installation.": "Der Befehl mount.nfs wurde nach der Installation nicht gefunden.", diff --git a/lang/es.json b/lang/es.json index b4e4673d..2ef2f932 100644 --- a/lang/es.json +++ b/lang/es.json @@ -8574,6 +8574,7 @@ "missing": "faltan", "mkfs.btrfs not found. Install btrfs-progs and retry.": "mkfs.btrfs no encontrado. Instala btrfs-progs y vuelve a intentarlo.", "more": "más", + "more packages; the full list is in the update log": "paquetes más; la lista completa está en el registro de la actualización", "motionEye web interface": "Interfaz web de motionEye", "mount.cifs command not found after installation.": "El comando mount.cifs no se encuentra después de la instalación.", "mount.nfs command not found after installation.": "El comando mount.nfs no se encuentra después de la instalación.", diff --git a/lang/fr.json b/lang/fr.json index 4a206da6..11a30e42 100644 --- a/lang/fr.json +++ b/lang/fr.json @@ -8600,6 +8600,7 @@ "missing": "manquant", "mkfs.btrfs not found. Install btrfs-progs and retry.": "mkfs.btrfs introuvable. Installez btrfs-progs et réessayez.", "more": "plus", + "more packages; the full list is in the update log": "paquets de plus ; la liste complète se trouve dans le journal de la mise à jour", "motionEye web interface": "Interface web motionEye", "mount.cifs command not found after installation.": "Commande mount.cifs introuvable après l'installation.", "mount.nfs command not found after installation.": "Commande mount.nfs introuvable après l'installation.", diff --git a/lang/it.json b/lang/it.json index 77ba9116..2edf9026 100644 --- a/lang/it.json +++ b/lang/it.json @@ -8601,6 +8601,7 @@ "missing": "mancanti", "mkfs.btrfs not found. Install btrfs-progs and retry.": "mkfs.btrfs non trovato. Installare btrfs-progs e riprovare.", "more": "in più", + "more packages; the full list is in the update log": "pacchetti in più; l'elenco completo è nel log dell'aggiornamento", "motionEye web interface": "Interfaccia web motionEye", "mount.cifs command not found after installation.": "Comando mount.cifs non trovato dopo l'installazione.", "mount.nfs command not found after installation.": "Comando mount.nfs non trovato dopo l'installazione.", diff --git a/lang/pt.json b/lang/pt.json index e3f4005f..0463f27f 100644 --- a/lang/pt.json +++ b/lang/pt.json @@ -8601,6 +8601,7 @@ "missing": "faltando", "mkfs.btrfs not found. Install btrfs-progs and retry.": "mkfs.btrfs não encontrado. Instale o btrfs-progs e tente novamente.", "more": "mais", + "more packages; the full list is in the update log": "pacotes a mais; a lista completa está no registo da atualização", "motionEye web interface": "Interface Web motionEye", "mount.cifs command not found after installation.": "Comando mount.cifs não encontrado após a instalação.", "mount.nfs command not found after installation.": "Comando mount.nfs não encontrado após a instalação.", diff --git a/lang/sk.json b/lang/sk.json index 0a86db13..d60b5588 100644 --- a/lang/sk.json +++ b/lang/sk.json @@ -8602,6 +8602,7 @@ "missing": "chýba", "mkfs.btrfs not found. Install btrfs-progs and retry.": "mkfs.btrfs sa nenašiel. Nainštalujte btrfs-progs a skúste to znova.", "more": "ďalších", + "more packages; the full list is in the update log": "ďalších balíkov; úplný zoznam je v zázname aktualizácie", "motionEye web interface": "Webové rozhranie motionEye", "mount.cifs command not found after installation.": "Príkaz mount.cifs sa po inštalácii nenašiel.", "mount.nfs command not found after installation.": "Príkaz mount.nfs sa po inštalácii nenašiel.", diff --git a/lang/sv.json b/lang/sv.json index 75d910a9..6c3e04e8 100644 --- a/lang/sv.json +++ b/lang/sv.json @@ -8601,6 +8601,7 @@ "missing": "saknad", "mkfs.btrfs not found. Install btrfs-progs and retry.": "mkfs.btrfs hittades inte. Installera btrfs-progs och försök igen.", "more": "mer", + "more packages; the full list is in the update log": "paket till; hela listan finns i uppdateringsloggen", "motionEye web interface": "motionEye webbgränssnitt", "mount.cifs command not found after installation.": "mount.cifs-kommandot hittades inte efter installationen.", "mount.nfs command not found after installation.": "mount.nfs-kommandot hittades inte efter installationen.", diff --git a/scripts/global/common-functions.sh b/scripts/global/common-functions.sh index dd95e9bf..da7a058d 100644 --- a/scripts/global/common-functions.sh +++ b/scripts/global/common-functions.sh @@ -445,19 +445,68 @@ cleanup_duplicate_repos() { # an operator reads the log days later to find out what moved. # Reads `apt list --upgradable` on stdin and writes one aligned -# `name old → new` line per package. Lines that do not carry the +# `name old → new` line per package, never wider than the dialog that +# shows it (70 columns by default): a package whose versions do not fit +# continues on indented lines. Lines that do not carry the # "[upgradable from: …]" part are passed through as they came. pmx_format_upgradable() { - awk 'NF == 0 { next } + awk -v width="${1:-70}" 'NF == 0 { next } $0 ~ /\[upgradable from: / { name = $1; sub(/\/.*/, "", name) from = $6; sub(/\]$/, "", from) - printf " %-30s %s → %s\n", name, from, $2 + # Widths are counted by hand: " → " is three columns but more + # bytes, and mawk measures bytes. + if (2 + 30 + 1 + length(from) + 3 + length($2) <= width && length(name) <= 30) + printf " %-30s %s → %s\n", name, from, $2 + else if (6 + length(from) + 3 + length($2) <= width) + printf " %s\n %s → %s\n", name, from, $2 + else + printf " %s\n %s\n → %s\n", name, from, $2 next } { print " " $0 }' } +# Asks whether to go ahead with an update, with Yes focused so Enter +# confirms. whiptail cannot focus a button while its text scrolls, so the +# box takes the height of the terminal and a list that does not fit ends +# with a line counting the packages left out; the update log keeps them all. +# +# pmx_confirm_update TITLE TEXT LIST QUESTION +pmx_confirm_update() { + local title="$1" text="$2" list="$3" question="$4" + local rows height room shown more + rows=$(tput lines 2>/dev/null) + [[ "$rows" =~ ^[0-9]+$ ]] || rows=24 + height=$(( rows > 50 ? 48 : rows - 2 )) + (( height < 16 )) && height=16 + # Frame, padding, the heading, the question and the buttons. + room=$(( height - 9 - $(printf '%b' "$text" | grep -c '') )) + (( room < 3 )) && room=3 + + # A package that spans several lines is shown whole or counted. + shown=$(printf '%s\n' "$list" | awk -v room="$room" ' + /^ [^ ]/ { package++ } + { lines[NR] = $0; owner[NR] = package } + END { + if (NR <= room) { for (i = 1; i <= NR; i++) print lines[i]; exit } + # One line is kept for the count of the packages left out. + cap = room - 1 + last = owner[cap] + if (owner[cap + 1] == last) last-- + for (i = 1; i <= NR && owner[i] <= last; i++) print lines[i] + printf "\n%d\n", package - last + }') + more="${shown##*$'\n'}" + if [[ "$shown" == *$'\n\n'"$more" && "$more" =~ ^[0-9]+$ ]]; then + shown="${shown%$'\n\n'"$more"}" + shown+="\n … ${more} $(translate "more packages; the full list is in the update log")" + fi + + whiptail --title "$title" --yesno \ + "${text}$(translate "Packages to be upgraded"):\n${shown}\n\n${question}" "$height" 78 +} + # Writes what dpkg actually did since `$1` (a "YYYY-MM-DD HH:MM:SS" # stamp taken before the upgrade). dpkg's own log is the authoritative # record of the transaction — it carries both versions of every package diff --git a/scripts/global/update-pbs.sh b/scripts/global/update-pbs.sh index 80200ba7..ac77c810 100644 --- a/scripts/global/update-pbs.sh +++ b/scripts/global/update-pbs.sh @@ -110,10 +110,9 @@ update_pbs() { return 0 fi - menu_text+="$(translate "Packages to be upgraded"):\n$upgradable_list\n\n" - menu_text+="$(translate "Do you want to proceed with the system update?")" local proceed=0 - whiptail --title "$(translate "Proxmox Backup Server Update")" --scrolltext --yesno "$menu_text" 24 78 || proceed=1 + pmx_confirm_update "$(translate "Proxmox Backup Server Update")" "$menu_text" "$upgradable_list" \ + "$(translate "Do you want to proceed with the system update?")" || proceed=1 clear show_proxmenux_logo diff --git a/scripts/global/update-pve-safe.sh b/scripts/global/update-pve-safe.sh index c9056fdd..3896c177 100644 --- a/scripts/global/update-pve-safe.sh +++ b/scripts/global/update-pve-safe.sh @@ -205,12 +205,9 @@ update_pve_safe() { fi # The package list rides in the same dialog as the summary, so the - # decision is taken knowing what is about to be replaced. Keep Yes focused - # while the list remains scrollable; --keep-tite restores the execution screen. - menu_text+="$(translate "Packages to be upgraded"):\n$upgradable_list\n\n" - menu_text+="$(translate "Do you want to proceed with the system update?")" - if ! dialog --keep-tite --title "$(translate "Proxmox Update")" \ - --default-button yes --no-collapse --scrollbar --yesno "$menu_text" 24 78; then + # decision is taken knowing what is about to be replaced. + if ! pmx_confirm_update "$(translate "Proxmox Update")" "$menu_text" "$upgradable_list" \ + "$(translate "Do you want to proceed with the system update?")"; then msg_info2 "$(translate "Update cancelled by user")" apt-get -y autoremove >/dev/null 2>&1 || true apt-get -y autoclean >/dev/null 2>&1 || true diff --git a/scripts/global/update-pve8.sh b/scripts/global/update-pve8.sh index 01503294..df47bd98 100644 --- a/scripts/global/update-pve8.sh +++ b/scripts/global/update-pve8.sh @@ -169,12 +169,8 @@ EOF else # The package list rides in the same dialog as the summary, so # the decision is taken knowing what is about to be replaced. - # Keep Yes focused while the full package list remains scrollable. - # --keep-tite restores the execution screen after confirmation. - menu_text+="$(translate "Packages to be upgraded"):\n$upgradable_list\n\n" - menu_text+="$(translate "Do you want to proceed with the system update?")" - if dialog --keep-tite --title "$(translate "Proxmox Update")" \ - --default-button yes --no-collapse --scrollbar --yesno "$menu_text" 24 78; then + if pmx_confirm_update "$(translate "Proxmox Update")" "$menu_text" "$upgradable_list" \ + "$(translate "Do you want to proceed with the system update?")"; then return 0 else return 1 diff --git a/scripts/global/update-pve9_2.sh b/scripts/global/update-pve9_2.sh index 6f01caff..2cd3e21f 100644 --- a/scripts/global/update-pve9_2.sh +++ b/scripts/global/update-pve9_2.sh @@ -272,12 +272,8 @@ EOF else # The package list rides in the same dialog as the summary, so # the decision is taken knowing what is about to be replaced. - # Keep Yes focused while the full package list remains scrollable. - # --keep-tite restores the execution screen after confirmation. - menu_text+="$(translate "Packages to be upgraded"):\n$upgradable_list\n\n" - menu_text+="$(translate "Do you want to proceed with the system update?")" - if dialog --keep-tite --title "$(translate "Proxmox Update")" \ - --default-button yes --no-collapse --scrollbar --yesno "$menu_text" 24 78; then + if pmx_confirm_update "$(translate "Proxmox Update")" "$menu_text" "$upgradable_list" \ + "$(translate "Do you want to proceed with the system update?")"; then return 0 else return 1 diff --git a/tests/test_update_confirmation.py b/tests/test_update_confirmation.py index 36281fd4..0acb3364 100644 --- a/tests/test_update_confirmation.py +++ b/tests/test_update_confirmation.py @@ -1,7 +1,7 @@ """Exercise only the update confirmation widget in a disposable terminal. -No update script is sourced or executed: the isolated UI command receives -synthetic package names. Requires dialog; skips on systems without it. +No update script is sourced or executed: the shared confirmation function +receives synthetic package names. Requires whiptail; skips without it. """ import errno @@ -27,17 +27,23 @@ UPDATERS = ( ) -def confirmation_command(name): - source = (ROOT / "scripts/global" / name).read_text() - match = re.search( - r'^\s*if (?:! )?((?:dialog|whiptail) --(?:keep-tite --)?title ' - r'"\$\(translate "Proxmox Update"\)"[^;]+); then', - source, - re.MULTILINE, - ) +COMMON = ROOT / "scripts/global/common-functions.sh" + + +def confirmation_function(): + match = re.search(r"^pmx_confirm_update\(\) \{\n.*?^\}\n", COMMON.read_text(), + re.MULTILINE | re.DOTALL) if not match: - raise AssertionError("Update confirmation not found: " + name) - return match.group(1) + raise AssertionError("pmx_confirm_update not found") + return match.group(0) + + +def confirmation_command(): + """The shared confirmation as an updater calls it; the package list comes + from UPDATE_CONFIRMATION_TEXT, formatted as pmx_format_upgradable writes it.""" + return (confirmation_function() + 'pmx_confirm_update "Proxmox Update" ' + '"System Update Information\\n\\n" "$UPDATE_CONFIRMATION_TEXT" ' + '"Do you want to proceed with the system update?"') def run_widget(command, text, keys): @@ -50,8 +56,7 @@ def run_widget(command, text, keys): for key in list(environment): if key.startswith("DIALOG_"): environment.pop(key) - script = ('translate() { printf "%s" "$1"; }\n' - 'menu_text=$UPDATE_CONFIRMATION_TEXT\n' + command) + script = 'translate() { printf "%s" "$1"; }\n' + command os.execve("/bin/bash", ["bash", "-c", script], environment) output = bytearray() @@ -90,36 +95,47 @@ def run_widget(command, text, keys): os.close(terminal) -@unittest.skipUnless(shutil.which("dialog"), "dialog is not installed") +def packages(count, long_every=0): + """Formatted list lines; every `long_every`-th package spans three lines.""" + lines = [] + for i in range(count): + if long_every and i % long_every == 0: + lines += [" libsamba-{:03d}".format(i), " 2:2.11.0+samba4.22.10+dfsg-0+deb13u1", + " → 2:2.11.0+samba4.22.11+dfsg-0+deb13u1"] + else: + lines.append(" package-{:03d} 1.0 → 2.0".format(i)) + return "\n".join(lines) + + +@unittest.skipUnless(shutil.which("whiptail"), "whiptail is not installed") class UpdateConfirmationTest(unittest.TestCase): def test_enter_confirms_yes_without_tab_for_short_and_long_lists(self): - for updater in UPDATERS: - for count in (1, 80): - with self.subTest(updater=updater, packages=count): - text = "System Update Information\\n\\n" + "\\n".join( - " package-{:02d} 1.0 -> 2.0".format(i) - for i in range(count) - ) + "\\n\\nDo you want to proceed with the system update?" - status, output = run_widget(confirmation_command(updater), text, b"\r") - self.assertEqual(status, 0, output) + for count in (1, 80, 200): + with self.subTest(packages=count): + status, output = run_widget(confirmation_command(), packages(count, 7), b"\r") + self.assertEqual(status, 0, output) def test_tab_can_select_no_and_escape_can_cancel(self): - for updater in UPDATERS: - for keys, expected in ((b"\t\r", 1), (b"\x1b", 255)): - with self.subTest(updater=updater, keys=keys): - status, output = run_widget(confirmation_command(updater), "Confirm update?", keys) - self.assertEqual(status, expected, output) + for keys, expected in ((b"\t\r", 1), (b"\x1b", 255)): + with self.subTest(keys=keys): + status, output = run_widget(confirmation_command(), packages(80), keys) + self.assertEqual(status, expected, output) - def test_full_list_remains_scrollable_with_yes_focused(self): - text = "\\n".join("package-{:03d}".format(i) for i in range(80)) - for updater in UPDATERS: + def test_a_long_list_fits_and_counts_the_packages_left_out(self): + status, output = run_widget(confirmation_command(), packages(200, 7), b"\r") + self.assertEqual(status, 0, output) + plain = re.sub(rb"\x1b(?:\[[0-?]*[ -/]*[@-~]|[()][A-Za-z0-9]|.)", b"", output) + self.assertIn(b"more packages; the full list is in the update log", plain) + # A package is shown whole or counted, never cut after its name. + self.assertNotRegex(plain, rb"libsamba-\d{3}\s+\.\.\.") + + def test_every_updater_asks_through_the_shared_confirmation(self): + for updater in UPDATERS + ("update-pbs.sh",): with self.subTest(updater=updater): - # G scrolls to the end of the yes/no text without moving focus. - status, output = run_widget(confirmation_command(updater), text, b"G\r") - self.assertEqual(status, 0, output) - # Curses may repaint only changed digits rather than emit a - # complete final line. Its 100% indicator confirms the end. - self.assertIn(b"100%", output) + source = (ROOT / "scripts/global" / updater).read_text() + self.assertIn("pmx_confirm_update ", source) + self.assertNotIn("--scrolltext --yesno", source) + self.assertNotIn("dialog --keep-tite", source) if __name__ == "__main__":