Re: grub-mkrescue problems in argp_parse

Re: grub-mkrescue problems in argp_parse

[Andrei Borzenkov]

В Mon, 13 Apr 2015 18:30:01 +0200 (CEST)
Dietmar Maurer <dietmar@proxmox.com> пишет:

> After compiling latest zol grub snapshot on wheezy, 
> I try to build an ISO with grub-mkrescue:
> 
> # grub-mkrescue -o pve-cd.iso data-gz -- -z -r -V 'PVE' -c boot/boot.cat
> -isohybrid-gpt-basdat -partition_offset 16 -c boot/boot.ca
> grub-mkrescue (GRUB) 2.02-beta2.9-ZOL11-7aa9f6-wheezy
> 
> and nothing else happens (no iso, no error).
> 
> Looking at the code in grub/util/grub-mkrescue.c reveals problems with
> argp_parse()
> 
> After reverting the following commit everything works again:
> 
> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=cf47a2fba5852014bc59959c5e357e8313933414
> 
> Any ideas?
> 

Does it work if you remove "--"?

Re: grub-mkrescue problems in argp_parse

[Dietmar Maurer]

> > After reverting the following commit everything works again:
> > 
> > http://git.savannah.gnu.org/cgit/grub.git/commit/?id=cf47a2fba5852014bc59959c5e357e8313933414
> > 
> > Any ideas?
> > 
> 
> Does it work if you remove "--"?
> 

no

Re: grub-mkrescue problems in argp_parse

[Andrei Borzenkov]

В Mon, 13 Apr 2015 19:16:02 +0200 (CEST)
Dietmar Maurer <dietmar@proxmox.com> пишет:

> > > After reverting the following commit everything works again:
> > > 
> > > http://git.savannah.gnu.org/cgit/grub.git/commit/?id=cf47a2fba5852014bc59959c5e357e8313933414
> > > 
> > > Any ideas?
> > > 
> > 
> > Does it work if you remove "--"?
> > 
> 
> no
> 

It chokes on -V which is "display version and exit" in argp. I still
believe this patch was mistake; it leaves no possibility to
disambiguate between grub and external options.

Vladimir?

Re: grub-mkrescue problems in argp_parse

[Thomas Schmitt]

Hi,

(i wonder why the original mail from Dietmar Maurer does
 not show up in grub-devel archives or my mailbox)

Dietmar Maurer wrote:
> > After reverting the following commit everything works again:
> > http://git.savannah.gnu.org/cgit/grub.git/commit/?id=cf47a2fba5852014bc59959c5e357e8313933414

Andrei Borzenkov wrote:
> It chokes on -V which is "display version and exit" in argp.

Alternative -as mkisofs option:  -volid 'PVE'

With old grub-mkrescue(.in) of 1.99, the version option was -v,
which would collide with the -as mkisofs option which has as alias
the word "-verbose".


> I still
> believe this patch was mistake; it leaves no possibility to
> disambiguate between grub and external options.

So we are back at the reasons why i proposed to introduce
a new tool  grub-mkiso.c  with clear distinction of arguments
for grub-mkiso and xorriso, and to have an artlessly implemented
but high-fidelity compatibility mode for emulating old
grub-mkrescue(.in).

My proposal of such an artless shell-to-C translation is posted as

  http://lists.gnu.org/archive/html/grub-devel/2014-10/msg00000.html

(Dietmar's command line would be for new grub-mkiso then.)


----------------------------------------------------------------
Some xorriso musings:

Dietmar Maurer wrote:
> > # grub-mkrescue -o pve-cd.iso data-gz -- -z -r -V 'PVE' -c boot/boot.cat
> > -isohybrid-gpt-basdat -partition_offset 16 -c boot/boot.ca

This seems to be mangled a bit. Especially the double
option -c. Were there more options ?

-isohybrid-gpt-basdat might collide with xorriso options
used by grub-mkrescue. (It is intended for ISOLINUX/GRUB2
BIOS/EFI hybrids. E.g. debian-7.7.0-amd64-netinst.iso.)

-partition_offset 16 is not really compliant with UEFI specs,
which prescribe a single MBR partition starting at block 1,
if GPT is present.
The whole ISOLINUX/GRUB2 isohybrid world does not care about
that prescription. But grub-mkrescue does.

To save some intermediate disk space:
The generic xorriso command
  -set_filter_r --zisofs ...paths.in.iso... --
can do the mkzftree(1) compression on the fly, if libz is
linked in.


Have a nice day :)

Thomas

Re: grub-mkrescue problems in argp_parse

[Dietmar Maurer]

> Dietmar Maurer wrote:
> > > After reverting the following commit everything works again:
> > > http://git.savannah.gnu.org/cgit/grub.git/commit/?id=cf47a2fba5852014bc59959c5e357e8313933414
> 
> Andrei Borzenkov wrote:
> > It chokes on -V which is "display version and exit" in argp.
> 
> Alternative -as mkisofs option:  -volid 'PVE'
> 
> With old grub-mkrescue(.in) of 1.99, the version option was -v,
> which would collide with the -as mkisofs option which has as alias
> the word "-verbose".

OK, that works - thanks!

> > I still
> > believe this patch was mistake; it leaves no possibility to
> > disambiguate between grub and external options.
> 
> So we are back at the reasons why i proposed to introduce
> a new tool  grub-mkiso.c  with clear distinction of arguments
> for grub-mkiso and xorriso, and to have an artlessly implemented
> but high-fidelity compatibility mode for emulating old
> grub-mkrescue(.in).

It is quite easy to detect if there is a '--' in the argument list.
So you could disable magic code if you find '--'?

> My proposal of such an artless shell-to-C translation is posted as
> 
>   http://lists.gnu.org/archive/html/grub-devel/2014-10/msg00000.html
> 
> (Dietmar's command line would be for new grub-mkiso then.)
> 
> 
> ----------------------------------------------------------------
> Some xorriso musings:
> 
> Dietmar Maurer wrote:
> > > # grub-mkrescue -o pve-cd.iso data-gz -- -z -r -V 'PVE' -c boot/boot.cat
> > > -isohybrid-gpt-basdat -partition_offset 16 -c boot/boot.ca
> 
> This seems to be mangled a bit. Especially the double
> option -c. Were there more options ?

This was just a mistake.

> -isohybrid-gpt-basdat might collide with xorriso options
> used by grub-mkrescue. (It is intended for ISOLINUX/GRUB2
> BIOS/EFI hybrids. E.g. debian-7.7.0-amd64-netinst.iso.)
> 
> -partition_offset 16 is not really compliant with UEFI specs,
> which prescribe a single MBR partition starting at block 1,
> if GPT is present.
> The whole ISOLINUX/GRUB2 isohybrid world does not care about
> that prescription. But grub-mkrescue does.

OK, I will try without those options.
 
> To save some intermediate disk space:
> The generic xorriso command
>   -set_filter_r --zisofs ...paths.in.iso... --
> can do the mkzftree(1) compression on the fly, if libz is
> linked in.

Many thanks for your help.

- Dietmar

Re: grub-mkrescue problems in argp_parse

[Thomas Schmitt]

Hi,

Dietmar Maurer wrote:
> It is quite easy to detect if there is a '--' in the argument list.
> So you could disable magic code if you find '--'?

It is about an unintended but actually beneficial change
in the syntax of grub-mkrescue arguments.

New grub-mkrescue.c interpreted and consumed the word '--',
which the old grub-mkrescue just forwarded to xorriso.
There it serves the important purpose to end the command -as
which performs the mkisofs emulation.

grub-mkrescue runs xorriso this way:

  xorriso -as mkisofs ...grub2.provided.mkisofs-ish.options... \
                      ...user.provided.arguments...

I.e. the first user provided argument will be interpreted by
xorriso as parameter of its command -as.
The range of -as ends at the first '--' which xorriso encounters.
Further arguments are interpreted by xorriso as commands and
their parameters. (Which are more expressive and capable than
mkisofs emulation.)


I understand that Vladimir reverted the behavioral change
on march 20, so that your run (without any generic xorriso
commands) was intended to work fine by the old style: 

  grub-mkrescue -o pve-cd.iso data-gz -z -r -V 'PVE' -c boot/boot.cat ...

But there is another difference to old grub-mkrescue:
The option to print the version string changed from -v to -V.
With mkisofs (and its emulation), -v is rarely used.
So it never mattered that old grub-mkrescue bailed out
when seeing it.
But -V is a frequently used mkisofs option.

So grub-mkrescue.c should either become really compatible
to its shell-based predecessor, or it should get a different
name and use the new syntax, which clearly distinguishes
between own options and xorriso arguments:

  grub-mkiso -o pve-cd.iso -- data-gz -z -r -V 'PVE' -c boot/boot.cat ...

All arguments after the first '--' would get forwarded to
xorriso without interpretation by grub-mkiso. The first
'--' itself would not get forwarded to xorriso.


Have a nice day :)

Thomas

Re: grub-mkrescue problems in argp_parse

[Dietmar Maurer]

> All arguments after the first '--' would get forwarded to
> xorriso without interpretation by grub-mkiso. The first
> '--' itself would not get forwarded to xorriso.

Ah, thanks for the details! It is more difficult than I thought ;-)

Re: grub-mkrescue problems in argp_parse

[Dietmar Maurer]

> Dietmar Maurer wrote:
> > It is quite easy to detect if there is a '--' in the argument list.
> > So you could disable magic code if you find '--'?
> 
> It is about an unintended but actually beneficial change
> in the syntax of grub-mkrescue arguments.
> 
> New grub-mkrescue.c interpreted and consumed the word '--',
> which the old grub-mkrescue just forwarded to xorriso.
> There it serves the important purpose to end the command -as
> which performs the mkisofs emulation.

Would it help if you use '---' as separator?

Re: grub-mkrescue problems in argp_parse

[Thomas Schmitt]

Hi,

> Would it help if you use '---' as separator?

grub-mkrescue could execute xorriso command
  -list_delimiter ---
before command -as.
But that would not solve the problem of old ambiguous
and new unambigous CLI which are not compatible.

Further it would make it even more difficult to understand
what scripts around grub-mkrescue and xorriso do.


Have a nice day :)

Thomas

Re: grub-mkrescue problems in argp_parse

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 13.04.2015 19:49, Andrei Borzenkov wrote:
> В Mon, 13 Apr 2015 19:16:02 +0200 (CEST)
> Dietmar Maurer <dietmar@proxmox.com> пишет:
> 
>>>> After reverting the following commit everything works again:
>>>>
>>>> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=cf47a2fba5852014bc59959c5e357e8313933414
>>>>
>>>> Any ideas?
>>>>
>>>
>>> Does it work if you remove "--"?
>>>
>>
>> no
>>
> 
> It chokes on -V which is "display version and exit" in argp. I still
> believe this patch was mistake; it leaves no possibility to
> disambiguate between grub and external options.
> 
> Vladimir?
> 
I believe that behavioural change was a mistake. In all I kinda like
that grub-mkrescue would be drop-in replacement for xorrisofs/mkisofs
with additional boot semantics. Unfortunately it requires additional
discipline of avoiding option clashes like this one.
I fixed this bug.
In principle I'm ok with having more elegant command line if the name of
tool changes.
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

Re: grub-mkrescue problems in argp_parse

[Thomas Schmitt]

Hi,

Vladimir wrote:
> In all I kinda like
> that grub-mkrescue would be drop-in replacement for xorrisofs/mkisofs
> with additional boot semantics. Unfortunately it requires additional
> discipline of avoiding option clashes like this one.

You would have to reserve option names within xorriso's
command set and its -as mkisofs option set. I would then
avoid to give them a meaning in those sets.
You would also have to resolve the old collision -v.

There are further incompatibilities between old and new
grub-mkrescue by the not yet implemented old options 
  --modules
  --grub-mkimage
  --override-directory
Will they be re-introduced ? (Should they ? What did they do ?)


> In principle I'm ok with having more elegant command line if the name of
> tool changes.

Please count 1 vote for "grub-mkiso". :))
As said, both personalities could share one body to avoid
duplication of maintainance effort.

I propose to describe the new CLI in the docs as "unambiguous"
rather than as "more elegant".


Have a nice day :)

Thomas

Re: grub-mkrescue problems in argp_parse

[Andrei Borzenkov]

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

В Fri, 24 Apr 2015 17:54:28 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> On 13.04.2015 19:49, Andrei Borzenkov wrote:
> > В Mon, 13 Apr 2015 19:16:02 +0200 (CEST)
> > Dietmar Maurer <dietmar@proxmox.com> пишет:
> > 
> >>>> After reverting the following commit everything works again:
> >>>>
> >>>> http://git.savannah.gnu.org/cgit/grub.git/commit/?id=cf47a2fba5852014bc59959c5e357e8313933414
> >>>>
> >>>> Any ideas?
> >>>>
> >>>
> >>> Does it work if you remove "--"?
> >>>
> >>
> >> no
> >>
> > 
> > It chokes on -V which is "display version and exit" in argp. I still
> > believe this patch was mistake; it leaves no possibility to
> > disambiguate between grub and external options.
> > 
> > Vladimir?
> > 
> I believe that behavioural change was a mistake. In all I kinda like
> that grub-mkrescue would be drop-in replacement for xorrisofs/mkisofs
> with additional boot semantics. Unfortunately it requires additional
> discipline of avoiding option clashes like this one.
> I fixed this bug.

Now help text does not match.

> In principle I'm ok with having more elegant command line if the name of
> tool changes.
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> > 
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: grub-mkrescue problems in argp_parse

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 24.04.2015 18:31, Thomas Schmitt wrote:
> Hi,
> 
> Vladimir wrote:
>> In all I kinda like
>> that grub-mkrescue would be drop-in replacement for xorrisofs/mkisofs
>> with additional boot semantics. Unfortunately it requires additional
>> discipline of avoiding option clashes like this one.
> 
> You would have to reserve option names within xorriso's
> command set and its -as mkisofs option set. I would then
> avoid to give them a meaning in those sets.
> You would also have to resolve the old collision -v.
> 
-v collision is fixed.
We need for compatibility:
-o
-?
-d
-k
--output
--rom-directory
--xorriso
--grub-glue-efi
--grub-render-label
--label-font
--label-color
--label-bgcolor
--product-name
--product-version
--sparc-boot
--arcs-boot
--modules
--install-modules
--themes
--fonts
--locales
--compress
--core-compress
--directory
--override-directory
--locale-directory
--themes-directory
--grub-mkimage
--pubkey
--verbose

Are any of those a problem? In future we can make a rule to add only
options starting with --. We can add a test to ensure this. Do we need
any other requirements to avoid clashes?

> There are further incompatibilities between old and new
> grub-mkrescue by the not yet implemented old options 
>   --modules
>   --grub-mkimage
>   --override-directory
> Will they be re-introduced ? (Should they ? What did they do ?)
> 
They're there. If you have data suggesting otherwise, please detail it
> 
>> In principle I'm ok with having more elegant command line if the name of
>> tool changes.
> 
> Please count 1 vote for "grub-mkiso". :))
> As said, both personalities could share one body to avoid
> duplication of maintainance effort.
> 
> I propose to describe the new CLI in the docs as "unambiguous"
> rather than as "more elegant".
> 
> 
> Have a nice day :)
> 
> Thomas
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> .
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

Re: grub-mkrescue problems in argp_parse

[Andrei Borzenkov]

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

В Wed, 29 Apr 2015 16:05:04 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> Are any of those a problem? In future we can make a rule to add only
> options starting with --. We can add a test to ensure this. Do we need
> any other requirements to avoid clashes?
> 

Not sure what do you mean. The right way to avoid clashes is to follow
standard established practice of using "--" to signal end of options.
Any new tool in grub using argp will follow it automatically.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: grub-mkrescue problems in argp_parse

[Thomas Schmitt]

Hi,

> We need for compatibility:
> -o
> --output

This is an intentional interference because grub-mkrescue -o
is supposed to overload mkisofs -o.
--output is not a mkisofs alias of -o, but rather -output.

So in order to keep grub-mkrescue users from circumventing
the overloading, grub-mkrescue should accept -output as alias
of -o and --output.


> -?

Yikes ! You can have that one.

> -d

Used in old mkisofs.
"Do not add trailing dot to ISO file names without dot."
An alias is available: -omit-period

> -k

Seems to be unused in traditional mkisofs and genisoimage.
I do not add single-letter options on my own.


> --rom-directory
> ...
> --verbose

Not in use yet by xorriso resp. xorriso -as mkisofs .
Some of the option names are quite unspecific, though.


> In future we can make a rule to add only options starting with --.

This will not really create clarity because i already added
options with "--" to -as mkisofs. Mainly to get out of
the way of future mkisofs extensions. The current option
set of xorriso -as mkisofs is strictly irregular.

The generic command interpreter of xorriso ignores leading '-'
completely and maps inner '-' to underscores '_'.
I.e.  set_filter_r , ----set-filter_r, --set-filter-r
are all recognized as valid command -set_filter_r.
(The interpreter can afford this because it is always aware
 when the next word is supposed to be a command.)
 

> > --modules --grub-mkimage --override-directory
> > Will they be re-introduced ? (Should they ? What did they do ?)
> They're there. If you have data suggesting otherwise, please detail it

You are right. I did not see them in
  http://git.savannah.gnu.org/cgit/grub.git/tree/util/grub-mkrescue.c
but now found them in
  http://git.savannah.gnu.org/cgit/grub.git/tree/include/grub/util/install.h
  http://git.savannah.gnu.org/cgit/grub.git/tree/util/grub-install-common.c

(Would it be ok and sufficient to call grub_install_parse ()
 from my proposed legacy parser when it encounters one of
 the three options ?)


---------------------------------------------------------

I have now made a file with the reserved option/command
names. Hopefully i remember to check it when i introduce
new names. (If not, i will have to solve collisions by
introducing aliases and deprecating my bad choices.)

Nevertheless i doubt that it is a good idea to stick for
future development with the implicit distinction between
arguments for grub-mkrescue and for xorriso.
This has potential for recurrent pain.


Have a nice day :)

Thomas

Re: grub-mkrescue problems in argp_parse

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

> Used in old mkisofs.
> "Do not add trailing dot to ISO file names without dot."
> An alias is available: -omit-period
> 
Or user can use --d
>> In future we can make a rule to add only options starting with --.
> 
> This will not really create clarity because i already added
> options with "--" to -as mkisofs. Mainly to get out of
> the way of future mkisofs extensions. The current option
> set of xorriso -as mkisofs is strictly irregular.
> 
In retrospect we should have put a --grub-<grub_option> there. But OTOH
--grub- makes options that longer. For rarely used options it's not a
problem though. Can we have --grub* space?



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 213 bytes --]

Re: grub-mkrescue problems in argp_parse

[Andrei Borzenkov]

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

В Thu, 07 May 2015 16:40:29 +0200
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> > Used in old mkisofs.
> > "Do not add trailing dot to ISO file names without dot."
> > An alias is available: -omit-period
> > 
> Or user can use --d
> >> In future we can make a rule to add only options starting with --.
> > 
> > This will not really create clarity because i already added
> > options with "--" to -as mkisofs. Mainly to get out of
> > the way of future mkisofs extensions. The current option
> > set of xorriso -as mkisofs is strictly irregular.
> > 
> In retrospect we should have put a --grub-<grub_option> there. But OTOH
> --grub- makes options that longer. For rarely used options it's not a
> problem though. Can we have --grub* space?
> 
> 
util/grub-mkrescue.c: In function ‘main’:
util/grub-mkrescue.c:455:27: error: assignment discards ‘const’ qualifier from pointer target type [-Werror]
    argp_argv[argp_argc++] = "--output";
                           ^

Folks, I really suggest to bite the bullet and switch to argp. There is
nothing wrong in changing syntax once in a lifetime; moreover, I expect
that most users who actually care already adapted to new syntax - we
have it for one and half year. So you just introduced yet another
incompatibility without really fixing anything.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: grub-mkrescue problems in argp_parse

[Thomas Schmitt]

Hi,

> Can we have --grub* space?

No hard collision to see in the man pages of xorriso and xorrisofs. 

Except the word "grub" with command -boot_image,
there are only some--grub2-* options of -as mkisofs:
  --grub2-boot-info
  --grub2-mbr
  --grub2-sparc-core
All three are supposed to be under control of grub-mkrescue,
and not of its caller. So their use as addon arguments can be
considered as having been discouraged since the beginnings
of grub-mkrescue.


Have a nice day :)

Thomas

Re: grub-mkrescue problems in argp_parse

[Thomas Schmitt]

Hi,

> Folks, I really suggest to bite the bullet and switch to argp.

I already gave in to your reasoning for the better CLI. :))

But since Vladimir is not yet annoyed enough by the consequences
of the grub-mkrescue CLI, i provide my part of our cooperation.

(I am with Vladimir that grub-mkrescue should stay compatible
 with the one of GRUB2 release 2.00. But this can be easily
 done by introducing the better CLI by a new program name.)


Have a nice day :)

Thomas