Re: [PATCH] normalize default prefix in grub-mkimage

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 21.01.2014 17:48, Andrey Borzenkov wrote:
> В Tue, 21 Jan 2014 08:19:24 +0100
> Vladimir 'φ-coder/phcoder' Serbinenko <address@hidden> пишет:
> 
>> On 27.12.2013 18:41, Andrey Borzenkov wrote:
>>> This fixes bogus "C:\boot\grub" at runtime for Windows build as well as
>>> the case when /boot is on separate mount point for other systems.
>>>
>> grub-mkimage shouldn't perform any FS-based resolving, that's job of the
>> caller. This default should either be hardwired to
>> "/"GRUB_BOOT_DIR_NAME"/"GRUB_DIR_NAME
>> or perhaps even to "/boot/grub" or default should be killed altogether.
> 
> I prefer the latter. Any default will most likely be wrong, so it is
> better to be explicit about it.
> 
If you call grub-mkimage directly you're either dev or doing sth wrong.
The default allows quicker creation of test images for new systems where
you don't expect it to get to normal mode yet anyway. This being said, I
agree with restraining possibilities of direct-tool-usage screwup.
Go ahead.
> From: Andrey Borzenkov <address@hidden>
> Subject: [PATCH] do not set default prefix in grub-mkimage
> 
> Default prefix is likely wrong on Unix and completely wrong on Windows.
> Let caller set it explicitly to avoid any ambiguity.
> 
> ---
>  util/grub-mkimage.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/util/grub-mkimage.c b/util/grub-mkimage.c
> index a2bd4c1..1e0bcf1 100644
> --- a/util/grub-mkimage.c
> +++ b/util/grub-mkimage.c
> @@ -64,7 +64,7 @@ static struct argp_option options[] = {
>    {"directory",  'd', N_("DIR"), 0,
>     /* TRANSLATORS: platform here isn't identifier. It can be translated.  */
>     N_("use images and modules under DIR [default=%s/<platform>]"), 0},
> -  {"prefix",  'p', N_("DIR"), 0, N_("set prefix directory [default=%s]"), 0},
> +  {"prefix",  'p', N_("DIR"), 0, N_("set prefix directory"), 0},
>    {"memdisk",  'm', N_("FILE"), 0,
>     /* TRANSLATORS: "memdisk" here isn't an identifier, it can be translated.
>      "embed" is a verb (command description).  "*/
> @@ -93,8 +93,6 @@ help_filter (int key, const char *text, void *input 
> __attribute__ ((unused)))
>      {
>      case 'd':
>        return xasprintf (text, grub_util_get_pkglibdir ());
> -    case 'p':
> -      return xasprintf (text, DEFAULT_DIRECTORY);
>      case 'O':
>        {
>       char *formats = grub_install_get_image_targets_string (), *ret;
> @@ -268,6 +266,15 @@ main (int argc, char *argv[])
>        exit(1);
>      }
>  
> +  if (!arguments.prefix)
> +    {
> +      char *program = xstrdup(program_name);
> +      printf ("%s\n", _("Prefix not specified (use the -p option)."));
> +      argp_help (&argp, stderr, ARGP_HELP_STD_USAGE, program);
> +      free (program);
> +      exit(1);
> +    }
> +
>    if (arguments.output)
>      {
>        fp = grub_util_fopen (arguments.output, "wb");
> @@ -287,8 +294,7 @@ main (int argc, char *argv[])
>        strcpy (ptr, dn);
>      }
>  
> -  grub_install_generate_image (arguments.dir,
> -                            arguments.prefix ? : DEFAULT_DIRECTORY, fp,
> +  grub_install_generate_image (arguments.dir, arguments.prefix, fp,
>                              arguments.output, arguments.modules,
>                              arguments.memdisk, arguments.pubkeys,
>                              arguments.npubkeys, arguments.config,
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/grub-devel
>

signature.asc
Description: OpenPGP digital signature