[PATCH] Fix grub-install to use prefix for modules dir

[PATCH] Fix grub-install to use prefix for modules dir

[Colin D Bennett]

[-- Attachment #1: Type: text/plain, Size: 217 bytes --]

I fixed ``grub-install`` so it can function when installed
to a prefix other than ``/usr/local``.  This simply meant adding

  --directory="${pkglibdir}

to the command line arguments for grub_mkimage.

Regards,
Colin

[-- Attachment #2: grub_install_pkglibdir_fix_2008-06-17.patch --]
[-- Type: text/x-patch, Size: 759 bytes --]

=== modified file 'util/i386/pc/grub-install.in'
--- util/i386/pc/grub-install.in	2008-06-14 19:04:44 +0000
+++ util/i386/pc/grub-install.in	2008-06-17 17:07:08 +0000
@@ -234,7 +234,10 @@
 # The order in this list is critical.  Be careful when modifiing it.
 modules="$modules $fs_module $partmap_module biosdisk $devabstraction_module"
 
-$grub_mkimage --output=${grubdir}/core.img --prefix=`make_system_path_relative_to_its_root ${grubdir}`/ $modules || exit 1
+$grub_mkimage --output=${grubdir}/core.img \
+              --directory="${pkglibdir}" \
+              --prefix=`make_system_path_relative_to_its_root ${grubdir}`/ \
+	      $modules || exit 1
 
 # Now perform the installation.
 $grub_setup --directory=${grubdir} --device-map=${device_map} \

Re: [PATCH] Fix grub-install to use prefix for modules dir

[Pavel Roskin]

On Tue, 2008-06-17 at 10:51 -0700, Colin D Bennett wrote:
> I fixed ``grub-install`` so it can function when installed
> to a prefix other than ``/usr/local``.  This simply meant adding
> 
>   --directory="${pkglibdir}
> 
> to the command line arguments for grub_mkimage.

The default is GRUB_LIBDIR, which is set to pkglibdir at the compile
time.  Your patch would have no effect if anything is compiled and
installed properly.

Maybe you are trying to change prefix during "make install"?  That's not
the best approach.  DESTDIR should be used to install to a temporary
location.

Or maybe you have some other scenario in mind?

-- 
Regards,
Pavel Roskin

Re: [PATCH] Fix grub-install to use prefix for modules dir

[Colin D Bennett]

On Tue, 17 Jun 2008 15:46:12 -0400
Pavel Roskin <proski@gnu.org> wrote:

> On Tue, 2008-06-17 at 10:51 -0700, Colin D Bennett wrote:
> > I fixed ``grub-install`` so it can function when installed
> > to a prefix other than ``/usr/local``.  This simply meant adding
> > 
> >   --directory="${pkglibdir}
> > 
> > to the command line arguments for grub_mkimage.
> 
> The default is GRUB_LIBDIR, which is set to pkglibdir at the compile
> time.  Your patch would have no effect if anything is compiled and
> installed properly.
> 
> Maybe you are trying to change prefix during "make install"?  That's
> not the best approach.  DESTDIR should be used to install to a
> temporary location.
> 
> Or maybe you have some other scenario in mind?

I'm not sure.  All I know is that I did 
  ./configure --prefix=/home/cdb/bin/grub && make && make install,
then I tried to do /home/cdb/bin/grub/bin/grub-install /media/sdc1 and
grub-mkimage was getting called with /usr/local/lib/something.  Since I
specified --prefix to configure, I expect grub never ever to refer
to /usr/local.  This patch fixed my problem.

Colin

Re: [PATCH] Fix grub-install to use prefix for modules dir

[Pavel Roskin]

On Tue, 2008-06-17 at 13:04 -0700, Colin D Bennett wrote:

> I'm not sure.  All I know is that I did 
>   ./configure --prefix=/home/cdb/bin/grub && make && make install,
> then I tried to do /home/cdb/bin/grub/bin/grub-install /media/sdc1 and
> grub-mkimage was getting called with /usr/local/lib/something.  Since I
> specified --prefix to configure, I expect grub never ever to refer
> to /usr/local.  This patch fixed my problem.

I think I know what you actually did:

./configure
make
./configure --prefix=/home/cdb/bin/grub
make
make install

"/usr/local" was hardcoded into grub-mkimage in the first run.  In the
second run, grub-mkimage wasn't rebuilt because it's not a script and
because config.h didn't change.

We don't want wrong path to be hardcoded in grub-mkimage.  Working it
around in scripts would keep grub-mkimage incorrect.

I think the best solution would be to add Makefile to
the grub-mkimage.o dependencies.  GRUB_LIBDIR is written to Makefile
and grub-mkimage.c uses it.  Here's the patch (*.mk changes are not
included):

diff --git a/conf/i386-efi.rmk b/conf/i386-efi.rmk
index 87626c2..311b6ab 100644
--- a/conf/i386-efi.rmk
+++ b/conf/i386-efi.rmk
@@ -17,6 +17,7 @@ sbin_UTILITIES = grub-mkdevicemap
 # For grub-mkimage.
 grub_mkimage_SOURCES = util/i386/efi/grub-mkimage.c util/misc.c \
 	util/resolve.c
+util/i386/efi/grub-mkimage.c_DEPENDENCIES = Makefile
 
 # For grub-setup.
 #grub_setup_SOURCES = util/i386/pc/grub-setup.c util/biosdisk.c	\
diff --git a/conf/i386-ieee1275.rmk b/conf/i386-ieee1275.rmk
index ac2f45d..e4f2a66 100644
--- a/conf/i386-ieee1275.rmk
+++ b/conf/i386-ieee1275.rmk
@@ -50,6 +50,7 @@ endif
 grub_mkimage_SOURCES = util/elf/grub-mkimage.c util/misc.c \
 	util/resolve.c
 grub_mkimage_LDFLAGS = $(LIBLZO)
+util/elf/grub-mkimage.c_DEPENDENCIES = Makefile
 
 # For grub-mkdevicemap.
 grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c		\
diff --git a/conf/i386-linuxbios.rmk b/conf/i386-linuxbios.rmk
index 31f03a9..d2546be 100644
--- a/conf/i386-linuxbios.rmk
+++ b/conf/i386-linuxbios.rmk
@@ -49,6 +49,7 @@ endif
 grub_mkimage_SOURCES = util/elf/grub-mkimage.c util/misc.c \
 	util/resolve.c
 grub_mkimage_LDFLAGS = $(LIBLZO)
+util/elf/grub-mkimage.c_DEPENDENCIES = Makefile
 
 # For grub-mkdevicemap.
 grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c		\
diff --git a/conf/i386-pc.rmk b/conf/i386-pc.rmk
index a3f8467..7b7924f 100644
--- a/conf/i386-pc.rmk
+++ b/conf/i386-pc.rmk
@@ -76,6 +76,7 @@ grub_mkimage_SOURCES = util/i386/pc/grub-mkimage.c util/misc.c \
 	util/resolve.c
 grub_mkimage_CFLAGS = -DGRUB_MEMORY_MACHINE_LINK_ADDR=$(GRUB_MEMORY_MACHINE_LINK_ADDR)
 grub_mkimage_LDFLAGS = $(LIBLZO)
+util/i386/pc/grub-mkimage.c_DEPENDENCIES = Makefile
 
 # For grub-setup.
 util/i386/pc/grub-setup.c_DEPENDENCIES = grub_setup_init.h
diff --git a/conf/powerpc-ieee1275.rmk b/conf/powerpc-ieee1275.rmk
index 2c16bc1..51442c6 100644
--- a/conf/powerpc-ieee1275.rmk
+++ b/conf/powerpc-ieee1275.rmk
@@ -37,6 +37,7 @@ endif
 # For grub-mkimage.
 grub_mkimage_SOURCES = util/elf/grub-mkimage.c util/misc.c \
         util/resolve.c 
+util/elf/grub-mkimage.c_DEPENDENCIES = Makefile
 
 # For grub-mkdevicemap.
 grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c util/misc.c		\


-- 
Regards,
Pavel Roskin

Re: [PATCH] Fix grub-install to use prefix for modules dir

[Colin D Bennett]

On Tue, 17 Jun 2008 20:04:29 -0400
Pavel Roskin <proski@gnu.org> wrote:

> On Tue, 2008-06-17 at 13:04 -0700, Colin D Bennett wrote:
> 
> > I'm not sure.  All I know is that I did 
> >   ./configure --prefix=/home/cdb/bin/grub && make && make install,
> > then I tried to do /home/cdb/bin/grub/bin/grub-install /media/sdc1
> > and grub-mkimage was getting called with /usr/local/lib/something.
> > Since I specified --prefix to configure, I expect grub never ever
> > to refer to /usr/local.  This patch fixed my problem.
> 
> I think I know what you actually did:
> 
> ./configure
> make
> ./configure --prefix=/home/cdb/bin/grub
> make
> make install
> 
> "/usr/local" was hardcoded into grub-mkimage in the first run.  In the
> second run, grub-mkimage wasn't rebuilt because it's not a script and
> because config.h didn't change.

You are probably right... sorry to bother you with my error.

> We don't want wrong path to be hardcoded in grub-mkimage.  Working it
> around in scripts would keep grub-mkimage incorrect.
> 
> I think the best solution would be to add Makefile to
> the grub-mkimage.o dependencies.  GRUB_LIBDIR is written to Makefile
> and grub-mkimage.c uses it.  Here's the patch (*.mk changes are not
> included):
> 
> diff --git a/conf/i386-efi.rmk b/conf/i386-efi.rmk
> index 87626c2..311b6ab 100644
> --- a/conf/i386-efi.rmk
> +++ b/conf/i386-efi.rmk
> @@ -17,6 +17,7 @@ sbin_UTILITIES = grub-mkdevicemap
>  # For grub-mkimage.
>  grub_mkimage_SOURCES = util/i386/efi/grub-mkimage.c util/misc.c \
>  	util/resolve.c
> +util/i386/efi/grub-mkimage.c_DEPENDENCIES = Makefile
>  
>  # For grub-setup.
>  #grub_setup_SOURCES = util/i386/pc/grub-setup.c
> util/biosdisk.c	\ diff --git a/conf/i386-ieee1275.rmk
> b/conf/i386-ieee1275.rmk index ac2f45d..e4f2a66 100644
> --- a/conf/i386-ieee1275.rmk
> +++ b/conf/i386-ieee1275.rmk
> @@ -50,6 +50,7 @@ endif
>  grub_mkimage_SOURCES = util/elf/grub-mkimage.c util/misc.c \
>  	util/resolve.c
>  grub_mkimage_LDFLAGS = $(LIBLZO)
> +util/elf/grub-mkimage.c_DEPENDENCIES = Makefile
>  
>  # For grub-mkdevicemap.
>  grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c
> util/misc.c		\ diff --git a/conf/i386-linuxbios.rmk
> b/conf/i386-linuxbios.rmk index 31f03a9..d2546be 100644
> --- a/conf/i386-linuxbios.rmk
> +++ b/conf/i386-linuxbios.rmk
> @@ -49,6 +49,7 @@ endif
>  grub_mkimage_SOURCES = util/elf/grub-mkimage.c util/misc.c \
>  	util/resolve.c
>  grub_mkimage_LDFLAGS = $(LIBLZO)
> +util/elf/grub-mkimage.c_DEPENDENCIES = Makefile
>  
>  # For grub-mkdevicemap.
>  grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c
> util/misc.c		\ diff --git a/conf/i386-pc.rmk
> b/conf/i386-pc.rmk index a3f8467..7b7924f 100644
> --- a/conf/i386-pc.rmk
> +++ b/conf/i386-pc.rmk
> @@ -76,6 +76,7 @@ grub_mkimage_SOURCES = util/i386/pc/grub-mkimage.c
> util/misc.c \ util/resolve.c
>  grub_mkimage_CFLAGS =
> -DGRUB_MEMORY_MACHINE_LINK_ADDR=$(GRUB_MEMORY_MACHINE_LINK_ADDR)
> grub_mkimage_LDFLAGS = $(LIBLZO)
> +util/i386/pc/grub-mkimage.c_DEPENDENCIES = Makefile 
>  # For grub-setup.
>  util/i386/pc/grub-setup.c_DEPENDENCIES = grub_setup_init.h
> diff --git a/conf/powerpc-ieee1275.rmk b/conf/powerpc-ieee1275.rmk
> index 2c16bc1..51442c6 100644
> --- a/conf/powerpc-ieee1275.rmk
> +++ b/conf/powerpc-ieee1275.rmk
> @@ -37,6 +37,7 @@ endif
>  # For grub-mkimage.
>  grub_mkimage_SOURCES = util/elf/grub-mkimage.c util/misc.c \
>          util/resolve.c 
> +util/elf/grub-mkimage.c_DEPENDENCIES = Makefile
>  
>  # For grub-mkdevicemap.
>  grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c
> util/misc.c		\

Sounds good to me!

Thanks,
Colin

Re: [PATCH] Fix grub-install to use prefix for modules dir

[Pavel Roskin]

On Tue, 2008-06-17 at 23:09 -0700, Colin D Bennett wrote:
> On Tue, 17 Jun 2008 20:04:29 -0400
> Pavel Roskin <proski@gnu.org> wrote:
> 
> > +util/elf/grub-mkimage.c_DEPENDENCIES = Makefile

> Sounds good to me!

Committed.

-- 
Regards,
Pavel Roskin

Re: [PATCH] Fix grub-install to use prefix for modules dir

[Isaac Dupree]

Pavel Roskin wrote:
> On Tue, 2008-06-17 at 10:51 -0700, Colin D Bennett wrote:
>> I fixed ``grub-install`` so it can function when installed
>> to a prefix other than ``/usr/local``.  This simply meant adding
>>
>>   --directory="${pkglibdir}
>>
>> to the command line arguments for grub_mkimage.
> 
> The default is GRUB_LIBDIR, which is set to pkglibdir at the compile
> time.  Your patch would have no effect if anything is compiled and
> installed properly.

indeed, grub-install works for me with ./configure --prefix=$PWD/inst; 
make; make install; sudo ./inst/sbin/grub-install ...

-Isaac

Re: [PATCH] Fix grub-install to use prefix for modules dir

[Colin D Bennett]

On Tue, 17 Jun 2008 15:46:12 -0400
Pavel Roskin <proski@gnu.org> wrote:

> On Tue, 2008-06-17 at 10:51 -0700, Colin D Bennett wrote:
> > I fixed ``grub-install`` so it can function when installed
> > to a prefix other than ``/usr/local``.  This simply meant adding
> > 
> >   --directory="${pkglibdir}
> > 
> > to the command line arguments for grub_mkimage.
> 
> The default is GRUB_LIBDIR, which is set to pkglibdir at the compile
> time.  Your patch would have no effect if anything is compiled and
> installed properly.


Ok, I tried it again with current GRUB CVS and it works.  I was working
with a CVS version from a couple of weeks ago; maybe the problem was
related to that.

Anyway, it sounds like my patch is unnecessary, so you can ignore it
now.

Regards,
Colin