[installer] ENTER in guided partitioner destroys partition table

OpenSubmitted by Tobias Geerinckx-Rice.
Details
3 participants
  • Ludovic Courtès
  • Tobias Geerinckx-Rice
  • Mathieu Othacehe
Owner
unassigned
Severity
important
T
T
Tobias Geerinckx-Rice wrote on 16 Aug 01:05 +0200
(name . Bug reports for GNU Guix)(address . bug-guix@gnu.org)
87zgg5f6el.fsf@nckx
Hi all,

Whilst testing the graphical installer, I selected ‘guided
partitioning’ and pressed ENTER on the main drive (sda) to see
what that does.

What that does is immediately and without confirmation wipe the
on-disc partition table. And its back-up.

Testdisk is great,

T G-R
-----BEGIN PGP SIGNATURE-----

iIMEARYKACsWIQT12iAyS4c9C3o4dnINsP+IT1VteQUCYvrS0g0cbWVAdG9iaWFz
LmdyAAoJEA2w/4hPVW155zkBANCuJZOLhCmTze41p3MlDWMAaxz3zI4b4FAuHB9V
VK9VAP9RKtVp4WeCoAnPY9OIy21kTknFSt2zyS9D4v5acf7WDQ==
=q9w4
-----END PGP SIGNATURE-----

M
M
Mathieu Othacehe wrote on 16 Aug 10:51 +0200
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)(address . 57232@debbugs.gnu.org)
87y1voefkw.fsf@gnu.org
Hey Tobias,

Toggle quote (3 lines)
> What that does is immediately and without confirmation wipe the on-disc
> partition table. And its back-up.

Oops, glad you were able to recover, I was also bitten in the past. The
attached patch adds an extra confirmation page before wiping everything,
WDYT?

Mathieu
From 4a9c1fb1fe7f9a65b2b7d1f9e4419b1d28a8082e Mon Sep 17 00:00:00 2001
From: Mathieu Othacehe <othacehe@gnu.org>
Date: Tue, 16 Aug 2022 10:49:07 +0200
Subject: [PATCH 1/1] installer: partition: Add a confirmation page before
formatting.


* gnu/installer/newt/partition.scm (run-label-confirmation-page): New
procedure.
(run-label-page): Call the above procedure before proceeding.
---
gnu/installer/newt/partition.scm | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)

Toggle diff (38 lines)
diff --git a/gnu/installer/newt/partition.scm b/gnu/installer/newt/partition.scm
index e7a97810ac..f11a644f92 100644
--- a/gnu/installer/newt/partition.scm
+++ b/gnu/installer/newt/partition.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2018, 2019 Mathieu Othacehe <m.othacehe@gmail.com>
+;;; Copyright © 2018, 2019, 2022 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2019, 2020 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2020 Tobias Geerinckx-Rice <me@tobias.gr>
 ;;;
@@ -92,6 +92,15 @@ (define (device-items)
          (device (car result)))
     device))
 
+(define (run-label-confirmation-page callback)
+  (lambda (item)
+    (and (run-confirmation-page
+          (format #f (G_ "This will create a new ~a partition table, \
+all data on disk will be lost, are you sure you want to proceed?") item)
+          (G_ "Format disk?")
+          #:exit-button-procedure callback)
+         item)))
+
 (define (run-label-page button-text button-callback)
   "Run a page asking the user to select a partition table label."
   ;; Force the GPT label if UEFI is supported.
@@ -103,6 +112,8 @@ (define (run-label-page button-text button-callback)
        #:title (G_ "Partition table")
        #:listbox-items '("msdos" "gpt")
        #:listbox-item->text identity
+       #:listbox-callback-procedure
+       (run-label-confirmation-page button-callback)
        #:button-text button-text
        #:button-callback-procedure button-callback)))
 
-- 
2.37.1
T
T
Tobias Geerinckx-Rice wrote on 16 Aug 18:55 +0200
(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 57232@debbugs.gnu.org)
87ilmsf68j.fsf@nckx
Hi Mathieu!

Mathieu Othacehe 写道:
Toggle quote (7 lines)
>> What that does is immediately and without confirmation wipe the
>> on-disc
>> partition table. And its back-up.
>
> Oops, glad you were able to recover, I was also bitten in the
> past.

My mistake for insisting on a bare metal test without a throwaway
machine handy :-)

Toggle quote (3 lines)
> The attached patch adds an extra confirmation page before wiping
> everything, WDYT?

Code looks all right. I'll try it out. Thanks!

The help text for users reads, in part:

“You can change a disk's partition table by selecting it and
pressing ENTER.”

Er, I was… expecting that to mean it would pop up a pretty window
or something. Is this really a feature? Should it be?

I have to be honest: I was extremely let down by the installer UX,
*because* I read a lot of the code and can see how much effort
went into it. I hate pointing out that the partitioner is at once
less useful and more dangerous than (system "fdisk").

:-/,

T G-R
-----BEGIN PGP SIGNATURE-----

iIMEARYKACsWIQT12iAyS4c9C3o4dnINsP+IT1VteQUCYvvTrA0cbWVAdG9iaWFz
LmdyAAoJEA2w/4hPVW15yNEBALvR/5YUi1ioZJ/8Hh0tqXsjT9AQfzbe4U9cm3nB
stJ3AQCzrPhdcH/eARHGaqD1XdLkDpYZfbMG1c5uwZHINEfUBg==
=yelZ
-----END PGP SIGNATURE-----

M
M
Mathieu Othacehe wrote on 16 Aug 23:42 +0200
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)(address . 57232@debbugs.gnu.org)
87ilmroogn.fsf@gnu.org
Hey,

Toggle quote (6 lines)
> “You can change a disk's partition table by selecting it and pressing
> ENTER.”
>
> Er, I was… expecting that to mean it would pop up a pretty window or
> something. Is this really a feature? Should it be?

I'm not sure what's the point here.

Toggle quote (5 lines)
> I have to be honest: I was extremely let down by the installer UX, *because* I
> read a lot of the code and can see how much effort went into it. I hate
> pointing out that the partitioner is at once less useful and more dangerous
> than (system "fdisk").

Many people have contributed to the installer over the years. Your
remark is both harsh and unconstructive. I don't think that the fact
that you, Tobias, are extremely let down matters much to the project.

Calling "fdisk" or "parted" directly would not provide the
auto-partitioning feature, and would be less convenient when it comes to
encryption and partition mount points selection.

It's no secret that the partitioning code can be improved like many
other Guix areas. If you feel like you can refine it, in term of
stability and general UX, we would be glad to get your support.

Mathieu
T
T
Tobias Geerinckx-Rice wrote on 17 Aug 01:40 +0200
(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 57232@debbugs.gnu.org)
87edxfg0pw.fsf@nckx
Hi Mathieu,

Mathieu Othacehe 写道:
Toggle quote (6 lines)
>> Er, I was… expecting that to mean it would pop up a pretty
>> window or
>> something. Is this really a feature? Should it be?
>
> I'm not sure what's the point here.

I'm not sure whose point you're referring to, so I'll clarify
mine.

Currently, pressing ENTER on a whole disc device wipes its
partition table, or at least did so for me (MBR). That's an
unexpected way ‘to change a disk's partition table’ even if it's
technically true.

It wasn't clear to me whether it was intentional, or a bug, or
perhaps a bit of both. Hence my question. I agree that we should
warn before writing anything, but that's somewhat orthogonal.

I had rewritten the dialogue before encountering this issue, but
it doesn't seem like I understand the intention behind the design
very well. I'll drop the UI-related patches and leave that effort
to someone who does.

Toggle quote (2 lines)
> we would be glad to get your support.

That wasn't the impression I got in response to some mild
criticism, but thanks. I'll keep working on it when I get that
spare machine :-)

(I do consider the installer important to the project, even if I
never claimed to be its target audience.)

Kind regards,

T G-R
-----BEGIN PGP SIGNATURE-----

iIMEARYKACsWIQT12iAyS4c9C3o4dnINsP+IT1VteQUCYvw5bw0cbWVAdG9iaWFz
LmdyAAoJEA2w/4hPVW15neYA/ikW132MHf1kcNvKt2rWLJ1Xn/FfIYSSXsQ0d27p
aSG2AQC5uYWM/rh0ikPskmx4BMl2GkVl8kVqt/nIBicp/3vpAg==
=ipLh
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 5 Sep 15:07 +0200
control message for bug #57232
(address . control@debbugs.gnu.org)
87zgfe2cm8.fsf@gnu.org
severity 57232 important
quit
L
L
Ludovic Courtès wrote on 5 Sep 15:07 +0200
control message for bug #53214
(address . control@debbugs.gnu.org)
87y1uy2cm0.fsf@gnu.org
block 53214 by 57232
quit
?