Bug 2855

Request is missing a few things, see:
http://rpmfusion.org/Contributors#Create_a_package_review_request

Are you already a Fedora Packager?

Full URLs to the spec file and source rpm of the package.

spec:
http://fedorapeople.org/~klaatu/flowblade/flowblade.spec

source rpm:
http://fedorapeople.org/~klaatu/flowblade/flowblade-0.8.0-1.src.rpm

%description
Flowblade is a fast and efficient video editor written in Python 
and based on the MLT and ffmpeg engines.

Ineligible for Fedora due to ffmpeg dependency

This is my first RPM Fusion Package.
I am seeking a sponsor, as I am not a Fedora Packager.

please remove all this :

%define _topdir         /tmp/rpmbuild
%define buildroot %{_topdir}/%{name}-%{version}-root
BuildRoot:      	%{_topdir}/%{name}-buildroot
Prefix:                 /usr
%clean
rm -rf $RPM_BUILD_ROOT

are redundant or worst 


#% this will not comment, just you need remove %  

where is ? 
URL:
and 
Source0:

Updated spec file to roll in Sergio's suggestions.

URL to corrected spec file is the same as before:
http://fedorapeople.org/~klaatu/flowblade/flowblade.spec

URL for src RPM:
http://fedorapeople.org/~klaatu/flowblade/flowblade-0.8.0-1.fc18.src.rpm

rpmlint issues no errors, these warnings:
flowblade.src: W: spelling-error %description -l en_US ffmpeg -> imperf
## ignoring since 'ffmpeg' in description is properly spelled

flowblade.src: W: invalid-url Source0: https://flowblade.googlecode.com/files/flowblade-0.8.0.tar.gz HTTP Error 404: Not Found
## ignoring because a manual wget of that url works as expected.

Hi, looks good thanks, 

you may also clean :
rm -fr $RPM_BUILD_ROOT 

I can do a little review but I'm too busy to have this task .

(In reply to comment #5)
> Hi, looks good thanks, 
> 
> you may also clean :
> rm -fr $RPM_BUILD_ROOT 
> 
> I can do a little review but I'm too busy to have this task .

Sergio, in your previous comment (comment #3) I think you told me to take the rm -fr $RPM_BUILD_ROOT out, Did I misunderstand? For now I'm adding it back in because it feels right to be tidy.

(In reply to comment #6)
> (In reply to comment #5)
> > Hi, looks good thanks, 
> > 
> > you may also clean :
> > rm -fr $RPM_BUILD_ROOT 
> > 
> > I can do a little review but I'm too busy to have this task .
> 
> Sergio, in your previous comment (comment #3) I think you told me to take the
> rm -fr $RPM_BUILD_ROOT out, Did I misunderstand? For now I'm adding it back in
> because it feels right to be tidy.

You can remove it as it's done automatically. It hasn't been needed since Fedora 14. There's some corner cases when you have multiple sources, but that's not the case here.

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > Hi, looks good thanks, 
> > > 
> > > you may also clean :
> > > rm -fr $RPM_BUILD_ROOT 
> > > 
> > > I can do a little review but I'm too busy to have this task .
> > 
> > Sergio, in your previous comment (comment #3) I think you told me to take the
> > rm -fr $RPM_BUILD_ROOT out, Did I misunderstand? For now I'm adding it back in
> > because it feels right to be tidy.
> 
> You can remove it as it's done automatically. It hasn't been needed since
> Fedora 14. There's some corner cases when you have multiple sources, but that's
> not the case here.

Re-removed.

(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > (In reply to comment #5)
> > > > Hi, looks good thanks, 
> > > > 
> > > > you may also clean :
> > > > rm -fr $RPM_BUILD_ROOT 
> > > > 
> > > > I can do a little review but I'm too busy to have this task .
> > > 
> > > Sergio, in your previous comment (comment #3) I think you told me to take the
> > > rm -fr $RPM_BUILD_ROOT out, Did I misunderstand? For now I'm adding it back in
> > > because it feels right to be tidy.
> > 
> > You can remove it as it's done automatically. It hasn't been needed since
> > Fedora 14. There's some corner cases when you have multiple sources, but that's
> > not the case here.
> 
> Re-removed.

thanks, I though that you had miss that at first time ... 
but the reason that I ask you to do so, is when we use mock with --no-clean or --no-clean-after etc , for debug proposes, sometimes this cleans may change the mock behavior and may make us lose more time.

(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > (In reply to comment #6)
> > > > (In reply to comment #5)
> > > > > Hi, looks good thanks, 
> > > > > 
> > > > > you may also clean :
> > > > > rm -fr $RPM_BUILD_ROOT 
> > > > > 
> > > > > I can do a little review but I'm too busy to have this task .
> > > > 
> > > > Sergio, in your previous comment (comment #3) I think you told me to take the
> > > > rm -fr $RPM_BUILD_ROOT out, Did I misunderstand? For now I'm adding it back in
> > > > because it feels right to be tidy.
> > > 
> > > You can remove it as it's done automatically. It hasn't been needed since
> > > Fedora 14. There's some corner cases when you have multiple sources, but that's
> > > not the case here.
> > 
> > Re-removed.
> 
> thanks, I though that you had miss that at first time ... 
> but the reason that I ask you to do so, is when we use mock with --no-clean or
> --no-clean-after etc , for debug proposes, sometimes this cleans may change the
> mock behavior and may make us lose more time.

Understood, thanks.

Spec URL: http://fedorapeople.org/~klaatu/flowblade/flowblade.spec 
SRPM URL: http://fedorapeople.org/~klaatu/flowblade/flowblade-0.8.0-1.src.rpm

let see if "fedora-review -u https://bugzilla.rpmfusion.org/show_bug.cgi?id=2855"
works !

Spec URL: http://fedorapeople.org/~klaatu/flowblade/flowblade.spec 
SRPM URL: http://fedorapeople.org/~klaatu/flowblade/flowblade-0.8.0-1.fc18.src.rpm


fixed srpm URL

fedora-review -uhttps://bugzilla.rpmfusion.org/show_bug.cgi?id=2855
or 
fedora-review --other-bz https://bugzilla.rpmfusion.org -b 2855

(...)
error: %changelog not in descending chronological order

and breaks fedora-review :) 
ERROR: Exception down the road...(logs in /home/sergio/.cache/fedora-review.log) 
which is a bug in fedora-review , but we can't processed without fix changelog dates .

Thanks

(In reply to comment #13)
> fedora-review -uhttps://bugzilla.rpmfusion.org/show_bug.cgi?id=2855
> or 
> fedora-review --other-bz https://bugzilla.rpmfusion.org -b 2855
> 
> (...)
> error: %changelog not in descending chronological order
> 
> and breaks fedora-review :) 
> ERROR: Exception down the road...(logs in
> /home/sergio/.cache/fedora-review.log) 
> which is a bug in fedora-review , but we can't processed without fix changelog
> dates .
> 
> Thanks

Fixed changelog order.
Tried to run fedora-review but it failed; complains about me not running rawhide (true) and claims there is no /usr/bin/python (false) so I'm assuming I have mock or something configured incorrectly. This is the first I'd heard of fedora-review, so I am probably using it wrong.

Just some drive-by comments:

I'd guess that you are indeed missing /usr/bin/python in the mock chroot,  you're missing the mandatory python BuiltRequires, see http://fedoraproject.org/wiki/Packaging:Python#BuildRequires

BTW: fedora-review does nothing wrong here (or when crashing on bad changelog order). Basically, fedora-review expects the package to build in mock and cannot really do much about if it doesn't.

(In reply to comment #14)

> Fixed changelog order.
> Tried to run fedora-review but it failed; complains about me not running
> rawhide (true) and claims there is no /usr/bin/python (false) so I'm assuming I
> have mock or something configured incorrectly. This is the first I'd heard of
> fedora-review, so I am probably using it wrong.

try see if this doc helps:
http://www.serjux.com/alps/how_to_use_mock.txt


Command line :/usr/bin/fedora-review --other-bz https://bugzilla.rpmfusion.org -b 2855
pre-review:
===== SHOULD items =====

Generic:
[!]: Buildroot is not present
     Note: Invalid buildroot found: %{_topdir}/%{name}-buildroot
[!]: Dist tag is present.
[!]: update-mime-database is invoked as required
     Note: mimeinfo files in: flowblade
[!]: Spec use %global instead of %define.
     Note: %define name flowblade %define release 1%{?dist} %define version
     0.8.0

seems be related with:

[!]: Spec file according to URL is the same as in SRPM.
     Note: Spec file as given by url is not the same as in SRPM (see attached
     diff).

Spec URL: http://fedorapeople.org/~klaatu/flowblade/flowblade.spec 
SRPM URL:
http://fedorapeople.org/~klaatu/flowblade/flowblade-0.8.0-1.src.rpm


removed %dist, s/%define/%global/g, installed python2-devel...trying fedora-review again..

Still getting an error in mock logs regarding /usr/bin/python: no such file or directory

I've installed python2-devel and have added myself to mock group.

the relevant error seems to be this:

+ python setup.py build --build-base=/builddir/build/BUILDROOT/flowblade-0.8.0-1.i386
/var/tmp/rpm-tmp.jk9gxU: line 35: python: command not found

I've tried using both the macro %{python} and just plain 'python' in the spec file.

What am I missing? how can I ensure that /usr/bin/python gets included into my chroot environment?

(In reply to comment #18)
> Still getting an error in mock logs regarding /usr/bin/python: no such file or
> directory
> 
> I've installed python2-devel and have added myself to mock group.
> 
> the relevant error seems to be this:
> 
> + python setup.py build
> --build-base=/builddir/build/BUILDROOT/flowblade-0.8.0-1.i386
> /var/tmp/rpm-tmp.jk9gxU: line 35: python: command not found
> 
> I've tried using both the macro %{python} and just plain 'python' in the spec
> file.
> 
> What am I missing? how can I ensure that /usr/bin/python gets included into my
> chroot environment?

You add to your spec:

BuildRequires: python2-devel

as indicated in Comment #15.

(In reply to comment #19)
> (In reply to comment #18)
> > Still getting an error in mock logs regarding /usr/bin/python: no such file or
> > directory
> > 
> > I've installed python2-devel and have added myself to mock group.
> > 
> > the relevant error seems to be this:
> > 
> > + python setup.py build
> > --build-base=/builddir/build/BUILDROOT/flowblade-0.8.0-1.i386
> > /var/tmp/rpm-tmp.jk9gxU: line 35: python: command not found
> > 
> > I've tried using both the macro %{python} and just plain 'python' in the spec
> > file.
> > 
> > What am I missing? how can I ensure that /usr/bin/python gets included into my
> > chroot environment?
> 
> You add to your spec:
> 
> BuildRequires: python2-devel
> 
> as indicated in Comment #15.

one guess , you know that you need a lots of internet, every mock build downloads all packages from internet .

I think he got a little confused. When you use mock, what you have installed on your system (other than the mock requirements) are irrelevant. Mock builds your package in a clean chroot[1] and only installs some defaults and whatever you specify in "BuildRequires:" so the fact you have python2-devel installed on your system doesn't matter, it must be in the BuildRequires:.

[1] http://lwn.net/Articles/252794/

(In reply to comment #21)
> I think he got a little confused. When you use mock, what you have installed on
> your system (other than the mock requirements) are irrelevant. Mock builds your
> package in a clean chroot[1] and only installs some defaults and whatever you
> specify in "BuildRequires:" so the fact you have python2-devel installed on
> your system doesn't matter, it must be in the BuildRequires:.
> 
> [1] http://lwn.net/Articles/252794/

BuildRequires. that explains it. thanks.

Fixed everything. Now clearing failures from fedora-review review.txt..

Apparenltly %dist is required; adding it back in, updating urls one more time:

Spec URL: http://fedorapeople.org/~klaatu/flowblade/flowblade.spec 
SRPM URL:
http://fedorapeople.org/~klaatu/flowblade/flowblade-0.8.0-1.fc18.src.rpm

fedora-review failures:
SHOULD:
[!]: Dist tag is present.

##I have %dist included in the same way rpmdev-newspec does. Am I supposed to include it elsewhere as well?


[!]: update-mime-database is invoked as required
     Note: mimeinfo files in: flowblade

## I have it running in %post.  Is this incorrect? or perhaps it's related to the following and final error:

EXTRA:
[!]: Rpmlint is run on all installed packages.
     Note: Mock build failed

## there are no errors in build.log or results/build.log. Where do I look for an explanation of this one?

No, I think it's just confused because of the way you have your spec laid out...

%global name		flowblade
%global release		1%{?dist}
%global version		0.8.0

Get rid of this, just use the tags directly, i.e.:

Name:			flowblade
Version:		0.8.0
Release:		1%{?dist}

Also:

%install
rm -fr $RPM_BUILD_ROOT

kill the rm -fr $RPM_BUILD_ROOT...

(In reply to comment #24)
> No, I think it's just confused because of the way you have your spec laid
> out...
> 
> %global name        flowblade
> %global release        1%{?dist}
> %global version        0.8.0
> 
> Get rid of this, just use the tags directly, i.e.:
> 
> Name:            flowblade
> Version:        0.8.0
> Release:        1%{?dist}
> 
> Also:
> 
> %install
> rm -fr $RPM_BUILD_ROOT
> 
> kill the rm -fr $RPM_BUILD_ROOT...

Got rid of the %global declarations, got rid of the rm -fr $RPM_BUILD_ROOT.

fedora-review still fails in two places; update mime is not run, and rpmlint is not run on all packages. I think the install itself is failing; I see errors about it requiring mlt, ffmpeg, and some others, and I tried changing those to BuidRequires in spite of being pretty sure they aren't required to build a python application, and it made no difference.

(In reply to comment #25)

> I think the install itself is failing; I see errors
> about it requiring mlt, ffmpeg, and some others,

yeah I though about this, it will fail because fedora-review may use "fedora buildroot" and not use "rpmfusion buildroot" which of course will fail. I have to investigate this further , and find a solution for fedora-review works 100% with rpmfusion .

Yup, that's why. Try e. g.,  fedora-review --other-bz https://bugzilla.rpmfusion.org/ -b 2855 -m fedora-rawhide-i386-rpmfusion_nonfree or use the example on https://fedorahosted.org/FedoraReview/wiki/UsageScenarios

fedora-review's devel version has this message, I don't think the released version  handles it:
- update-desktop-database is invoked as required
  Note: desktop file(s) with MimeType entry in flowblade
  See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-
  database

Thanks for the help.

I tried the dev version of fedora-review, fixed the %post and %postun command as recommended in the output of (try-)fedora-review and I get no further errors.

rpmlint still throws an error about a non-executable file; this seems to  be more of a setup.py issue than mine, but I added a find/chmod to fix it. Strictly speaking, I'm not even sure it's necessary, since I've never seen an issue with it when using Flowblade.

Created attachment 1126 [details]
package versioning

Hi, 
I suggest that you version yours change , like my patch does ,is much more readable. 
other suggestion is build src.rpm with rpmbuild, to .spec not have differences of .spec in src.rpm like this 
 
rpmbuild -bs flowblade.spec --define "_sourcedir ../srpm-unpacked/" --define "_srcrpmdir ." 

you may define where is the SOURCES dir and where is wrote src.rpm .

(In reply to comment #29)
> Created attachment 1126 [details]
> package versioning
> 
> Hi, 
> I suggest that you version yours change , like my patch does ,is much more
> readable. 
> other suggestion is build src.rpm with rpmbuild, to .spec not have differences
> of .spec in src.rpm like this 
> 
> rpmbuild -bs flowblade.spec --define "_sourcedir ../srpm-unpacked/" --define
> "_srcrpmdir ." 
> 
> you may define where is the SOURCES dir and where is wrote src.rpm .

patch applied, new src rpm uploaded.

spec:
http://fedorapeople.org/~klaatu/flowblade/flowblade.spec

source rpm:
http://fedorapeople.org/~klaatu/flowblade/flowblade-0.8.0-4.fc18.src.rpm

Hi, 
(In reply to comment #30) 
> patch applied, new src rpm uploaded.

now begins the hard part :) 

Issues:
=======
- update-desktop-database is invoked when required
  Note: desktop file(s) in flowblade
  See: http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Icon_Cache
- Package installs a %{name}.desktop using desktop-file-install if there is
  such a file.
  See: http://fedoraproject.org/wiki/Packaging/Guidelines#desktop

I had to do the same with smb4k and if I have time right now, I had do it for you .
see http://pkgs.fedoraproject.org/cgit/smb4k.git/tree/smb4k.spec#n42
begins in line 42 to 76 

you need use desktop-file-install and update-desktop-database

Closing due to inactivity, please reopen if you're interested in pursuing this review request.

Amending my standard note above as I'm doing some bugzilla housekeeping. I would really like to see this in RPM Fusion.

While is is preferable to be sponsored in Fedora first it's not a requirement, but if you have time, please consider submitting a package there and if you need help picking a package start here:

https://fedoraproject.org/wiki/Package_maintainers_wishlist?rd=PackageMaintainers/WishList

*** This bug has been marked as a duplicate of bug 3694 ***