Bug 249949 - Review Request: beldi - Belug Linux Distribution Burner
Summary: Review Request: beldi - Belug Linux Distribution Burner
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Christoph Wickert
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-07-28 12:14 UTC by Robert Scheck
Modified: 2008-11-05 23:51 UTC (History)
4 users (show)

Fixed In Version: 0.9.16-3.fc8
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-07-15 12:17:14 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
cwickert: fedora-review+
a.badger: fedora-cvs+


Attachments (Terms of Use)
My spec for the previous release. Edit: The "Requires(pre)" is not necessary. (2.65 KB, text/plain)
2007-09-19 13:22 UTC, Christoph Wickert
no flags Details
Updated spec for 0.9.16 (2.35 KB, text/plain)
2008-07-03 18:37 UTC, Christoph Wickert
no flags Details

Description Robert Scheck 2007-07-28 12:14:04 UTC
Spec URL: http://labs.linuxnetz.de/bugzilla/beldi.spec
SRPM URL: http://labs.linuxnetz.de/bugzilla/beldi-0.9-1.src.rpm
Description: BeLDi, the Belug (Linux) Distribution Burner, is a program designed to burn distributions. BeLDi consist of a simple graphic user 
interface where the main screen shows the available distributions in a list. 
When the user selects one, a dialog will ask which version and for which 
architecture, the distribution has to be burned. Once the architecture and 
version are selected the burn procedure starts. You can watch the progress 
on the progression bar. All user operations can be completed with the mouse 
or a touchscreen. It is designed to require the less administration and 
knowledge as possible.

Comment 1 Robert Scheck 2007-07-28 12:30:41 UTC
I know the *.desktop file is missing, but I would like to get some support 
regarding this, as I never did such a thing before. Christoph, do you have the 
time and interest in reviewing and maybe maintaining or co-maintaining this 
package as mentioned at the LinuxTag?

Comment 2 Christoph Wickert 2007-07-28 14:04:49 UTC
Yes, I do. I'll look at this tomorow. Thanks for submitting this review.

Comment 3 Christoph Wickert 2007-07-31 18:26:22 UTC
The desktop file is no big deal, but what troubles me is that we need write
access to /var/lib/beldi [¹], so we have two possibilities:
1. make /var/lib/beldi writable for a special "beldi" group and require the user
to be added to this group
2. install beldi to /usr/sbin and call it through consolehelper for normal users

Robert, what do you prefer?

[¹] Have you tested what happens when everything is downloaded? Do we still need
special permissions for burning the CDs?

Comment 4 Robert Scheck 2007-07-31 21:21:09 UTC
Good question. I don't know what's better and I also don't know whether
we need special permissions for burning the CDs. Option 1 IMHO requires an 
upstream modification while second could be downstream only, right? Otherwise 
we would have to maintain a patch for beldi or am I wrong? Second has the 
issue, that userhelper involves root permissions for beldi which are maybe not 
required and which could be a security issue?! I'll talk with Andy about it.

Comment 5 Robert Scheck 2007-09-19 08:23:06 UTC
Christoph, Beldi changed and uses now a directory in $HOME unless configured 
somehow else. Can you please review the package again?

SRPM URL: http://labs.linuxnetz.de/bugzilla/beldi-0.9-2.src.rpm

Oh and burning CDs on Fedora 7 works without special permissions for me. Fedora 
Core 6 can't be supported by Beldi right now, because of qemu requirement. EOL 
of FC6 is reached in a few months as well.

Can we restart/go on with the review?

Comment 6 Christoph Wickert 2007-09-19 13:19:56 UTC
(In reply to comment #5)
> Christoph, Beldi changed and uses now a directory in $HOME unless configured 
> somehow else.

Huh? Beldi changed? But the source is still named beldi-0.9.tar.gz. It's a bad
habit to change something without increasing the version. This version is
completely different from the previous one , so it should at least be called 0.9.1.

In fact I liked beldi in /var/lib. If we use userhelper multiple users could
share a single beldi installation, now every user needs to download all the ISOs
again, which is more than 20 GB per user!

By default beldi now uses /tmp, which is a bad idea, because
- now beldi complains that it can't read files already in /tmp
- it bears the risk that beldi changes files it can read/write
- /tmp will be wiped out on reboot and people will have to start "beldi
--update" and all the downloads again! 

Even worse: One can't change the download location because of
"Configuration::Save can't open file /etc/beldi_settings.xml in write mode."
So we still need root privileges or beldi needs to store the configuration in
~/.beldi_settings.xml.

> Oh and burning CDs on Fedora 7 works without special permissions for me.

Burning did not work for me with the previous release. There were permission
issues for normal users and as root there were problems with wodim, it was
probing the wrong devices (/dev/hd* instead of /dev/sd*).

With the new version I still see a lot of warnings but at least burning works.

> Core 6 can't be supported by Beldi right now, because of qemu requirement.

I'm not sure if beldi should require qemu because the "test with qemu" feature
doesn't work for most distributions I've tested. Ether not enough RAM or no
harddisk, but all the installers fail. This is only useful for those
distributions that come with an installable livecd.

This version of beldi looks so 'unfinished' to me (look at the configuration
page) that I only would allow it to enter rawhide but none of the stable
releases. In fact I noticed a regression: The downloader no longer seems to
accept "file://" as mirror url (I used local copies of the ISOs for testing).

I'm attaching my spec, sorry I did not put it into this review before. This is
how I would have packaged the previous version. Please do a diff against yours
and take what you think is useful.

Comment 7 Christoph Wickert 2007-09-19 13:22:27 UTC
Created attachment 199471 [details]
My spec for the previous release.

Edit: The "Requires(pre)" is not necessary.

Note: This spec is out of date for the new version.

Comment 8 Robert Scheck 2007-09-19 13:36:12 UTC
Okay, I've to slap upstream. Changing source is worse, but this is an upstream 
issue, not downstream. But Beldi should take $HOME/beldi or something per 
default as only one user per machine normally really uses Beldi. Independent of 
this, Beldi can given another parameter to use a common location for the files 
it uses.

Removing the qemu requirement again is nothing big. Suid is what upstream would 
like to avoid. Upstream prefers either separate directories or one directory 
which is read-writable to all users. The last of these solutions is discouraged 
in Fedora as I got from #fedora-devel.

It was also talked with upstream, that a copy of the config file is used in 
$HOME and /etc is only a fallback/default...but looks like it wasn't done in 
that way now.

Well...the version number 0.9 should indicate some things regarding features 
and stability ;-) I'll now talk with upstream first.

Comment 9 Christoph Wickert 2007-09-19 14:11:50 UTC
(In reply to comment #8)
> Okay, I've to slap upstream. Changing source is worse, but this is an upstream 
> issue, not downstream.

Yes, it's an upstream issue, but it makes tracking changes for us (you as the
maintainer) harder. :(

> But Beldi should take $HOME/beldi or something per 
> default as only one user per machine normally really uses Beldi.

And how do you want achieve this if not with userhelper? 

> Independent of 
> this, Beldi can given another parameter to use a common location for the files 
> it uses.

$HOME/beldi is a bad idea I think, because it can interfere with
system-config-users (in the unlikely case someone creates a user called "beldi")
/var/lib/beldi is better and follows the FHS.

If we use $HOME/beldi we _need_ to create the beldi user during %post.

> Removing the qemu requirement again is nothing big.

Removing qemu from Requires: is no big deal but it leaves us with the "Test with
qemu" button.

> Suid is what upstream would 
> like to avoid. Upstream prefers either separate directories or one directory 
> which is read-writable to all users. The last of these solutions is discouraged 
> in Fedora as I got from #fedora-devel.

I wasn't suggesting to make beldi suid or creating a world writable directory
but about using userhelper like we do for revisor and other tools. To me this is
the best solution, the "fedora way": It guarantees that beldi always is executed
as a certain user, so that all users can share a single beldi installation.

Comment 10 Christoph Wickert 2007-09-20 18:18:26 UTC
(In reply to comment #9)

> $HOME/beldi is a bad idea I think, because it can interfere with
> system-config-users (in the unlikely case someone creates a user called "beldi")

Sorry, this was nonsense. $HOME/beldi is ok, I accidentally misread /home/beldi.

Comment 11 Robert Scheck 2007-12-16 16:39:15 UTC
Christoph, can you please review the package again? Andreas did several changes 
and below is an updated SRPM:

SRPM URL: http://labs.linuxnetz.de/bugzilla/beldi-0.9.12-1.src.rpm

Comment 12 Robert Scheck 2008-01-21 22:38:09 UTC
Christoph, ping?

Comment 13 Christoph Wickert 2008-01-27 19:07:56 UTC
I don't like the new behavior:

When you start for the first time it starts in fullscreen. It creates
~/beldi_settings.xml and downloads new distro config files to ~.

When you start beldi for the second time it's no longer in full screen and there
are no distros available. :( The newly created beldi_settings.xml defines
/home/beldi/iso as target, but you don't create a beldi usr, so (most likely)
there is no /home/beldi.

Suggestions: 
beldi_settings.xml should be hidden
target should be ~/.beldi/iso" by default
distro xml files should be moved to ~/.beldi (I don't like a bunch of xml files
scattered through my $HOME.

With these changes we could at least get beldi into Fedora quickly. We don't
have no multi user option, but IMO we could enable this later when beldi is
really ready for it. Or we could just add the necessary files for using
consolehelper/pam and a README-multiuser to $doc so people who know what they
are doing could enable this themselves.

The current behavior is very confusing for users. What do you think?

Comment 14 Christoph Wickert 2008-07-03 18:37:58 UTC
Created attachment 310949 [details]
Updated spec for 0.9.16

Take what you like from it and then let's finish this review.

Comment 15 Robert Scheck 2008-07-13 17:57:23 UTC
Christoph, can you please review the package again? I think, I merged all of 
the changes and suggestions into one updated SRPM:

SRPM URL: http://labs.linuxnetz.de/bugzilla/beldi-0.9.16-2.src.rpm

Comment 16 Christoph Wickert 2008-07-13 20:42:46 UTC
Review for 145e2eb18b87a2dc7a12ce237c9c75c1  beldi-0.9.16-2.src.rpm:

OK - MUST: rpmlint /var/lib/mock/fedora-rawhide-i386/result/beldi-*
3 packages and 0 specfiles checked; 0 errors, 0 warnings.
OK - MUST: The package is named according to the  Package Naming Guidelines
Ok - MUST: The spec file name matches the base package %{name}, in the format
%{name}.spec
OK - MUST: The package meets the  Packaging Guidelines
OK - MUST: The package is licensed with a Fedora approved license and meets the
 Licensing Guidelines

FAIL - MUST: The License field in the package spec file does not match the
actual license: Code is GPLv3+, but License tag is GPLv2+
FAIL - MUST: License text from source is included in %doc, but the License is
out of date (GPLv2)

OK - MUST: The spec file is written in American English
OK - MUST: The spec file for the package is legible
OK - MUST: The sources used to build the package match the upstream source by
md5 420555ec522884dcb771c98c0960a1f5
OK - MUST: The package successfully compiles and builds into binary rpms on i386

FAIL - MUST: All build dependencies are listed in BuildRequires, but
gtkglextmm-devel is only needed when building with --enable-opengl. The OpenGL
interface looks really cool and works here and so I suggest to include it. What
do you think?
Pigment support (requires pigment-devel >= 0.3 and gstreamer-plugins-base-devel)
is still experimental and does not build here, so I suggest not to enable it.

OK - MUST: The package owns all directories that it creates
OK - MUST: The package does not contain any duplicate files in the %files listing
OK - MUST: Permissions on files are set properly
OK - MUST: The package has a %clean section, which contains rm -rf %{buildroot}
OK - MUST: The package consistently uses macros, as described in the macros
section of Packaging Guidelines
OK - MUST: The package contains code, or permissible content
OK - MUST: Files included in %doc do not affect the runtime of the application
OK - MUST: The package does not contain any .la libtool archives
OK - MUST: The package contains a GUI application and includes a %{name}.desktop
file that file is properly installed with desktop-file-install
OK - MUST: The packages does not own files or directories already owned by other
packages
OK - MUST: The package runs rm -rf %{buildroot} at the beginning of %install
OK - MUST: All filenames in the package are valid UTF-8

FIX - SHOULD: Please bug upstream to include an updated copy of the license text.
FIX? - SHOULD: Could you include a German translation of description and summary?
FIX - SHOULD: Typo in description: less -> least, consist -> consists.
IMHO the description could be simplified a little:
----->
BeLDi, the Belug (Linux) Distribution Burner, is a program designed to burn
distributions. It is designed to require the least administration and knowledge
as possible.

BeLDi has a intuitive graphic user interface where the main screen shows the
available distributions in a list. If the user selects one, he will be asked
which version and architecture he wants to burn. Once the burn procedure starts
a bar shows its progress. All user operations can be completed with the mouse or
a touchscreen.
<-----

OK - SHOULD: The package builds in mock
OK - SHOULD: The package functions as described
OK - SHOULD: Latest version of the application


So the only blocker is OpenGL. The license text is no real issue for me as long
as you fix the license tag in the spec.

NEEDSWORK

Comment 17 Robert Scheck 2008-07-13 21:17:04 UTC
(In reply to comment #16)
> FAIL - MUST: The License field in the package spec file does not match the
> actual license: Code is GPLv3+, but License tag is GPLv2+
> FAIL - MUST: License text from source is included in %doc, but the License is
> out of date (GPLv2)

Changed to GPLv3+ in the spec file, sent e-mail to upstream to correct that for 
the next upstream release.

> FAIL - MUST: All build dependencies are listed in BuildRequires, but
> gtkglextmm-devel is only needed when building with --enable-opengl. The OpenGL
> interface looks really cool and works here and so I suggest to include it. 
> What do you think?

I agree with you, enabled now. Thought, that would be catched up automagically
once the dependency is satified, but it looks not as it would be the case.

> FIX - SHOULD: Please bug upstream to include an updated copy of the license 
> text.

Done, see above and within your mailbox.

> FIX? - SHOULD: Could you include a German translation of description and
> summary?

No not really, this is something for specspo package.

> FIX - SHOULD: Typo in description: less -> least, consist -> consists.
> IMHO the description could be simplified a little:

I've taken your rewrite now, sounds better. Former description was taken from
the Beldi website.

> NEEDSWORK

Done, SRPM: http://labs.linuxnetz.de/bugzilla/beldi-0.9.16-3.src.rpm

Comment 18 Christoph Wickert 2008-07-13 22:26:04 UTC
(In reply to comment #17)
> (In reply to comment #16)
> > The OpenGL
> > interface looks really cool and works here and so I suggest to include it. 
> > What do you think?
> 
> I agree with you, enabled now. Thought, that would be catched up automagically
> once the dependency is satified, but it looks not as it would be the case.

It is still experimental and has some bugs, nevertheless I think we should
include it because it's not default and can do no harm. I leave the decision up
to you.

> Done, SRPM: http://labs.linuxnetz.de/bugzilla/beldi-0.9.16-3.src.rpm

OK - MUST: The License field in the package spec file matches the actual
license: GPLv3+
OK - MUST: BuildRequires sane
OK - SHOULD: Description updated

This program definitely has some serious bugs, but we can work on them with
upstream once the package is in Fedora. From a reviewers point of view
everything is fine, so this is 

APPROVED

Comment 19 Mamoru TASAKA 2008-07-14 06:51:09 UTC
One note:
Please remove unneeded autotool related BuildRequires (autoconf, automake).
Also, BuildRequires: cairo-devel, gtk2-devel are somewhat redundant (always
required by gtkmm24-devel)

Comment 20 Robert Scheck 2008-07-14 10:41:38 UTC
Thanks Mamoru for pointing that out. Should be minor and I'll fix this before 
the initial import. Autotool is overleft from times where upstream had broken
files there.


New Package CVS Request
=======================
Package Name: beldi
Short Description: Belug Linux Distribution Burner
Owners: robert, cwickert
Branches: F-8 F-9
InitialCC: 
Cvsextras Commits: yes

Comment 21 Kevin Fenzi 2008-07-14 15:57:05 UTC
cvs done.

Comment 22 Robert Scheck 2008-07-14 18:54:00 UTC
Package: beldi-0.9.16-3.fc8 Tag: dist-f8-updates-candidate Status: complete
Package: beldi-0.9.16-3.fc9 Tag: dist-f9-updates-candidate Status: complete
Package: beldi-0.9.16-3.fc10 Tag: dist-f10 Status: complete

Comment 23 Fedora Update System 2008-07-14 18:55:39 UTC
beldi-0.9.16-3.fc8 has been submitted as an update for Fedora 8

Comment 24 Fedora Update System 2008-07-14 19:21:47 UTC
beldi-0.9.16-3.fc9 has been submitted as an update for Fedora 9

Comment 25 Fedora Update System 2008-07-15 12:17:12 UTC
beldi-0.9.16-3.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 26 Fedora Update System 2008-07-15 12:20:15 UTC
beldi-0.9.16-3.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 27 Fedora Update System 2008-07-26 06:05:17 UTC
beldi-0.9.16-3.fc9 has been pushed to the Fedora 9 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 28 Christoph Wickert 2008-11-05 21:12:00 UTC
Robert and I agreed to switch primary ownership.

Package Change Request
======================
Package Name: beldi
Owners: cwickert, robert

Comment 29 Jason Tibbitts 2008-11-05 21:46:16 UTC
Is there some reason you can't make this change in the packagedb yourselves?  There's no CVS admin action involved here.

Comment 30 Christoph Wickert 2008-11-05 22:28:11 UTC
How can I do this in packagedb? I have been an owner right from the beginning and have all privileges, still packagedb lists Robert as owner, see
https://admin.fedoraproject.org/pkgdb/packages/name/beldi

Comment 31 Robert Scheck 2008-11-05 23:00:44 UTC
Theoretically packagedb works, in fact when I'm using it, only the flavor broken is available. Anyway solved with help of abadger1999 on #fedora-admin:

[23:35:05] < rsc> who broke packagedb?
[23:35:11] < abadger1999> rsc: I did.
[23:35:12] < rsc> I'm not able to release an ownership.
[23:35:23] < abadger1999> rsc: Which page?
[23:35:26] < rsc> abadger1999: I knew that ;)
[23:35:31] < abadger1999> heh
[23:35:32] < rsc> abadger1999: https://admin.fedoraproject.org/pkgdb/packages/name/beldi
[23:35:47] < rsc> cwickert shall get owner and I as co-maintainer (so just vice versa change)
[23:36:35] < abadger1999> rsc: Do you get an error message?
[23:36:59] < rsc> abadger1999: that would made me lucky. Just nothing happens. Even no ajax animation.
[23:38:24] < abadger1999> rsc: can you try refreshing the page for me and hitting it again.
[23:38:31] < rsc> of course. Hang on.
[23:38:44] < rsc> Refreshed, will click now.
[23:38:57] < abadger1999> If I'm lucky, you hit the page while I was restarting the servers and they lost the session information.
[23:39:00] < rsc> Clicked to all three butons.
[23:39:09] < rsc> *buttons
[23:40:22] < rsc> abadger1999: anything nice found?
[23:41:20] < abadger1999> rsc: Nope.  I see the request that you put in before but not the one you're putting in now.
[23:41:45] < abadger1999> Ah hah
[23:41:46] < rsc> maybe the buttons itself are broken?
[23:41:47] < abadger1999> There it is
[23:42:13] < abadger1999> Oh wait... That's the login URL
[23:42:24] < abadger1999> 1 minute ago
[23:42:43] < rsc> hmpf.
[23:42:57] < rsc> re-login now.
[23:43:13] < rsc> clicked "released ownership" again, nothing happend
[23:43:53] < abadger1999> Let me try... I'm in cvsadmin which sometimes means I can't reproduce the error but worth a shot.
[23:44:47] < abadger1999> Yeah. Something's broken.
[23:45:09] < abadger1999> Ah... I updated some of the javascript... since that's static it gets cached.
[23:45:17] < abadger1999> Let me clear mod_cache on the proxies
[23:50:34] < abadger1999> rsc: Okay, try again
[23:51:26] < rsc> abadger1999: works.
[23:51:28] < rsc> cwickert: take it.
[23:51:39] < cwickert> rsc: mom...
[23:51:41] < rsc> abadger1999: thank you. So you really broke it? ;)
[23:52:36] < cwickert> abadger1999: I can't take the package from rsc, the button does nothing for me ether
[23:52:52] < abadger1999> rsc: heh :-)  Somewhat
[23:52:52] < rsc> cwickert: haha!
[23:53:13] < rsc> abadger1999: okay, now you've to switch back, that he can take it. Looks like this is maybe the old JavaScript? ;)
[23:53:19] < abadger1999> cwickert: Refresh the page and try again.  I needed to flush the cache on the web servers when I upgraded.
[23:54:02] < abadger1999> rsc: yeah.  The old javascript was in the cache.  But it referenced things that are no longer in the new server.  So things broke.
[23:54:12] < cwickert> abadger1999: I did reload the page...
[23:54:12] < abadger1999> after flushing the cache, the new javascript should be being saved.
[23:54:23] < cwickert> let me restart my browser
[23:54:53] < abadger1999> Hmmm...  yeah if it's still broken after that, there's something fishy going on... it's being cached somewhere.
[23:55:41] < cwickert> abadger1999: works now
[23:55:54] < cwickert> rsc: ok, now re-add the permissions you need
[23:56:05] < abadger1999> cwickert: Cool.
[23:56:36] < abadger1999> rsc, cwickert: Thanks for letting me know about that.  I need to add flush the cache to the TurboGears SOP.
[23:57:03] < rsc> cwickert: done, you've to approve the spam flooding now.


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