Bug 217197 - (MyBashBurn) Review Request: MyBashBurn - Burn data and create songs with interactive dialogs.
Review Request: MyBashBurn - Burn data and create songs with interactive dial...
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Package Reviews List
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2006-11-24 20:24 EST by Wilmer Jaramillo M.
Modified: 2008-02-25 19:23 EST (History)
3 users (show)

See Also:
Fixed In Version: 1.0.2-3.fc8
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-02-25 19:23:09 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)
rpmlint error free SPEC file (4.31 KB, application/octet-stream)
2006-12-02 09:12 EST, Parag AN(पराग)
no flags Details

  None (edit)
Description Wilmer Jaramillo M. 2006-11-24 20:24:53 EST
Spec URL:
http://www.fedora-ve.org/mybashburn/downloads/mybashburn.spec

SRPM URL: http://www.fedora-ve.org/mybashburn/downloads/mybashburn-0.1-0.src.rpm

Project URL: 
http://www.fedora-ve.org/mybashburn/

Description: 
MyBashBurn is a applications of use in console, this is a version/fork owned for the cd burning shell script called BashBurn(see http://bashburn.sf.net where also i'm developer) for Linux, this originally not have the best eye-candy CD-burning, neverthless, MyBashBurn use dialog box function which draws (using ncurses) windows onto the screen. MyBashBurn dialog box offer best funcionability, has very good capabilities of automatically finding dependencies and autodetect devices CD/DVD RW. 

MyBashBurn can burn data cds, music cds, multisession cds. It can burn and create ISO files. It can burn bin/cue files, create mp3s, oggs and flac files. Supports burning DVD-images and data DVDs, and others funny options. MyBashBurn depends on cdrecord and others backend aplications, so basically if your writing device works with it, MyBashBurn will work flawlessly.
Comment 1 Wilmer Jaramillo M. 2006-11-24 20:29:48 EST
the support of mp3 depends of the backends applications in fedora core this are
disable, like also the support for each one of the others formats.
Comment 2 Parag AN(पराग) 2006-11-24 23:55:02 EST
Is this your first package to Fedora Extras? If yes then you need to add
FE-NEEDSPONSOR in textbox given below next to Bug 217197 blocks. Just had a look
at you SPEC and found they are not following
http://fedoraproject.org/wiki/Packaging/Guidelines
like macros,scriptlet rules then %files section included files twice
Just look at those guidelines and resubmit updated package
Comment 3 Wilmer Jaramillo M. 2006-11-26 01:41:26 EST
okay, it's good to hear from you, just now added myself on FE-NEEDSPONSOR.

I'm uploading the new SRPM and SPEC files, note that i decided change this
initial release to 1.0-1 for compatibility with the PackagingGuidelines and
NamingGuidelines.

In the spec file the line install -c -m666 this is needed to that every user can
to do read-write operations, in next realease i think to split it into of
program in some as: ~/.bashburnrc

CREDITS and HOWTO are duplicate as on %{doc} as %{buildroot}%{_datadir}/%{name}
In %{doc} for compatibility with the guidelines
In %{buildroot}%{_datadir}/%{name} why the program need read it there.
Comment 5 Parag AN(पराग) 2006-12-02 09:01:05 EST
Still packaging needs to be improved.
Comment 6 Parag AN(पराग) 2006-12-02 09:08:33 EST
Got build error
install: cannot stat `etc/mybashburnrc.fedora': No such file or directory
Comment 7 Parag AN(पराग) 2006-12-02 09:12:33 EST
Created attachment 142661 [details]
rpmlint error free SPEC file

Use this script bump release adding changelog as fixed SPEC file Version,added
build section and removed dot from summary.
Comment 8 Wilmer Jaramillo M. 2006-12-04 18:06:48 EST
Thx again, also i fix the location of man file in the SPEC file, added
/usr/lib/debug for build section and you was include in the global changelog file.

Please, check it out new updates:

Spec URL:
http://www.fedora-ve.org/mybashburn/downloads/mybashburn.spec

SRPM URL:
http://www.fedora-ve.org/mybashburn/downloads/mybashburn-1.0-1.src.rpm
Comment 9 Parag AN(पराग) 2006-12-04 23:24:07 EST
Kindly remove my name from changelog its at all not necessary. If you got from
somebody modified SPEC then you write your own name in Chnagelog.
Comment 10 Wilmer Jaramillo M. 2006-12-05 14:03:03 EST
(In reply to comment #9)
> Kindly remove my name from changelog its at all not necessary. If you got from
> somebody modified SPEC then you write your own name in Chnagelog.

How you want, now, there are some more for check on review? 
when should the package be approved?
Greetings.
Comment 11 Michael Schwendt 2006-12-05 17:51:49 EST
> %post
> ln -s %{_datadir}/%{name}/MyBashBurn.sh %{_bindir}/mybashburn > /dev/null 2>&1
> ln -s %{_datadir}/%{name}/MyBashBurn.sh %{_bindir}/bashburn > /dev/null 2>&1
> 
> %postun
> rm -f %{_bindir}/bashburn
> rm -f %{_bindir}/mybashburn

Why? This is extremely ugly. The files are not even %ghosted. They
are not tracked in the RPM database. Why not simply create these links
at build-time and include them in the package? That is the way it ought
to be.


> %files
> %defattr(-,root,root,0755)
> %{_datadir}/%{name}/*

Directory %{_datadir}/%{name} is not included. Correct would be:

%{_datadir}/%{name}/

Find one brief explanation in bug 165616 comment 8.


> %doc COPYING CREDITS ChangeLog FAQ FILES HOWTO INSTALL README TODO

Is the "INSTALL" file the typical NLS documentation which is irrelevant
to users of your RPM package? If so, don't include it. It is confusing.


> install -d %{buildroot}/usr/lib/debug

What's this?


Further, prefer "install" with option -p  (or "cp" with option -p)
to preserve timestamps wherever it is possible.
Comment 12 Wilmer Jaramillo M. 2006-12-06 01:42:56 EST
(In reply to comment #11)
> > %post
> > ln -s %{_datadir}/%{name}/MyBashBurn.sh %{_bindir}/mybashburn > /dev/null 2>&1
> > ln -s %{_datadir}/%{name}/MyBashBurn.sh %{_bindir}/bashburn > /dev/null 2>&1
> > 
> > %postun
> > rm -f %{_bindir}/bashburn
> > rm -f %{_bindir}/mybashburn
> 
> Why? This is extremely ugly. The files are not even %ghosted. They
> are not tracked in the RPM database. Why not simply create these links
> at build-time and include them in the package? That is the way it ought
> to be.

yeah, It sound kickass, better and more appropriate, I've added it in build-time.
 
> > %files
> > %defattr(-,root,root,0755)
> > %{_datadir}/%{name}/*
> 
> Directory %{_datadir}/%{name} is not included. Correct would be:
> 
> %{_datadir}/%{name}/
> 

Unknown that the contents would be recursive, now the directory trailing use is
fixed.
 
> > %doc COPYING CREDITS ChangeLog FAQ FILES HOWTO INSTALL README TODO
> 
> Is the "INSTALL" file the typical NLS documentation which is irrelevant
> to users of your RPM package? If so, don't include it. It is confusing.

Exactly. INSTALL file not is appropiate for users except for possible developers
of MyBashBurn only, it content "troubleshooting" section and installation's
instruction for tar file, neverthless, just i exclude the INSTALL file. Fixed.

> > install -d %{buildroot}/usr/lib/debug
> 
> What's this?

Really i don't know, i think that may be strictly needed for %build section, if
ommited it to get the next error:
find: /var/tmp/mybashburn-1.0-1-root-k0k/usr/lib/debug: No such file or directory

If create the directory %{buildroot}/usr/lib/debug issue fixed.
Please, remember that this package is a couple of ShellScripts written to make
cd burning in the console easier, just not is a program written in C/C++ or some
similar, for this i can't see why must be include %build section for this package?.
 
> Further, prefer "install" with option -p  (or "cp" with option -p)
> to preserve timestamps wherever it is possible.

Okay. I should adjusted the install -p for which components: files or
directories or both?

Please let me know what more i need to do, Thanks in advance for your response.
Comment 13 Wilmer Jaramillo M. 2006-12-06 02:51:23 EST
New package ready, the same URL, please check this time and report any problems.
Comment 14 Wilmer Jaramillo M. 2006-12-06 03:05:27 EST
I believe that all items was fixed.
Comment 15 Parag AN(पराग) 2006-12-06 04:33:32 EST
Wilmer,
 Remember CC list is not a list that the person in CC list is surely going to
REVIEW your package. Also, there can be as many as persons be sited in CC list.
When your package is under review your package status will be changes accordingly.
Also, Being a newbie you still have to go a lot for improving a packaging.
  So i will wonder if someone will take this under review immediately. You need
to start reviewing other packages waiting for review.
Comment 16 Wilmer Jaramillo M. 2006-12-08 18:31:55 EST
Parag okeys.
Comment 17 Wilmer Jaramillo M. 2006-12-09 18:50:56 EST
(In reply to comment #15)
Sorry, I just disabled mail delivery CC for you by a time for know how the
bugzilla system worked, also i'm learning fine with others review process and
i'm not worried aobut the approved thing, i'm just learning to get it how to
work, any recommend is welcome. Sorry for my english again.
Comment 18 Wilmer Jaramillo M. 2006-12-09 19:40:03 EST
(In reply to comment #15)
Sorry, I just disabled mail delivery CC for you by a time for know how the
bugzilla system worked, also i'm learning fine with others review process and
i'm not worried aobut the approved thing, i'm just learning to get it how to
work, any recommend is welcome. Sorry for my english again.
Comment 19 Parag AN(पराग) 2006-12-10 02:47:20 EST
(In reply to comment #18)
> (In reply to comment #15)
> Sorry, I just disabled mail delivery CC for you by a time for know how the
> bugzilla system worked,
aah OK.
> also i'm learning fine with others review process and
  Nice to hear from you. 
> i'm not worried aobut the approved thing, i'm just learning to get it how to
> work, any recommend is welcome. Sorry for my english again.
OK. So start posting reviews on others packages. See also some progressed BUGS
with FE-REVIEW status for how review progresses and how to post them.


Comment 20 Parag AN(पराग) 2006-12-10 02:51:58 EST
You must increase release tag number each time you modifies SPEC file. I saw you
said you did some changes to SPEC in comment #12 but you kept package name as
mybashburn-1.0-1.src.rpm which SHOULD be mybashburn-1.0-2.src.rpm
Comment 21 Wilmer Jaramillo M. 2006-12-10 03:07:21 EST
if I understand it, that is something that Jorge(jtorres) me said it out, during
today Sunday i increase tag number release. thx
Comment 22 Wilmer Jaramillo M. 2006-12-11 22:09:00 EST
Now, I'm a little confused with presence  of %build section, without this
rpmlint is silent, with it is necessary to make %{buildroot}/usr/lib/debug
directory.

Also remember that i need a sponsor. ;)

All other suggestions have been implemented:
Updated to 1.0-2 -- package at:
http://www.fedora-ve.org/mybashburn/downloads/mybashburn.spec
SRPM URL:
http://www.fedora-ve.org/mybashburn/downloads/mybashburn-1.0-2.src.rpm
Comment 23 Wilmer Jaramillo M. 2006-12-11 22:37:16 EST
Others considerations:
I run rpmlint command and the output:

$ rpmlint mybashburn-1.0-2.noarch.rpm 
W: mybashburn conffile-without-noreplace-flag /etc/mybashburnrc
E: mybashburn script-without-shebang
/usr/share/mybashburn/lang/Swedish/iso_menu.lang
E: mybashburn script-without-shebang
/usr/share/mybashburn/lang/Czech/convert_mp3s.lang
[...]

--> I can't understand how fix it.

E: mybashburn world-writable /etc/mybashburnrc 0666
---> Note that this is good and is necessary for the package,  see comment #3

E: mybashburn standard-dir-owned-by-package /usr/bin
E: mybashburn wrong-script-end-of-line-encoding
/usr/share/mybashburn/lang/Polish/multi.lang

--> WTF?.

That's a new one to me. These are apparently false positives?, Thanks for
spending time on this!
Comment 24 Michael Schwendt 2006-12-12 04:43:57 EST
> E: mybashburn world-writable /etc/mybashburnrc 0666
> ---> Note that this is good and is necessary for the package,
>  see comment #3

Are you kidding? Files in /etc must not be writable by ordinary
users. The software is flawed, if there is no implementation of
user-local configuration files in $HOME.

> %{buildroot}/usr/lib/debug

Then your RPM config or installation is broken. You don't need to
create that directory.

> mybashburn conffile-without-noreplace-flag /etc/mybashburnrc

This refers to

  %config(noreplace) /etc/mybashburnrc

which influences RPM's strategy for *.rpmsave/*.rpmnew config files.

> E: mybashburn script-without-shebang

This is a hint about all the executable files which should not be
executable. Take a close look at "rpm --query --list mybashburn" and
notice the questionable file permissions.

The manual page is executable, too. And so are many other files which
need not be executable.

> E: mybashburn standard-dir-owned-by-package /usr/bin

List the files included in your package:
rpm --query --list --verbose mybashburn

You include the directory /usr/bin which is wrong, since it belongs into
the core "filesystem" package already.

> E: mybashburn wrong-script-end-of-line-encoding
> /usr/share/mybashburn/lang/Polish/multi.lang
> 
> --> WTF?.

There are DOS/Windows-style 0x0d 0x0a (carriage return, linefeed) line
delimiters used in that file instead of just 0x0a (linefeed). Should be
fixed upstream. Can be fixed with dos2unix or sed.
Comment 25 Wilmer Jaramillo M. 2006-12-12 23:48:36 EST
(In reply to comment #24)
> > E: mybashburn world-writable /etc/mybashburnrc 0666
> > ---> Note that this is good and is necessary for the package,
> >  see comment #3
> 
> Are you kidding? Files in /etc must not be writable by ordinary
> users. The software is flawed, if there is no implementation of
> user-local configuration files in $HOME.
> 

Yes, i'm sure, this should be ignored.

> > %{buildroot}/usr/lib/debug
> Then your RPM config or installation is broken. You don't need to
> create that directory.

Yes, fixed.

> You include the directory /usr/bin which is wrong, since it belongs into
> the core "filesystem" package already.

Fixed.

All other rpmlint E: have been solved:
1) Fixed DOS/Windows-like (CRLF) end-of-line encoding with %%{__sed} tag.
2) Replaced %%{_bindir}/* tag of %%files section by %%{_bindir}/files.
3) Cleanup in %%install section.
4) Replaced %%config(noreplace) instead %%config.

Updated to 1.0-2 -- package at:
http://www.fedora-ve.org/mybashburn/downloads/mybashburn.spec
SRPM URL:
http://www.fedora-ve.org/mybashburn/downloads/mybashburn-1.0-3.src.rpm
Comment 26 Wilmer Jaramillo M. 2006-12-12 23:53:12 EST
(In reply to comment #25)
> Updated to 1.0-2 -- package at:

i meant 1.0-3.
Comment 27 Ralf Corsepius 2006-12-13 00:17:55 EST
E: mybashburn world-writable /etc/mybashburnrc 0666
(In reply to comment #25)
> (In reply to comment #24)
> > > E: mybashburn world-writable /etc/mybashburnrc 0666
> > > ---> Note that this is good and is necessary for the package,
> > >  see comment #3
> > 
> > Are you kidding? Files in /etc must not be writable by ordinary
> > users. The software is flawed, if there is no implementation of
> > user-local configuration files in $HOME.
> > 
> 
> Yes, i'm sure, this should be ignored.

Bummer, I guess, I can't avoid some _very clear_ words:
* A world-wide writeable file is NOT acceptable.
* A package relying on such a "feature" is maldesigned.

=> Your only choices are 
* Fix this defect/maldesign of the package
* Withdraw this review request.

Unless this has been fixed, there is NO WAY for this package to make into Fedora.
Comment 28 Wilmer Jaramillo M. 2006-12-13 02:47:48 EST
Really thanks for this very clear words, now with the amount of problems arisen
with this revision i lost of see the development of the project, consequently, I
maintain the request of this revision and I concentrate in the development and
adjustment of defect/maldesign of the package.

Thanks.
Comment 29 Wilmer Jaramillo M. 2006-12-13 04:10:09 EST
Ready, removed /etc/mybashburnrc, now it's configure in run time to
~/.mybashburnrc, Builds OK, rpmlint is silent, permissions fine.

New files updated:
http://www.fedora-ve.org/mybashburn/downloads/mybashburn.spec
SRPM URL:
http://www.fedora-ve.org/mybashburn/downloads/mybashburn-1.0-3.src.rpm
Comment 30 Wilmer Jaramillo M. 2007-01-07 18:17:34 EST
greetings and happy new year,

I want announce a new release of MyBashBurn 1.0.1 with major feature and
enhancements, with some security bugfix important for this review, as a function
against race conditions on temp files, now MyBashBurn look for root/user config
in /etc/mybashburnrc, ~/.mybashburnrc in that order, fix permissions, updated
man file and the option of change the 'root directory' (/usr/share/mybashburn/)
have been deprecated for some time and will be removed.

New files location:

SPEC: http://mybashburn.sourceforge.net/mybashburn.spec
SRPMS: http://mybashburn.sourceforge.net/mybashburn-1.0.1-0.src.rpm
HOMEPAGE: http://mybashburn.sourceforge.net/
Comment 31 Wilmer Jaramillo M. 2007-01-16 23:17:33 EST
What's the status on this review?
Comment 32 Parag AN(पराग) 2007-01-16 23:59:11 EST
Looks like you need to correct Changelog entry
rpmlint on rpm gave me
W: mybashburn incoherent-version-in-changelog 1.0.1 1.0.1-0
The last entry in %changelog contains a version identifier that is not
coherent with the epoch:version-release tuple of the package.

Comment 33 Wilmer Jaramillo M. 2007-01-29 18:48:41 EST
After of a short vacations here are some corrections:

SPEC: http://mybashburn.sourceforge.net/mybashburn.spec
SRPMS: http://mybashburn.sourceforge.net/mybashburn-1.0.1-2.src.rpm

Everybody remember, i need a sponsor, Thanks.
Comment 34 Wilmer Jaramillo M. 2007-04-11 20:58:21 EDT
^sponser^sponsor :D
Comment 35 Wilmer Jaramillo M. 2007-06-01 22:09:21 EDT
New Release of MyBashBurn for more review:

SPEC: http://mybashburn.sourceforge.net/mybashburn.spec
SRPM: http://mybashburn.sourceforge.net/mybashburn-1.0.2-1.src.rpm

- More Clean up code.
- Fixed bug of compatibility for RHEL and CentOS Enterprise Linux.
- Fixed bug on define burn data menu when navigate names with blank spaces.
- Updated Makefile.
- Updated man and INSTALL files.
Comment 36 Jason Tibbitts 2007-07-09 13:19:44 EDT
This has been sitting around for far too long, and it looks as if the major
issues detailed above have been fixed.  I'll review this and emesene together
and, if those reviews go well, I'll sponsor you.  A warning, though, emesene
might need some work.
Comment 37 Jason Tibbitts 2007-07-10 20:30:29 EDT
OK, this builds and installs fine in mock on x86_64 running rawhide.  rpmlint says:
  W: mybashburn spurious-executable-perm /usr/share/man/man1/mybashburn.1.gz
which is easily fixed by setting the permissions to 644 when you install it.

Unfortunately when I attempted to test this, all I get is a message flashingg by
telling me "Terminal size must be fix, the size now is 80x24 Fai MyBashBurn
1.0.2" and then the program exits.  This is on an i386 machine running rawhide.

You need to be consistent about your macro usage.  If you want to use the
macroized forms like %{__sed} then you need to use %{__rm}, %{__install},
%{__cp} and %{__ln}.

Don't start the summary with the name of the package.

You install CREDITS and HOWTO twice.  If they're required as internal help
files, they shouldn't also be installed as %doc.

Review:
* source files match upstream:
   058395728c295988c3d633f5a2a25224f25babea6117d4b35c26c7b7b93e0d6a  
   mybashburn-1.0.2.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named
X specfile does not use macros consistently.
X summary should not start with the name of the package.
* description is OK.
* dist tag is present.
* 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 (development, x86_64).
* package installs properly
X rpmlint has a valid complaint.
* final provides and requires are sane:
   mybashburn = 1.0.2-1.fc8
  =
   /bin/sh
   /usr/bin/env
   cdda2wav
   cdrdao
   cdrecord
   coreutils
   dialog >= 1.0
   dvd+rw-tools
   eject
   flac
   mkisofs
   vorbis-tools
* %check is not present; no test suite upstream.
X Manual testing was not at all successful.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
X CREDITS and and HOWTO are duplicated.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
Comment 38 Wilmer Jaramillo M. 2007-07-11 01:10:33 EDT
Hi Jason, Thanks for testing it.

(In reply to comment #37)
> OK, this builds and installs fine in mock on x86_64 running rawhide.  rpmlint
says:
>   W: mybashburn spurious-executable-perm /usr/share/man/man1/mybashburn.1.gz
> which is easily fixed by setting the permissions to 644 when you install it.

Permissions now are correct, Fixed with: install -p m644 mybashburn.1.gz
 
> Unfortunately when I attempted to test this, all I get is a message flashingg by
> telling me "Terminal size must be fix, the size now is 80x24 Fai MyBashBurn
> 1.0.2" and then the program exits.  This is on an i386 machine running rawhide.

This is oks, for that MyBashBurn working correctly when is running in any X term
is necessary change the window "default size" manually, it not is a bug, does it
draw the windows based on internal dialog program, also attempt that the user
obtains best experience using the program, tries yourself for example to execute
the program in a size of terminal major to 80x20, later resize terminal minor to
80x20 and you know of that I speak to you, whatever with or without resize it
simply function right although it is not seen well. :D
Finnally the user only must resize/extend a little the terminal and execute
mybashburn, made simply, nevertheless when MyBashBurn is running in any not X
terminal it working good.

> You need to be consistent about your macro usage.  If you want to use the
> macroized forms like %{__sed} then you need to use %{__rm}, %{__install},
> %{__cp} and %{__ln}.

I do not want to use that command macroized forms, s/%{_sed}/sed/
Fixed.

> Don't start the summary with the name of the package.

Fixed. 

> You install CREDITS and HOWTO twice.  If they're required as internal help
> files, they shouldn't also be installed as %doc.

oks, you are right, removed CREDITS and HOWTO of %doc.

Finnally thanks for your help on getting this worked out Jason. I've posted an
updated mybashburn spec/SRPM on its review request, update files are on
web project, please refresh:

Spec: http://mybashburn.sourceforge.net/mybashburn.spec
SRPM: http://mybashburn.sourceforge.net/mybashburn-1.0.2-2.src.rpm

(In reply to comment #36)
> I'll review this and emesene together
> and, if those reviews go well, I'll sponsor you.  A warning, though, emesene
> might need some work.

I'll working on a new emesene based on stable SVN version.
Comment 39 Jason Tibbitts 2007-07-27 00:19:03 EDT
I'm just now finding some time to get back to these reviews.

This builds fine and rpmlint now finds nothing to complain about.

I'm afraid I don't understand your comments about the necessary size of the
window.  Perhaps what you're not understanding is that it fails to run
regardless of the size of the window; even on a full screen shell window with an
8pt font on a 24" monitor.  It flashes an almost unreadable message which I
thihnk tells me that the window size is 80x24 (which is most certainly isn't)
and then exits.

A minor nit: I think a summary of "Burn data and create songs with interactive
dialogs" is slightly more correct English.

Otherwise, though, the package is looking good and I'd approve it if I could run it.
Comment 40 Wilmer Jaramillo M. 2007-08-08 20:04:57 EDT
Updated with you suggestions:

Spec: http://mybashburn.sourceforge.net/mybashburn.spec
SRPM: http://mybashburn.sourceforge.net/mybashburn-1.0.2-1.src.rpm

Thanks.
Comment 41 Jason Tibbitts 2007-08-15 12:49:57 EDT
I'm confused; this package is older (release 1) than the package linked in
comment 38 (release 2).
Comment 42 Wilmer Jaramillo M. 2007-08-26 17:42:02 EDT
I'm sorry, I was confused too, it's just fixed, i can looking some changes in
rpmlint tool and was changed license name of GPL to GPLv2.

Update files:

SPEC: http://mybashburn.sourceforge.net/mybashburn.spec
RPMS: http://mybashburn.sourceforge.net/mybashburn-1.0.2-3.src.rpm
Comment 44 Wilmer Jaramillo M. 2008-01-02 12:23:40 EST
(In reply to comment #43)
> You should use this URL in the Source-Tag:
> http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.bz2

Fixed, updates both RPMS files: (release 3)

SPEC: http://mybashburn.sourceforge.net/mybashburn.spec
RPMS: http://mybashburn.sourceforge.net/mybashburn-1.0.2-3.src.rpm

* package builds in mock.
* package installs properly.
* rpmlint has valid complaints.

;)

Comment 45 Wilmer Jaramillo M. 2008-01-02 12:26:43 EST
[ERRATA] Sorry, the correct links of SRPM file is:
RPMS: http://mybashburn.sourceforge.net/mybashburn-1.0.2-3.fc8.src.rpm

Thanks for the review.
Comment 46 Jason Tibbitts 2008-01-15 17:20:10 EST
It seems to me that the tarball in the srpm is not the same as the upstream tarball.

Looking at the MyBashBurn.sh file, it's clear that the license should be GPLv2+ as they allow "any later version".

I believe those are the only issues remaining.

To refresh my memory, let me run through my checklist again:
X source files do not match upstream.
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
 description is OK.
 dist tag is present.
 build root is OK.
X license field matches the actual license (should be GPLv2+)
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper (none)
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly
* rpmlint is silent.
* final provides and requires are sane:
   mybashburn = 1.0.2-3.fc9
  =
   /bin/sh
   /usr/bin/env
   cdda2wav
   cdrdao
   cdrecord
   coreutils
   dialog >= 1.0
   dvd+rw-tools
   eject
   flac
   mkisofs
   vorbis-tools

* %check is not present; no test suite upstream.  I did some light testing
   (over the network so I didn't actually try to burn anything) and it seems
   to work OK.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
Comment 47 Wilmer Jaramillo M. 2008-01-27 22:32:04 EST
Hi Jason, I uploaded new files and some brief comments.
"source files do not match upstream.", yeah, I was possibly packaging SVN source 
files.
"license field matches the actual license (should be GPLv2+)" Changed to GPLv2+
according.

SPEC: http://wilmer.fedorapeople.org/files/review/mybashburn/mybashburn.spec
SRPMS:
http://wilmer.fedorapeople.org/files/review/mybashburn/SRPMS/mybashburn-1.0.2-3.fc8.src.rpm
RPMS:
http://wilmer.fedorapeople.org/files/review/mybashburn/RPMS/mybashburn-1.0.2-3.fc8.noarch.rpm
Comment 48 Jason Tibbitts 2008-01-31 15:00:16 EST
OK, the license is fixed, but the tarball still does not match the upstream tarball.

If you have a Source0: url of 
http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
then I simply must be able to download that tarball and get exactly, byte for
byte, what you are including in the srpm.  It is not acceptable for you to
modify that tarball in any way unless it contains content which we cannot
legally distribute.

If you are packaging something out of upstream SVN, we have rules about how you
document that.  You certainly cannot use the same version name because what you
want to distribute is not the same as what upstream ships with that version.  So
please see http://fedoraproject.org/wiki/Packaging/NamingGuidelines.  And if you
are going to package an SVN checkout then we also have rules about how you
document that so that I can make the same SVN checkout as you made:
http://fedoraproject.org/wiki/Packaging/SourceURL.

But honestly, do you have some compelling reason not to just ship what upstream
ships?  Just use their tarball in place of whatever you are trying to package.
Comment 49 Wilmer Jaramillo M. 2008-02-11 20:56:06 EST
Hi Jason, sorry for out long time, of course that there is a reason, I have a
problem with 'rpmlint' command, I get an encoding error about the file 'FILES'
of mybasburn upstream, they was writing in ISO-8859-1 and the CLI rpmlint
recommend using UTF-8, the output of rpmlint:
mybashburn.noarch: W: file-not-utf8 /usr/share/doc/mybashburn-1.0.2/FILES

So, I have to run this command for the conversion:
$ iconv -f ISO-8859-1 -t UTF8 FILES > FILES.tmp
$ mv FILES.tmp FILES

You recommend writing that in the SPEC file?
Comment 50 Jason Tibbitts 2008-02-11 21:27:37 EST
No, you should run iconv in the %prep section of the specfile to convert the
file at build time.  You don't make the changes in the tarball.
Comment 52 Jason Tibbitts 2008-02-13 13:33:30 EST
Cool, this looks good.  I believe all of the issues I had are now fixed.

APPROVED

I have already set you up in the account system, so you should be ready to make
your CVS request.
Comment 53 Wilmer Jaramillo M. 2008-02-13 16:39:08 EST
Jason, I received a email confirmation of you sponsorship.  I'm to get this
message when try to change fedora-cvs flag to '?' for the CVS admin attention:
"Flag Modification Denied
You tried to request fedora-cvs. Only an authorized user can make this change."

I have the existing memberships:
Group Name	Username	R.Domain R.Type	 R Status Sponsor
cvsextras 	wilmer 		user 	approved 	tibbs
fedorabugs 	wilmer 		user 	approved 	Unneeded

Any comment?
Comment 54 Jason Tibbitts 2008-02-13 21:41:06 EST
Well, you do have to give it time to propagate to all of the various systems and
databases.  I think this happens at least once an hour, so it should almost
certainly be working now.
Comment 55 Wilmer Jaramillo M. 2008-02-14 22:38:25 EST
New Package CVS Request
=======================
Package Name: mybashburn
Short Description: MyBashBurn - burn data and songs.
Owners: wilmer
Branches: F-8
InitialCC: wilmer
Cvsextras Commits: yes
Comment 56 Kevin Fenzi 2008-02-15 12:08:02 EST
Shouldn't the short description here be something like: 
"Burn data and create songs with interactive dialogs" ?

Comment 57 Wilmer Jaramillo M. 2008-02-17 21:18:09 EST
Hi Kevin, I guess that it sounds more correct in English.

New Package CVS Request
=======================
Package Name: mybashburn
Short Description: "Burn data and create songs with interactive dialogs."
Owners: wilmer
Branches: F-8
InitialCC: wilmer
Cvsextras Commits: yes
Comment 58 Kevin Fenzi 2008-02-18 12:21:35 EST
cvs done.
Comment 59 Wilmer Jaramillo M. 2008-02-18 21:45:31 EST
Thanks Kevin and specially to Jason for review.
Comment 60 Fedora Update System 2008-02-25 19:23:07 EST
mybashburn-1.0.2-3.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

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