After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 762220 - Allow for more flexibility in generating grub.cfg
Allow for more flexibility in generating grub.cfg
Status: RESOLVED FIXED
Product: ostree
Classification: Infrastructure
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: OSTree maintainer(s)
OSTree maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2016-02-17 19:19 UTC by Gatis Paeglis
Modified: 2016-04-13 12:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch to disable grub2-mkconfig usage (1.97 KB, patch)
2016-02-17 19:19 UTC, Gatis Paeglis
reviewed Details | Review
sample configuration generator (1.67 KB, application/x-shellscript)
2016-02-17 19:20 UTC, Gatis Paeglis
  Details

Description Gatis Paeglis 2016-02-17 19:19:13 UTC
Created attachment 321540 [details] [review]
Patch to disable grub2-mkconfig usage

Hard-coding grub settings in _ostree_bootloader_grub2_generate_config() is not an optimal solution.
   
Comment from "grub2-15_ostree":

# Pick up stuff from grub's helper that we want to inject into our
# generated bootloader configuration.  Yes, this is pretty awful, but
# it's a lot better than reimplementing the config-generating bits of
# OSTree in shell script.

I tried reimplementing this logic in shell and it results in more flexible approach
in my experience, see the attached patch with sample 'ostree-grub-generator' configuration.

This custom ostree-grub-generate script can call grub2-mkconfig if desired. The attached sample file simply converts boot loader entries into grub.cfg and adds custom header. Yocto scripts for example do not attempt to chroot sysroot, it simply runs python script to generate grub.cfg where some values can be altered via exposed variables, so similar to ostree-grub-generator.

The attached patch is a sample to demonstrate the approach, a final patch would need to keep a backward compatibility with grub2-mkconfig approach. 

if (OSTREE_NO_GRUB2_MKCONFIG)
   procctx = gs_subprocess_context_newv ("ostree-grub-generator", "-o",
else
   procctx = gs_subprocess_context_newv ("grub2-mkconfig", "-o",

This case relates to:
https://bugzilla.gnome.org/show_bug.cgi?id=761180
Comment 1 Gatis Paeglis 2016-02-17 19:20:16 UTC
Created attachment 321541 [details]
sample configuration generator
Comment 2 Colin Walters 2016-02-19 02:11:59 UTC
Review of attachment 321540 [details] [review]:

A compile time option like --with-grub2-generator=/usr/libexec/myos-grub2-generate would be pretty simple, right?  Then it'd just default to grub2-generate.  And if provided we'd not install 15_ostree.

I'm a little uncertain about encouraging people to parse the loader snippets as well.  But, they're intentionally simple, so eh.
Comment 3 Colin Walters 2016-02-19 02:22:11 UTC
It seems like what you're avoiding wrt performance is the grub2-probe stuff right?  Certainly on an embedded system it makes no sense to try to look for Windows (or other distributions, etc.) installations every time a kernel is installed...

I'm totally open to making this area of code flexible - my high level concern is that we avoid *too* many special cases and try to drive standardization across distributions as much as possible.
Comment 4 Gatis Paeglis 2016-02-29 11:41:32 UTC
I was thinking of having a new argument "--with-grub2-mkconfig" (this would be dafault), and if it is explicitly disabled --with-grub2-mkconfig=no, then use some other name for a script to execute, like "ostree-grub-generator". ostree-grub-generator should be name that does not change, it is OS installer task to do something like:

cp ostree-grub-generator-intel-nuc /usr/sbin/ostree-grub-generator

Then it would be easier to find this script on a device, as it has one defined name. And thinking from build system side of view (for example yocto), it is simpler to write one recipe that has --with-grub2-mkconfig=no then adjusting:

--with-grub2-generator=/usr/libexec/my_os-grub2-generate
--with-grub2-generator=/usr/libexec/my_other_os-grub2-generate

for different device builds.

> It seems like what you're avoiding wrt performance is the grub2-probe stuff right?

That is one reason. Other is that I would prefer not to install grub package on a device.

>  my high level concern is that we avoid *too* many special cases and try to drive standardization across distributions as much as possible.

Embedded often is a special case. I think this approach adds reasonable level of flexibility and does not brake backward compatibility.
Comment 5 Colin Walters 2016-02-29 18:39:47 UTC
(In reply to gatis.paeglis@theqtcompany.com from comment #4)
> I was thinking of having a new argument "--with-grub2-mkconfig" (this would
> be dafault), and if it is explicitly disabled --with-grub2-mkconfig=no, then
> use some other name for a script to execute, like "ostree-grub-generator".
> ostree-grub-generator should be name that does not change, it is OS
> installer task to do something like:

That sounds fine to me, although I'd put it outside of $PATH i.e. /usr/libexec to avoid an extra character of typing in bash completion.

> That is one reason. Other is that I would prefer not to install grub package
> on a device.

Ah yes, true.  Indeed, grub2 has way too much internal static linking between the tools
 
> Embedded often is a special case. I think this approach adds reasonable
> level of flexibility and does not brake backward compatibility.

Agreed, it's OK by me.  Are you going to do an updated patch?
Comment 6 Gatis Paeglis 2016-02-29 20:42:20 UTC
> i.e. /usr/libexec to avoid an extra character of typing in bash completion.

This path does not exists on Ubuntu, or as it seems any debian based distribution. Not sure if there are other standard path with a similar purpose. Maybe that script should have a check, if it was called from interactive shell then it prints warning and exits. But that does not solve typing extra character issue.

> Are you going to do an updated patch?

Yes, I can make a patch and push it on github, end of this week or next week.
Comment 7 Colin Walters 2016-02-29 21:01:00 UTC
(In reply to gatis.paeglis@theqtcompany.com from comment #6)
> > i.e. /usr/libexec to avoid an extra character of typing in bash completion.
> 
> This path does not exists on Ubuntu, or as it seems any debian based
> distribution.

Yeah, https://www.google.com/search?q=debian+libexec&ie=utf-8&oe=utf-8

Using $libdir/$(PACKAGE) is also fine.

(I've been meaning to move the switch-root binaries there for the same reason)
Comment 8 Gatis Paeglis 2016-03-22 15:51:08 UTC
Pull request in https://github.com/GNOME/ostree/pull/216 :)
Comment 9 Gatis Paeglis 2016-04-13 12:55:31 UTC
This has been fixed.