Re: [PATCH] grub-probefs --root-device

[Robert Millan]

On Tue, Sep 26, 2006 at 08:29:33PM +0300, Vesa Jääskeläinen wrote:
> Hi,
> 
> Here are some comments about your patch. As requested :)

Thanks for the corrections.  Some of them make me ashamed, I should have
looked better at all those grub-probefs instances... ;)

> Robert Millan wrote:
> > 2006-09-23  Robert Millan  <address@hidden>
> > 
> >     * util/i386/pc/getroot.c (grub_guess_root_device): Stop using
> >     grub_util_biosdisk_get_grub_dev to convert system device to GRUB device.
> >     * util/grub-emu.c: Use grub_util_biosdisk_get_grub_dev with the string
> >     returned by grub_guess_root_device.
> 
> Method name missing from grub-emu.

Ok (main).

> >     * util/i386/pc/grub-setup.c: Ditto.
> >     * util/i386/pc/grub-probefs.c: Ditto.
> > 
> >     * util/i386/pc/grub-probefs.c: Rename to ...
> >     * util/i386/pc/grub-probe.c: ... this.
> >     * DISTLIST: Update reference to grub-probefs.
> >     * conf/i386-efi.mk: Ditto.
> >     * conf/i386-efi.rmk: Ditto.
> >     * conf/i386-pc.mk: Ditto.
> >     * conf/i386-pc.rmk: Ditto.
> >     * util/i386/pc/grub-install.in: Ditto.
> 
> Describe what changed in terms of source code not in terms of "english".
>  Eg. DISTLIST: Removed grub-probefs.  Added grub-probe.
> 
> I am not sure is "ditto" a good term.. I have seen "Likewise." in other
> entries and used that myself too.
> 
> And no need to inform about changes to .mk as they are generated files.
> Just make sure on commit they are there.

Ok.  I'm also omitting .mk files from my patch.  I'll take care of regenerating
later.

> >     * util/i386/pc/grub-probe.c: Add --target=(fs|device|drive) option to
> >     choose which information we want to print.
> 
> Method...

This change applies in various places of the file (main, probe, options..)

> Should be grub_probe as name changes... May need changes to other places
> too.

Checked them all this time.

> >    --no-floppy             do not probe any floppy drive
> >    --recheck               probe a device map even if it already exists
> >  
> > @@ -92,8 +92,8 @@
> >     grub_mkimage=`echo "$option" | sed 's/--grub-mkimage=//'` ;;
> >      --grub-mkdevicemap=*)
> >     grub_mkdevicemap=`echo "$option" | sed 's/--grub-mkdevicemap=//'` ;;
> > -    --grub-probefs=*)
> > -   grub_probefs=`echo "$option" | sed 's/--grub-probefs=//'` ;;
> > +    --grub-probe=*)
> > +   grub_probefs=`echo "$option" | sed 's/--grub-probe=//'` ;;
> 
> Needs change here at least...

Done.  Also added an explicit --target=fs option in grub-install (better be
safe).

Please let me know if it's ok now:

2006-09-28  Robert Millan  <address@hidden>

        * util/i386/pc/getroot.c (grub_guess_root_device): Stop using
        grub_util_biosdisk_get_grub_dev to convert system device to GRUB device.
        * util/grub-emu.c (main): Use grub_util_biosdisk_get_grub_dev with the
        string returned by grub_guess_root_device.
        * util/i386/pc/grub-setup.c: Likewise.
        * util/i386/pc/grub-probefs.c: Likewise.

        * util/i386/pc/grub-probefs.c: Rename to ...
        * util/i386/pc/grub-probe.c: ... this.
        * DISTLIST: Remove grub-probefs, add grub-probe.
        * conf/i386-efi.rmk: Likewise.
        * conf/i386-pc.rmk: Likewise.
        * util/i386/pc/grub-install.in: Likewise.

        * util/i386/pc/grub-probe.c: Add --target=(fs|device|drive) option to
        choose which information we want to print.

-- 
Robert Millan

My spam trap is address@hidden  Note: this address is only intended for
spam harvesters.  Writing to it will get you added to my black list.

print_paths.diff
Description: Text document