If the problem is that the user doesn't know he won't be able to reboot...

[Kapil Hari Paranjape]

For some reason this was mangled when it went to the list.
So I'm posting it directly to the Pkg-grub-devel list.

In my previous response, I missed this post to this bug list.

On Wed, 13 Sep 2006, Marc Fargas wrote:
> Steve Langasek <vorlon at debian.org> writes:
> >Surely this is still a bug in the grub shell for failing to notice that it
> >had rendered the system unbootable?
> 
> Grub shell is supposed to be used by "experienced" users, and if the
> big problem is that setup() doesn't copy those files to /boot/ there
> are two lines of action:
> a) On a call to setup() output a nice warning message that clearly
> says that the files MUST be manually copied by the user encouragin the
> use of grub-install
> b) On a call to setup() take care of copying the files.
> 
> I'd vote for option a, with that warning in place the bug would be ok,
> right? I've never touched inside grub but I can try to patch it if you
> select A, and less maybe for B ;) [so, is option A ok for closing the
> bug? at least the user knows that he/she won't boot again... hehe]

I concur and enclose a patch which is the output of:

	interdiff -z grub_0.97-15.diff.gz grub_0.97-15.0pre1.diff.gz 

(This also includes the small patch for #385980 suggested by Joey Hess.)

On Thu, 14 Sep 2006, Jason Thomas wrote:
> option C, we create a way to extract the version information from every
> grub file. So that the grub shell can check that its version matches
> the stage files and if not generate an ERROR message.

Note that stage1 is a boot sector and so contains precious little
space for such version information. Implementing this option seems
quite difficult to me.

Regards,

Kapil.
--


diff -u grub-0.97/debian/patches/00list grub-0.97/debian/patches/00list
--- grub-0.97/debian/patches/00list
+++ grub-0.97/debian/patches/00list
@@ -19,6 +19,7 @@
 mprotect.diff
 savedefault.diff
 intelmac.diff
+setup_warn_version.diff
 
 # We aren't building amd64 binaries, see #244498
 #fix_amd64_compile.diff
diff -u grub-0.97/debian/changelog grub-0.97/debian/changelog
--- grub-0.97/debian/changelog
+++ grub-0.97/debian/changelog
@@ -1,3 +1,14 @@
+grub (0.97-15.0pre1) unstable; urgency=low
+
+  * Non-Maintainer Upload.
+  * debian/rules: Fixed up "clean" target as suggested by Joey Hess.
+    (Closes #385980)
+  * debian/patches/setup_warn_version.diff: Patch "stage2/builtins.c"
+    to warn user who uses "setup" from the grub shell.
+    (Closes: #345931)
+
+ -- Kapil Hari Paranjape <kapil at imsc.res.in>  Fri, 22 Sep 2006 11:45:09 +0530
+
 grub (0.97-15) unstable; urgency=low
 
   * Remove bashism from update-grub wrapper.
diff -u grub-0.97/debian/rules grub-0.97/debian/rules
--- grub-0.97/debian/rules
+++ grub-0.97/debian/rules
@@ -181,7 +181,7 @@
 	-( cd docs && rm -rf grub multiboot )
 
 	# remove files that will be change due our automake and autoconf rebuilding
-	-rm -f $(find . -name 'Makefile.in' -o \
+	-rm -f $(shell find . -name 'Makefile.in' -o \
                     -name 'aclocal.m4' -o \
                     -name 'configure')
 
only in patch2:
unchanged:
--- grub-0.97.orig/debian/patches/setup_warn_version.diff
+++ grub-0.97/debian/patches/setup_warn_version.diff
@@ -0,0 +1,42 @@
+--- grub-0.97/stage2/builtins.c.orig	2005-02-16 03:28:23.000000000 +0530
++++ grub-0.97/stage2/builtins.c	2006-09-22 11:40:00.000000000 +0530
+@@ -3798,10 +3798,19 @@
+   char *stage2_arg = 0;
+   char *prefix = 0;
+ 
++  auto void warn ();
+   auto int check_file (char *file);
+   auto void sprint_device (int drive, int partition);
+   auto int embed_stage1_5 (char * stage1_5, int drive, int partition);
+-  
++
++  /* Warn the user that stage files are version specific */
++  void warn()
++    {
++    	grub_printf("WARNING:The stage{1,1_5,2} files are version specific.\n");
++    	grub_printf("The tests below only check the existence and not the version.\n");
++    	grub_printf("Use the \"grub-install\" command if you are not sure.\n");
++    }
++
+   /* Check if the file FILE exists like Autoconf.  */
+   int check_file (char *file)
+     {
+@@ -3951,6 +3960,7 @@
+      `--prefix', attempt /boot/grub and /grub.  */
+   /* NOTE: It is dangerous to run this command without `--prefix' in the
+      grub shell, since that affects `--stage2'.  */
++  warn();
+   if (! prefix)
+     {
+       prefix = "/boot/grub";
+@@ -4082,6 +4092,10 @@
+   " If you install GRUB under the grub shell and you cannot unmount the"
+   " partition where GRUB images reside, specify the option `--stage2'"
+   " to tell GRUB the file name under your OS."
++  " WARNING: In order for this to work GRUB needs to find the images"
++  " \"stage1\" and \"stage2\" (or \"stage1_5\") corresponding to its"
++  " current version in the locations specified by INSTALL_DEVICE"
++  " (or IMAGE_DEVICE if specified; or STAGE2_FILE if specified)."
+ };
+ 
+ 




----- End forwarded message -----