Bug 458472 - Review Request: grub2 - Bootloader with support for Linux, Multiboot and more
Review Request: grub2 - Bootloader with support for Linux, Multiboot and more
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Marek Mahut
Fedora Extras Quality Assurance
:
: 228255 (view as bug list)
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-08-08 13:41 EDT by Lubomir Rintel
Modified: 2008-08-27 04:55 EDT (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-08-27 04:55:15 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
mmahut: fedora_requires_release_note-
mmahut: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Lubomir Rintel 2008-08-08 13:41:23 EDT
SPEC: http://netbsd.sk/~lkundrak/SPECS/grub2.spec
SRPM: http://netbsd.sk/~lkundrak/SRPMS/grub2-1.98-0.1.20080807svn.el5.src.rpm

Description:

This is the second version of the GRUB (Grand Unified Bootloader),
a highly configurable and customizable bootloader with modular
architecture.  It support rich scale of kernel formats, file systems,
computer architectures and hardware devices.
Comment 1 Jason Tibbitts 2008-08-08 13:57:45 EDT
*** Bug 228255 has been marked as a duplicate of this bug. ***
Comment 2 Till Maas 2008-08-08 14:22:59 EDT
Maybe you need trigger-scriptlets with kernel-PAE, too. Also I guess you need

Requires(post):       mkinitrd
Requires(preun):       mkinitrd

because you use grubby in preun and post.
Comment 3 Lubomir Rintel 2008-08-08 18:29:41 EDT
Thanks, Till, your suggestions have been integrated in the new revision. Marek, this now builds on x86_64.

SPEC: http://netbsd.sk/~lkundrak/SPECS/grub2.spec
SRPM: http://netbsd.sk/~lkundrak/mock-results/grub2-1.98-0.2.20080807svn.el5.x86_64/grub2-1.98-0.2.20080807svn.el5.src.rpm

mock build with build logs:
http://netbsd.sk/~lkundrak/mock-results/grub2-1.98-0.2.20080807svn.el5.x86_64/
Comment 4 Till Maas 2008-08-08 18:56:22 EDT
I just read on Fedoras Planet, that there will a kernel-vanilla package soon, maybe you keep this in mind for the next update of your spec.
Comment 5 Lubomir Rintel 2008-08-09 03:01:28 EDT
Till, right, there's also memtest, and probably some more packages, that make sense to be loaded by GRUB. Maybe it would make sense to adjust grubby for grub2's grub.cfg, and not use update-grub at all. But yes, I think I'll just add those triggers for now, not to deviate from upstream at this point.
Comment 6 Jason Tibbitts 2008-08-09 10:30:02 EDT
Please remember to set the review flag to '?' when you're reviewing.
Comment 7 Adam Jackson 2008-08-11 09:55:51 EDT
Dare I suggest that we really don't need more bootloaders?  Particularly when they have such a high maintenance cost in the installer, and (in this particular case) it offers no real features over the grub we've got.
Comment 8 Lubomir Rintel 2008-08-11 10:30:46 EDT
Adam: even though at the time GRUB 2 does not have any killer features over GRUB, GRUB 2 has more active code base, that has potential to be shared across distributions (unlike GRUB whose official upstream had been long dead until very recently, and developed into various vendor-specific forks each carrying a load of custom patches, often resulting in regressions).

Some of interesting features GRUB 2 has now is native support for UTF-8, support for variety of graphic modes, RAID and LVM support.

Note that including GRUB 2 in distribution does not imply any support in the installer. GRUB 2 kernel can be booted from withing GRUB in in the same way as Linux kernel. This package does not intend to replace GRUB (at least until it is feature-complete), but to serve users and developers that are interested in GRUB 2's features.
Comment 9 Till Maas 2008-08-11 10:38:05 EDT
Afaik grub2 supports booting directly from iso files, which is feature I would like to have very much to boot live cds.
Comment 10 Marek Mahut 2008-08-17 04:28:48 EDT
I agree with Till, we have to include this in distribution at least for QA purpose. It may become once the official boot loader and replace dead grub. 

The final version of package looks sane, Lubomir please include patch comments in the spec file as we agreed on IRC. This package is approved, please include a note about grub2 availability in release notes for Fedora 9.

(I'll help with testing once I'm back from my vacation.)
Comment 11 Lubomir Rintel 2008-08-17 08:13:59 EDT
New Package CVS Request
=======================
Package Name: grub2
Short Description: Bootloader with support for Linux, Multiboot and more
Owners: lkundrak
Branches: F-9 EL-5
Comment 12 Bill Nottingham 2008-08-20 20:00:30 EDT
I think release notes for a bootloader that won't be the default, won't work on some machines that the normal one does, won't be supported by any of the tools... would be rather silly. (Much like including the package, but... eh.)
Comment 13 Kevin Fenzi 2008-08-23 13:30:24 EDT
I'm a bit worried here about user confusion... what happens if someone removes grub and tries to just use grub2? (I would assume it would fail due at least to lack of selinux support). Have you tested how it would fail in that case... 

many users assume higher versions are better and will update to them even in the absense of any problems with the older version. ;(
Comment 14 Lubomir Rintel 2008-08-23 16:45:28 EDT
(In reply to comment #13)
> I'm a bit worried here about user confusion... what happens if someone removes
> grub and tries to just use grub2?

If anyone removes grub, he should be aware that he needs to install another bootloader (with grub-install). That's also true for lilo, or possibly even newer version of grub.

> (I would assume it would fail due at least to
> lack of selinux support).

Good point; as far as I know, grub2 needs an executable stack, due to use of nested functions. I'll need to eventually verify this and open a ticket against selinux-policy.

> Have you tested how it would fail in that case... 
> 
> many users assume higher versions are better and will update to them even in
> the absense of any problems with the older version. ;(

Apart from version number, grub and grub2 also differ in package name, and that's generally enough to distinguish that you are dealing with two different pieces of software. An ordinary desktop user won't even notice the presence of the grub2 package unless he wants to.
Comment 15 Kevin Fenzi 2008-08-23 16:53:49 EDT
Adding the grub maintainer here for comment. ;)
Comment 16 Peter Jones 2008-08-23 21:21:44 EDT
Adding another bootloader for x86 that we're not supporting in grubby and booty is just a mistake.
Comment 17 Lubomir Rintel 2008-08-24 09:03:56 EDT
I'd say it's a mistake to add support for a bootloader that's not packaged into grubby.
Comment 18 Kevin Fenzi 2008-08-25 15:50:24 EDT
ok, so where do we stand here? Shall I process the cvs request? 
Or do we think this package is not a good idea to add right now?
Comment 19 Jerone Young 2008-08-25 16:25:00 EDT
The consencous is that before grub2 is to go in, support needs to be added for tools (ex grubby). Then it should easily be able to go in. But fedora userspace tools need to support grub2 config files before going into fedora.
Comment 20 Lubomir Rintel 2008-08-25 18:29:04 EDT
(In reply to comment #19)
> The consencous is that before grub2 is to go in, support needs to be added for
> tools (ex grubby). Then it should easily be able to go in. But fedora userspace
> tools need to support grub2 config files before going into fedora.

No. It's just your opinion.

Please understand that the new grub2 package just adds some optional features. Noone forces you to do so, so if you disagree with the presence of a package, the nice thing about it is that you do not have to install it.

In case you feel grubby support is important at this point, you, or anyone, can add it whenever you want to provided your patch is accepted by the grubby maintainer, but please do not use it as argument, unless it is a packaging-comitee approved guideline. I find it highly odd to add support for a nonexistent package, and am not going to do so.

In case you question the quality of the review, please point out where does it violate the guidelines. I value comments with valuable opinions, but unsupported assertion presented as "consensus" is of hardly any use.

So, to summarize:

1.) grub2 is usable without grubby support
  - see distribution-agnostic grub2-update
2.) There actually are userspace tools to support grub2
  - see grub2-update and $EDITOR
3.) grub2 package does not interfere with anything already in base
  - none of its provides overlays any existing package, including grub
  - it does not replace grub2
4.) grub2 was properly reviewed and approved

Kevin: given the above explanation, please process the CVS request (unless a package guidelines violation is found, which I believe to be unlikely after a relatively long this request is open)
Comment 21 Jerone Young 2008-08-25 19:00:41 EDT
@Lubomir
 
   You have to rembmer I was the first to bring grub2 to fedora last year (as I am a grub2 maintainer .. though not very active as of late ... see bug 228255).

  While yes you can use grub2 with it's own tools manually, what happens when a new fedora kernel is installed? It doesn't run those tools to update the grub2 config. 

   My point is this issue is integration into the distribution. The grub2 tools are not apart of the distribution process and so this is where the issues are. Making these tools or updating the process to use the tools is what will allow grub2 to go into fedora.

  You have to stop just thinking about the project, and understand that it is not just a standalone tool. But must be integrated into the processes that fedora uses. Which is why there are comments about grubby & booty support.
Comment 22 Lubomir Rintel 2008-08-26 03:24:10 EDT
(In reply to comment #21)
> @Lubomir
> 
>    You have to rembmer I was the first to bring grub2 to fedora last year (as I
> am a grub2 maintainer .. though not very active as of late ... see bug 228255).

Right. Your package did not pass the review.

>   While yes you can use grub2 with it's own tools manually, what happens when a
> new fedora kernel is installed? It doesn't run those tools to update the grub2
> config. 

Are you sure?

>    My point is this issue is integration into the distribution.

You can not integrate nonexistent parts.

> The grub2 tools
> are not apart of the distribution process and so this is where the issues are.
> Making these tools or updating the process to use the tools is what will allow
> grub2 to go into fedora.

I'd say we developed the review process to judge which parts do we allow into fedora.

>   You have to stop just thinking about the project, and understand that it is
> not just a standalone tool.

It can be.

> But must be integrated into the processes that
> fedora uses. Which is why there are comments about grubby & booty support.

To reiterate: You can not integrate nonexistent parts. The guidelines don't require you to do so.

Furthermore, currently it integrates at the level which is sufficient for most use cases, and having the package in distribution encourages more integration.

And at last; You obviously haven't even bothered looking at the package or reading the spec file, so I do not consider you a discussion peer here anymore. Please don't put useless and uninformed comments here anymore.
Comment 23 Marek Mahut 2008-08-26 04:01:23 EDT
(In reply to comment #21)
[...]
>    My point is this issue is integration into the distribution. The grub2 tools
> are not apart of the distribution process and so this is where the issues are.
> Making these tools or updating the process to use the tools is what will allow
> grub2 to go into fedora..


I don't agree with you, this is not a part of default core system, it's an additional package that will be primary used for hacking and over-time to adjust our tools (which are closed to community commits, btw). I confirm that I'm approving this package.
Comment 24 Jerone Young 2008-08-26 08:21:46 EDT
@Marek
   Haha. No just just showing where I was coming from. Not to stroke my ego.
Comment 25 Kevin Fenzi 2008-08-26 19:28:25 EDT
ok, cvs done. 

I still think this could well confuse users, but I don't have any hard objections.
Comment 26 Lubomir Rintel 2008-08-27 04:55:15 EDT
Thanks, imported and built.
Description was extended and a README file was added to avoid the confusion as much as possible.

Note You need to log in before you can comment on or make changes to this bug.