Install_c merge intention

Install_c merge intention

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

Hello, all. install_c is feature complete and I intent to make it
upstream unless issues are raised until 16 Nov. It's available under
http://git.savannah.gnu.org/cgit/grub.git/log/?h=phcoder/install_c


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

Re: Install_c merge intention

[Andrey Borzenkov]

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

В Sat, 09 Nov 2013 13:51:09 +0100
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> Hello, all. install_c is feature complete and I intent to make it
> upstream unless issues are raised until 16 Nov. It's available under
> http://git.savannah.gnu.org/cgit/grub.git/log/?h=phcoder/install_c
> 

Does not build here

util/grub-mkimage.c:48:29: fatal error: grub/emu/config.h: Нет такого файла или каталога
compilation terminated.

HEAD is 1622e97606772a1a1c841ba15c67b8d5b419a027.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Install_c merge intention

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 10.11.2013 05:58, Andrey Borzenkov wrote:
> В Sat, 09 Nov 2013 13:51:09 +0100
> Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:
> 
>> Hello, all. install_c is feature complete and I intent to make it
>> upstream unless issues are raised until 16 Nov. It's available under
>> http://git.savannah.gnu.org/cgit/grub.git/log/?h=phcoder/install_c
>>
> 
> Does not build here
> 
> util/grub-mkimage.c:48:29: fatal error: grub/emu/config.h: Нет такого файла или каталога
> compilation terminated.
> 
It was improperly ignored. Added it and pushed.
Thanks.
> HEAD is 1622e97606772a1a1c841ba15c67b8d5b419a027.
> 
> 
> 
> _______________________________________________
> 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: 291 bytes --]

[PATCH] fix qsort invocation in grub-install

[Andrey Borzenkov]

Order of elements number and size is reversed.

---
 util/grub-install.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util/grub-install.c b/util/grub-install.c
index 11010cc..87a0b3b 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -560,7 +560,7 @@ device_map_check_duplicates (const char *dev_map)
 
   fclose (fp);
 
-  qsort (d, sizeof (d[0]), filled, (int (*) (const void *, const void *))strcmp);
+  qsort (d, filled, sizeof (d[0]), (int (*) (const void *, const void *))strcmp);
 
   for (i = 0; i + 1 < filled; i++)
     if (strcmp (d[i], d[i+1]) == 0)
-- 
tg: (24685ac..) u/install_c-qsort (depends on: phcoder/install_c)

Re: [PATCH] fix qsort invocation in grub-install

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 10.11.2013 15:26, Andrey Borzenkov wrote:
> Order of elements number and size is reversed.
> 
Go ahead.
> ---
>  util/grub-install.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util/grub-install.c b/util/grub-install.c
> index 11010cc..87a0b3b 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -560,7 +560,7 @@ device_map_check_duplicates (const char *dev_map)
>  
>    fclose (fp);
>  
> -  qsort (d, sizeof (d[0]), filled, (int (*) (const void *, const void *))strcmp);
> +  qsort (d, filled, sizeof (d[0]), (int (*) (const void *, const void *))strcmp);
>  
>    for (i = 0; i + 1 < filled; i++)
>      if (strcmp (d[i], d[i+1]) == 0)
> 



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

Re: Install_c merge intention

[Andrey Borzenkov]

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

В Sat, 09 Nov 2013 13:51:09 +0100
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> Hello, all. install_c is feature complete and I intent to make it
> upstream unless issues are raised until 16 Nov. It's available under
> http://git.savannah.gnu.org/cgit/grub.git/log/?h=phcoder/install_c
> 

As was discussed before

- shell grub-istall provides relatively easy way to track what is done
  (which modules are selected etc). Binary grub-install does not. It
  was suggested to enable easy access to this information (like echoing
  invocations of grub-mkimage and grub-setup). This is missing
  currently. 

- looks like shell backticks in /etc/default/grub are not evaluated or
  I missed it. Only normal shell quoting is interpreted.

- I guess before merging obsolete shell commands should be removed?
  They are still present.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Install_c merge intention

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 10.11.2013 15:52, Andrey Borzenkov wrote:
> В Sat, 09 Nov 2013 13:51:09 +0100
> Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:
> 
>> Hello, all. install_c is feature complete and I intent to make it
>> upstream unless issues are raised until 16 Nov. It's available under
>> http://git.savannah.gnu.org/cgit/grub.git/log/?h=phcoder/install_c
>>
> 
> As was discussed before
> 
> - shell grub-istall provides relatively easy way to track what is done
>   (which modules are selected etc). Binary grub-install does not. It
>   was suggested to enable easy access to this information (like echoing
>   invocations of grub-mkimage and grub-setup). This is missing
>   currently. 
>
I added it.  Is the one I added enough?

> - looks like shell backticks in /etc/default/grub are not evaluated or
>   I missed it. Only normal shell quoting is interpreted.
> 
There are 2 routines for loading config: Use sh helper or read directly.
We use sh helper when on unix-like OS and sh is available.
> - I guess before merging obsolete shell commands should be removed?
>   They are still present.
> 
Yes, it's to avoid messing with conflicts in deleted files. They'll be
removed in merge
> 
> 
> _______________________________________________
> 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: 291 bytes --]

Re: Install_c merge intention

[Andrey Borzenkov]

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

В Sun, 10 Nov 2013 16:38:09 +0100
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> On 10.11.2013 15:52, Andrey Borzenkov wrote:
> > В Sat, 09 Nov 2013 13:51:09 +0100
> > Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:
> > 
> >> Hello, all. install_c is feature complete and I intent to make it
> >> upstream unless issues are raised until 16 Nov. It's available under
> >> http://git.savannah.gnu.org/cgit/grub.git/log/?h=phcoder/install_c
> >>
> > 
> > As was discussed before
> > 
> > - shell grub-istall provides relatively easy way to track what is done
> >   (which modules are selected etc). Binary grub-install does not. It
> >   was suggested to enable easy access to this information (like echoing
> >   invocations of grub-mkimage and grub-setup). This is missing
> >   currently. 
> >
> I added it.  Is the one I added enough?
> 

Yes, thank you. May be having basic verbose level that just prints them
would be better, but that's something that can be implemented later.

> > - looks like shell backticks in /etc/default/grub are not evaluated or
> >   I missed it. Only normal shell quoting is interpreted.
> > 
> There are 2 routines for loading config: Use sh helper or read directly.
> We use sh helper when on unix-like OS and sh is available.

Yes, I missed it.

> > - I guess before merging obsolete shell commands should be removed?
> >   They are still present.
> > 
> Yes, it's to avoid messing with conflicts in deleted files. They'll be
> removed in merge
> > 
> > 
> > _______________________________________________
> > Grub-devel mailing list
> > Grub-devel@gnu.org
> > https://lists.gnu.org/mailman/listinfo/grub-devel
> > 
> 
> 


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Install_c merge intention

[Andrey Borzenkov]

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

В Sat, 09 Nov 2013 13:51:09 +0100
Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:

> Hello, all. install_c is feature complete and I intent to make it
> upstream unless issues are raised until 16 Nov. It's available under
> http://git.savannah.gnu.org/cgit/grub.git/log/?h=phcoder/install_c
> 

Hmm ... I noticed that you set quite a number of options as
OPTION_HIDDEN. That is fine, they are not meant for everyday use, but
how should users become aware of them? Read the source is not really the
option here (oops, pun unintended). They are not documented in grub.info
either.

I hoped arpg provides some way to show them (--help-all or similar) but
apparently not.

Those options were shown before so it can be considered regression.

Any ideas how to handle this?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: Install_c merge intention

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 16.11.2013 20:52, Andrey Borzenkov wrote:
> В Sat, 09 Nov 2013 13:51:09 +0100
> Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:
> 
>> Hello, all. install_c is feature complete and I intent to make it
>> upstream unless issues are raised until 16 Nov. It's available under
>> http://git.savannah.gnu.org/cgit/grub.git/log/?h=phcoder/install_c
>>
> 
> Hmm ... I noticed that you set quite a number of options as
> OPTION_HIDDEN. That is fine, they are not meant for everyday use, but
> how should users become aware of them? Read the source is not really the
> option here (oops, pun unintended). They are not documented in grub.info
> either.
> 
> I hoped arpg provides some way to show them (--help-all or similar) but
> apparently not.
> 
> Those options were shown before so it can be considered regression.
> 
> Any ideas how to handle this?
> 
The removed options is because instead of using external tools grub-*
the C version simply links with appropriate C files and uses functions
directly and so gets away with trouble of locating binaries.
> 
> 
> _______________________________________________
> 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: 291 bytes --]

Re: Install_c merge intention

[Vladimir 'φ-coder/phcoder' Serbinenko]

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

On 16.11.2013 20:52, Andrey Borzenkov wrote:
> В Sat, 09 Nov 2013 13:51:09 +0100
> Vladimir 'φ-coder/phcoder' Serbinenko <phcoder@gmail.com> пишет:
> 
>> Hello, all. install_c is feature complete and I intent to make it
>> upstream unless issues are raised until 16 Nov. It's available under
>> http://git.savannah.gnu.org/cgit/grub.git/log/?h=phcoder/install_c
>>
> 
> Hmm ... I noticed that you set quite a number of options as
> OPTION_HIDDEN. That is fine, they are not meant for everyday use, but
> how should users become aware of them? Read the source is not really the
> option here (oops, pun unintended). They are not documented in grub.info
> either.
> 
> I hoped arpg provides some way to show them (--help-all or similar) but
> apparently not.
> 
> Those options were shown before so it can be considered regression.
> 
> Any ideas how to handle this?
> 
I added --no-bootsector for skipping grub-setup stage and accept
incredible arguments to grub-setup as synonym to --no-bootsector
> 
> 
> _______________________________________________
> 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: 291 bytes --]

Re: Install_c merge intention

[Jonathan McCune]

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

On Sat, Nov 9, 2013 at 4:51 AM, Vladimir 'φ-coder/phcoder' Serbinenko <
phcoder@gmail.com> wrote:

> Hello, all. install_c is feature complete and I intent to make it
> upstream unless issues are raised until 16 Nov. It's available under
> http://git.savannah.gnu.org/cgit/grub.git/log/?h=phcoder/install_c


I just realized that the option --grub-mkimage is now accepted silently
without having any effect in
util/grub-install-common.c:grub_install_parse(). This makes it no longer
feasible to easily wrap the mkimage command.  While most of the meaningful
reasons to wrap grub-mkimage have now been added to grub-install, I've run
into something I'm struggling to work around:  How do I add custom contents
to a load.cfg, since util/grub-install.c seems to unconditionally
grub_util_unlink(load_cfg)?

As a second concern, I think it is more appropriate that passing
--grub-mkimage to grub-install is now an error. Existing scripts that use
that argument presumably do so for a reason, and it has taken me longer
than I would have liked to invest to root cause why my wrapper was not
being invoked.  I'll follow up with a (tiny) patch.

Thanks,
-Jon

[-- Attachment #2: Type: text/html, Size: 1771 bytes --]

Re: Install_c merge intention

[Andrey Borzenkov]

On Mon, Dec 2, 2013 at 11:09 PM, Jonathan McCune <jonmccune@google.com> wrote:
> On Sat, Nov 9, 2013 at 4:51 AM, Vladimir 'φ-coder/phcoder' Serbinenko
> <phcoder@gmail.com> wrote:
>>
>> Hello, all. install_c is feature complete and I intent to make it
>> upstream unless issues are raised until 16 Nov. It's available under
>> http://git.savannah.gnu.org/cgit/grub.git/log/?h=phcoder/install_c
>
>
> I just realized that the option --grub-mkimage is now accepted silently
> without having any effect in
> util/grub-install-common.c:grub_install_parse(). This makes it no longer
> feasible to easily wrap the mkimage command.  While most of the meaningful
> reasons to wrap grub-mkimage have now been added to grub-install, I've run
> into something I'm struggling to work around:  How do I add custom contents
> to a load.cfg, since util/grub-install.c seems to unconditionally
> grub_util_unlink(load_cfg)?
>

Could you provide examples of your grub.cfg usage? I think we need to
have more real-life use cases to understand better how and when it is
used.

> As a second concern, I think it is more appropriate that passing
> --grub-mkimage to grub-install is now an error. Existing scripts that use
> that argument presumably do so for a reason, and it has taken me longer than
> I would have liked to invest to root cause why my wrapper was not being
> invoked.  I'll follow up with a (tiny) patch.
>
> Thanks,
> -Jon
>
>
>
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>