Bug 188445

Locales need to be handled with %find_lang macro as per the guidelines on the wiki.

Missing URL, and Source0 is not a URL. If the source archive is publicly
available or it has a web page these should be added.

changelog is missing the version number, 1.0=1 in this case.

rpmlint is complaining that usermode is not required, yet it is for the base
package. You may want to move the Requires usermode to the gui subpackage for
clarity.

Ok, I've repackaged and fixed things up.  An updated package is at the same URL,
http://www.courier-mta.com/bootconf-1.0-1.src.rpm

Although it did not occur to me that this script is a big enough of a deal to
merit its own web page, that's not a problem.  The URLs in the updated spec
file, and the revised tarball, are waiting for DNS to propagate, which should be
done by tomorrow.  In the meantime, here's the revised .src.rpm

I did not look at this very closely, though I should add that grubby (command
line app) and booty (python library) already support multiple boot loaders.  If
you would like to create a GUI for booty, that sounds like a good thing,
although I am not sure if grubby needs a replacement as it already works pretty
darn well.

It makes good sense, IMHO, to support multiple boot loaders, otherwise the app
seems to collide a bit with grubconf.  Per the grubconf page, grubconf seems to
be being replaced by one of the Gnome system tools for boot configuration:

http://www.gnome.org/projects/gst/screenshots/boot.jpg

Which looks like an upgrade to what is in FC5 today. 
"Administration/bootloader" is fairly useless as it only lets you pick which
bootloader to use.

bootconf is not a multiple bootloader configuration tool.

bootconf is a configuration tool for the kernel boot command line:
enable/disable rhgb boot; enable/disable quiet boot; enable/disable VESA
framebuffer.

I'm just trying to provide useful feedback here, so please take these comments
as such.

Yes, I understand it's *NOT* a multiple bootloader configuration tool, and I'm
asking *why* it isn't.  The kernel boot command line is owned by the bootloader,
and you're configuring the bootloader -- so the question is relevant.  Have you
looked at grubby and booty?  If not, please do.

I ran the app, and yes, it allows configuration of just *three* kernel boot
parameters.  There are many more kernel boot options than this, why pick just
these three?  This seems to be of limited value, and I'm asking these questions
to understand why the tools out there aren't already doing the job.  If we want
to provide human-understandable editing of these options for people who don't
know what they do, well, aren't these people capable of editing grub.conf?  For
those that aren't, a full list of arguments would make a lot more sense -- and
this should be integrated into something that is also capable of adding and
removing kernels/initrds and general bootloader configuration.

Only being able to edit 3 options doesn't seem useful to me.  Maybe I'm not
seeing something.  But if such a tool were to be included, I would expect it
should be able to include most *any* kernel option, and it should support
multiple bootloaders because such support is already provided free via grubby
and booty.  This would include kickstart options, root options, and so forth.

The code also is totally commentless, for what it's worth.  I did not look much
deeper than this because of the lack of comments, and that I am unable to run
pychecker on it.

From README:

"""
bootconf will search and update every "kernel" line in grub.conf, setting the
requested options.  When reading the existing boot configuration, bootconf
parses only the first kernel entry in grub.conf
"""

While the 3 arguments in question are fairly harmless, in general kernel
parameter customization should affect a selected kernel or a selected list of
kernels, not every kernel.  From running the GUI, that intended behavior is not
obvious.

The app also just throws an IOError when invoked with an account that can't read
/etc/grub.conf, versus allowing the user to authenticate or throwing up a more
helpful warning message.

<< why the tools out there aren't already doing the job. >>

Neither grubby, nor booty, based on my cursory overview, allows a non-technical
person to set common kernel boot options without mucking around with the command
line, making a typo, and hosing himself.

grubby isn't in base, or Extras.  I installed booty.  Where's the GUI, that will
let me choose to enable or disable the VESA framebuffer, or enable/disable the
boot verbosity?

It's possible that I missed it, but I wasn't able to find a convenient GUI
interface for setting kernel boot options.

As far as why those three -- it's because these three are the most common
options that someone might need to adjust.

<<< While the 3 arguments in question are fairly harmless, in general kernel
parameter customization should affect a selected kernel or a selected list of
kernels, not every kernel.  From running the GUI, that intended behavior is not
obvious. >>>

The intended audience is not a technical IT developer, who needs to screw around
with different kernels and options.  The intended audience is a NON-TECHNICAL,
desktop user who wants to adjust common, global, boot parameters.

(In reply to comment #6)
> grubby isn't in base, or Extras. 

Actually it is; it's bundled with mkinitrd. The kernel post-install scripts use
it (see /sbin/new-kernel-pkg).

I am not saying to use grubby or booty instead if a graphical app is desired by
you, but rather that grubby/booty achieves important bootloader independance and
more robust/safe editing of /etc/grub.conf -- and should be used as a backend if
this app is to be released.

For the record, anaconda uses booty.  That's all I'm going to add on this one.

The more proper way to disable rhgb is to set GRAPHICAL=no in
/etc/sysconfig/init rather than messing with boot options.

The boot option seems ment more for temporarily disabling rhgb, and would make
far more sense if it was called "norhgb" and had the opposite effect. ;P

Sorry, I'm not interested in reviewing this further at the moment.

Reassigning, back to FE-NEW.

It's been fifteen months since there was last activity on this ticket, and
twenty months since there was last any response from the submitter.

The package still builds, but it does elicit a few complaints from rpmlint
and will need tweaks for system changes in the past two years:

  bootconf.noarch: W: incoherent-version-in-changelog - 1.0-1
Changelog needs to indicate the version.

  bootconf.noarch: W: invalid-license GPL
The version of the GPL is required.

  bootconf.noarch: W: no-url-tag
Please unclude a URL: tag with a pointer to the upstream web site.

  bootconf-gui.noarch: W: no-documentation
Not a problem.

  bootconf-gui.noarch: W: non-conffile-in-etc /etc/pam.d/bootconf
  bootconf-gui.noarch: W: non-conffile-in-etc 
   /etc/security/console.apps/bootconf
These are OK.

  bootconf-gui.noarch: E: use-old-pam-stack /etc/pam.d/bootconf (line 10)
The pam_stack module isn't used these days; include should be used instead.

Also, the specfile is copyrighted and points to some other file for information.
 But I guess that file is buried in the tarball, which is rather suboptimal.  If
the specfile is under GPL then please include the required GPL notice.

It's been three months since the last comment; is there still interest in this
package?

Hmmm -- I seem to have missed the January ping.

Anyway, please take a look at
http://www.courier-mta.org/bootconf/download/bootconf-1.2-1.src.rpm

There's no need to wait months for me to respond. Generally, if I'm not heard
from in a week, it's time to pester me again.

Sorry, I was just doing triage on these old tickets, trying to find out whether
the submitters were still interested.

But this seems pretty simple, and most of the comments above seem to have been
addressed.  This builds fine on current F9/rawhide; rpmlint is down to:

  bootconf.src: W: strange-permission bootconf.spec 0600
I don't particularly care about this as long as it's not 666 or something like that.

  bootconf-gui.noarch: W: no-documentation
  bootconf-gui.noarch: W: non-conffile-in-etc /etc/pam.d/bootconf
  bootconf-gui.noarch: W: non-conffile-in-etc 
   /etc/security/console.apps/bootconf
All addressed as OK above.

The specfile still refers to a file which doesn't exist when you look at the
srpm.  You have to realize that you need to unpack the tarball and look there
for the license file.  I brought the topic on fedora-packaging and nobody else
stated an opinion so I won't block on it though I do think it's suboptimal. 
(Honestly there's not much that's copyrightable in a specfile anyway.)

If you're going to use the macro-ized forms of things like %{__make} then you
should use %{__rm} and %{__cat} as well.

There are a couple of issues with the scriptlets.  Firstly, for the scriptlets
you have, you need fine-grained dependencies for desktop-file-utils (like
"Requires(pre):".  However, the desktop-database section of
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets has some explanatory
text about making the scriptlets so that they don't depend on desktop-file-utils
; can you check that page and update accordingly?

I will also say that I understand previous comments about this duplicating
existing functionality, but I don't really have a problem with that.  If there
were a hard rule against it, we wouldn't have apt and such in the distro.

http://www.courier-mta.org/bootconf/download/bootconf-1.2-2.src.rpm

This should address everything except the permissions on the spec file. I don't
know why rpmbuild does this. The spec file is packaged inside the tarball, the
srpm gets created by rpmbuild -ta, and that's what you get.

The spec file has normal permissions inside the tarball. My shell umask is 002,
and that's what happens when my build script runs rpmbuild -ta. I don't know
what's going on.

OK, I found some time to finish this up. Things look mostly good; there are a
couple of really minor issues but at the end of my checklist I found one
problematic issue.  All of the things I found are below:

I don't know what's up with the specfile permissions; it's probably a bug in
either rpmbuild or rpmlint (since it probably should only complain about weird
permissions like 200 or security problems like mode 666).

You don't use the dist tag.  I assume you don't want to use it and know how to
juggle different specs between Fedora branches to preserve the upgrade path.

The %description for the -gui package could use a period, I guess.

Perhaps consider passing -p to install (both in the spec and in your Makefile)
to preserve timestamps.

And, the lone significant issue: The desktop file needs to be installed properly
with desktop-file-install; see
http://fedoraproject.org/wiki/Packaging/Guidelines#desktop for more info.  When
you do that, things should go OK but you will notice a warning:
  bootconf.desktop: warning: value "Application;System;" for key "Categories" in 
   group "Desktop Entry" contains a deprecated value "Application"
I don't think that warning is particularly problematic.  Sorry for not noticing
this earlier.

* source files match upstream:
   8bda663ecc7aa661200a0b230302b0bc8ca9ce8c20128e9c8fef2775214d9b58  
   bootconf-1.2.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK (-gui package could use a period).
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
  bootconf-1.2-2.noarch.rpm
   bootconf = 1.2-2
  =
   /usr/bin/python

  bootconf-gui-1.2-2.noarch.rpm
   bootconf-gui = 1.2-2
  =
   /bin/sh
   bootconf = 0:1.2-2
   pygtk2
   usermode

* %check is not present; not possible to test this automatically.  I installed 
   and ran it and it seemed to work OK.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* scriptlets are OK (update-desktop-database).
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
X desktop file not installed properly.

Ok, fixed up the desktop file:
http://www.courier-mta.org/bootconf/download/bootconf-1.3-1.src.rpm

I also swiped a suitable icon from gnome-desktop -- shouldn't be a problem.

I don't see any pressing reason to use %{dist} for now; at this time the
supported versions are fairly basic, and the GUI is not exotic enough to be
distro-sensitive. It'll probably work on releases going back several years.

Wow, for some reason I really thought I had commented on this.  Maybe I had
typed in a comment but lost it somehow.

In any case, I think you misunderstand the reasons we use the dist tag, but
that's up to you.

The desktop file looks fine to me.

APPROVED

Ping, Sam. Please raise fedora‑cvs flag (see below) to "?" when you will be ready.

Everybody waiting for you :)

New Package CVS Request
=======================
Package Name: bootconf
Short Description: GRUB configuration utility
Owners: mrsam
Branches: F-9
InitialCC: mrsam
Cvsextras Commits: yes

cvs done.

bootconf-1.3-1.fc9 has been submitted as an update for Fedora 9

bootconf-1.3-1.fc9 has been pushed to the Fedora 9 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update bootconf'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2008-5995

bootconf-1.3-1.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.