Bug#706112: [PATCH v2] grub-installer: Support menu selection of grub boot disk

2013-06-08 Thread Cyril Brulebois 
Vincent McIntyre vincent.mcint...@csiro.au (30/04/2013):
 gah. The patch was not ok. My apologies for such a gross error.
 I caught this by testing with 1.86 as downloaded from the archive.

Thanks, Vincent.

Applied locally. I'll try and play with it, and see when that can be
pushed to unstable, and maybe into {the next,an upcoming} wheezy point
release.

Mraw,
KiBi.


signature.asc
Description: Digital signature

Bug#706112: [PATCH v2] grub-installer: Support menu selection of grub boot disk

2013-06-08 Thread Vincent McIntyre 
On Sun, Jun 09, 2013 at 01:47:36AM +0200, Cyril Brulebois wrote:
 Vincent McIntyre vincent.mcint...@csiro.au (30/04/2013):
  gah. The patch was not ok. My apologies for such a gross error.
  I caught this by testing with 1.86 as downloaded from the archive.
 
 Thanks, Vincent.
 
 Applied locally. I'll try and play with it, and see when that can be
 pushed to unstable, and maybe into {the next,an upcoming} wheezy point
 release.
 

Okay, thanks for letting me know.
Since I posted that stuff I looked at thie code again and prepared
a patch series that includes the fix you've applied locally and
goes on to would bring the disk naming into line with the way grub-pc
does it. I'll send them off-list to avoid confusing the thread.
If you like them feel free to repost to the list.
It seems to me that there should be some kind of bug for this issue
to track progress on it, not sure how best to do that.

Cheers
Vince


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#706112: [PATCH v2] grub-installer: Support menu selection of grub boot disk

2013-04-30 Thread Adam D. Barratt 
user release.debian@packages.debian.org
usertags 706112 + wheezy-can-defer
tags 706112 + wheezy-ignore
thanks

On Tue, 2013-04-30 at 00:34 +0200, Cyril Brulebois wrote:
 I suggest we work towards having a suitable grub-installer for
 inclusion into r1 (which will possibly be released roughly 1 month
 after r0), for which we'll have more time than a couple of hours/days
 of testing. We'll be able to receive translation updates without
 last-minute added pressure, which should nicely appease Christian. ;-)
 
 Cc-ing debian-release@ to let them know we're skipping a (very late)
 grub-installer update.

Thanks; tagging appropriately.

Regards,

Adam


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#706112: [PATCH v2] grub-installer: Support menu selection of grub boot disk

2013-04-29 Thread Cyril Brulebois 
Christian PERRIER bubu...@debian.org (29/04/2013):
 I applied and pushed your patch. The maint point is that PO files
 get updated by the daily l10n-sync script.
 
 Then, I propose we upload 1.86 immediately to unstable so that it's
 tested at least by those people who install unstable with D-I.
 
 Cyril, others, do you think there would be a risk that it interacts
 badly with the release preparation?  The point is not trying to get
 1.86 in testing before the release, of course...bt just have the
 patch to receive more testing.

While I haven't reviewed the patch yet, having the l10n bits in sync
was a point I had in mind this very morning, so I'm glad you did
that. And indeed, some feedback ASAP would be nice. I'll try to look
into this patch in a few hours at most.

Mraw,
KiBi.


signature.asc
Description: Digital signature

Bug#706112: [PATCH v2] grub-installer: Support menu selection of grub boot disk

2013-04-29 Thread Cyril Brulebois 
Cyril Brulebois k...@debian.org (29/04/2013):
 While I haven't reviewed the patch yet, having the l10n bits in sync
 was a point I had in mind this very morning, so I'm glad you did
 that. And indeed, some feedback ASAP would be nice. I'll try to look
 into this patch in a few hours at most.

Uploaded in time for the 1352 dinstall. Let's see if tests are
successful.

Mraw,
KiBi.


signature.asc
Description: Digital signature

Bug#706112: [PATCH v2] grub-installer: Support menu selection of grub boot disk

2013-04-29 Thread Christian PERRIER 
Quoting Cyril Brulebois (k...@debian.org):
 Cyril Brulebois k...@debian.org (29/04/2013):
  While I haven't reviewed the patch yet, having the l10n bits in sync
  was a point I had in mind this very morning, so I'm glad you did
  that. And indeed, some feedback ASAP would be nice. I'll try to look
  into this patch in a few hours at most.
 
 Uploaded in time for the 1352 dinstall. Let's see if tests are
 successful.

Of course, you're aware that translations are currently not in the git
tree, as I committed the modified templates this morning, and the sync
will happen in the upcoming night?

So, in short, if the patch is OK, we need another upload to get the
new template translated.




signature.asc
Description: Digital signature

Bug#706112: [PATCH v2] grub-installer: Support menu selection of grub boot disk

2013-04-29 Thread Vincent McIntyre 
On Mon, Apr 29, 2013 at 07:16:42PM +0200, Christian PERRIER wrote:
 Quoting Cyril Brulebois (k...@debian.org):
  Cyril Brulebois k...@debian.org (29/04/2013):
   While I haven't reviewed the patch yet, having the l10n bits in sync
   was a point I had in mind this very morning, so I'm glad you did
   that. And indeed, some feedback ASAP would be nice. I'll try to look
   into this patch in a few hours at most.
  
  Uploaded in time for the 1352 dinstall. Let's see if tests are
  successful.
 
 Of course, you're aware that translations are currently not in the git
 tree, as I committed the modified templates this morning, and the sync
 will happen in the upcoming night?
 
 So, in short, if the patch is OK, we need another upload to get the
 new template translated.
 
 

gah. The patch was not ok. My apologies for such a gross error.
I caught this by testing with 1.86 as downloaded from the archive.

[PATCH] Actually set bootdev.

After taking all the trouble to get the right value into
the $bootdev shell variable, ensure that we db_set
grub-installer/bootdev with that value.

Signed-off-by: Vincent McIntyre vincent.mcint...@csiro.au
---
 grub-installer |8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/grub-installer b/grub-installer
index 71e10c8..f3d62fe 100755
--- a/grub-installer
+++ b/grub-installer
@@ -683,6 +683,7 @@ while : ; do
previous_state=1
fi
if [ -e $bootdev ] ; then
+   db_set grub-installer/bootdev $bootdev
break
else
state=2
@@ -703,9 +704,12 @@ while : ; do
unset previous_state
fi
 
-   if [ ! -e $bootdev ]; then
-   db_input critical grub-installer/bootdev || true
+   if [ -e $bootdev ]; then
+   db_set grub-installer/bootdev $bootdev
+   break
fi
+
+   db_input critical grub-installer/bootdev || true
if ! db_go; then
if [ $q ]; then
state=1
-- 
vincent.mcint...@csiro.au


-- 
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org

Bug#706112: [PATCH v2] grub-installer: Support menu selection of grub boot disk

2013-04-29 Thread Cyril Brulebois 
Vincent McIntyre vincent.mcint...@csiro.au (30/04/2013):
 gah. The patch was not ok. My apologies for such a gross error.
 I caught this by testing with 1.86 as downloaded from the archive.
 
 [PATCH] Actually set bootdev.
 
 After taking all the trouble to get the right value into
 the $bootdev shell variable, ensure that we db_set
 grub-installer/bootdev with that value.
 
 Signed-off-by: Vincent McIntyre vincent.mcint...@csiro.au

Hi again Vincent,

I guess that settles it. My initial tests (with different VM configs,
with 1.85 and 1.86) were quite OK so far, but I see code still needs
refining. I don't think it's reasonable to push an updated ( 1.85)
grub-installer into r0 (meaning a migration to testing by wednesday).
I had been struggling the whole day, trying to balance fixing a quite
common use case, and resisting rushing a new version into testing at
this point.

I suggest we work towards having a suitable grub-installer for
inclusion into r1 (which will possibly be released roughly 1 month
after r0), for which we'll have more time than a couple of hours/days
of testing. We'll be able to receive translation updates without
last-minute added pressure, which should nicely appease Christian. ;-)

Cc-ing debian-release@ to let them know we're skipping a (very late)
grub-installer update.

Mraw,
KiBi.


signature.asc
Description: Digital signature

Bug#706112: [PATCH v2] grub-installer: Support menu selection of grub boot disk

2013-04-28 Thread Vincent McIntyre 
Hi,
Thanks for the comments I've received on this patch.
I did some reading to see how other packages handle the issues raised.
I found grub-pc and iso-scan particularly helpful.
Hopefully this attempt is better. Less than a week from release,
I expect it is too late to include this but perhaps it could be
considered for r1.

Kind regards
Vince

---
Support user selection of grub boot disk from a list of
disks via a new question, grub-installer/choose_bootdev.
Check for a mismatch between a preseeded value of
grub-installer/bootdev and the guess at the default boot
disk made by grub-installer, and prompt the user to choose
the correct disk.

This should help the user avoid grub-installer writing to the MBR
of the wrong device (e.g. #696877,#706112) and fix the issue with
preseeded values of bootdev being ignored (e.g. #666974).

v2:
 - try harder to avoid introducing new translatable strings
   - recycle question  first paragraph of grub-install/bootdev
   - use iso-scan/ask_device text for manual entry text
 - drop device_list() function; try to reuse existing functions
   and follow the methods in grub-pc  iso-scan
 - don't abuse the 'seen' flag

Signed-off-by: Vincent McIntyre vincent.mcint...@csiro.au
---
 debian/grub-installer.templates |   13 +
 grub-installer  |   99 +--
 2 files changed, 109 insertions(+), 3 deletions(-)

diff --git a/debian/grub-installer.templates b/debian/grub-installer.templates
index 888a656..e439ad0 100644
--- a/debian/grub-installer.templates
+++ b/debian/grub-installer.templates
@@ -86,6 +86,19 @@ _Description: Device for boot loader installation:
 drive;
   - /dev/fd0 will install GRUB to a floppy.
 
+Template: grub-installer/choose_bootdev
+Type: select
+Choices-C: manual, ${DEVICES_LIST}
+#flag:translate!:2
+__Choices: Enter device manually, ${DESCRIPTIONS}
+# :sl2:
+_Description: Device for boot loader installation:
+ You need to make the newly installed system bootable, by installing
+ the GRUB boot loader on a bootable device. The usual way to do this is to
+ install GRUB on the master boot record of your first hard drive. If you
+ prefer, you can install GRUB elsewhere on the drive, or to another drive,
+ or even to a floppy.
+
 Template: grub-installer/password
 Type: password
 # :sl2:
diff --git a/grub-installer b/grub-installer
index f01eda1..71e10c8 100755
--- a/grub-installer
+++ b/grub-installer
@@ -1,5 +1,6 @@
 #! /bin/sh
 
+# export DEBCONF_DEBUG=5
 set -e
 . /usr/share/debconf/confmodule
 #set -x
@@ -590,7 +591,81 @@ esac
 db_progress STEP 1
 db_progress INFO grub-installer/progress/step_bootdev
 
+select_bootdev() {
+   [ X = X${DEBCONF_DEBUG} ] || log select_bootdev: arg='$1'
+
+   local dev_list dev_descr grubdev devices disk_id dev descr
+   local default_choice chosen result
+
+   result=
+   default_choice=$1
+
+   # /dev/disk/by-id has multiple links for the same physical disk.
+   # Let's trust grub-mkdevicemap to select the most suitable ones
+   # and correctly handle systems with no /dev/disk/by-id.
+   # Use disk id string as a shortcut way to describe it.
+   # FIXME switch to grub-pc's far more elegant disk_descriptions()
+   dev_list=
+   dev_descr=
+   devices=$($chroot $ROOT grub-mkdevicemap --no-floppy -m - | cut -f2)
+   for grubdev in $devices; do
+   disk_id=$(device_to_id $grubdev)
+   dev=$(readlink -f $disk_id)
+   dev_list=${dev_list:+$dev_list, }$dev
+   descr=$(echo $disk_id |sed -e 's+^.*/++' |sed -e 's+,+\\,+g')
+   if [ $dev = $disk_id ]; then
+   dev_descr=${dev_descr:+$dev_descr, }$dev
+   else
+   #/dev/sdX (id)
+   dev_descr=${dev_descr:+$dev_descr, }$dev  ($descr)
+   fi
+   done
+
+   [ X = X${DEBCONF_DEBUG} ] || log Bootdev Choices: '$dev_list'
+   [ X = X${DEBCONF_DEBUG} ] || log Bootdev Descriptions: 
'$dev_descr'
+
+   db_subst grub-installer/choose_bootdev DEVICES_LIST $dev_list
+   db_subst grub-installer/choose_bootdev DESCRIPTIONS $dev_descr
+   # set initial selection
+   if [ -n $default_choice ] ; then
+   chosen=$(readlink -f $default_choice)
+   if [ -n $chosen ] ;then
+   db_set grub-installer/choose_bootdev $chosen
+   fi
+   fi
+
+   db_input high grub-installer/choose_bootdev || true
+   if ! db_go; then
+   log Returning to menu
+   db_progress STOP
+   exit 10
+   fi
+
+   db_get grub-installer/choose_bootdev || true
+   # Choices-C (not shown to user) can be set to 'manual'
+   if [ $RET = manual ] ; then
+   result=
+   else
+   result=$(echo $RET | cut -d' ' -f1)
+   fi
+
+   [ X =

Bug#706112: [PATCH v2] grub-installer: Support menu selection of grub boot disk

2013-04-28 Thread Christian PERRIER 
Quoting Vincent McIntyre (vincent.mcint...@csiro.au):
 Hi,
 Thanks for the comments I've received on this patch.
 I did some reading to see how other packages handle the issues raised.
 I found grub-pc and iso-scan particularly helpful.
 Hopefully this attempt is better. Less than a week from release,
 I expect it is too late to include this but perhaps it could be
 considered for r1.


Hell Vincent,

I applied and pushed your patch. The maint point is that PO files get
updated by the daily l10n-sync script.

Then, I propose we upload 1.86 immediately to unstable so that it's
tested at least by those people who install unstable with D-I. 

Cyril, others, do you think there would be a risk that it interacts
badly with the release preparation?  The point is not trying to get
1.86 in testing before the release, of course...bt just have the patch
to receive more testing.





signature.asc
Description: Digital signature