Bug 493246

Missed BuildRequires: desktop-file-utils

Here is a new package. I have added the missed buildrequires.
  
SRPM: http://liangsuilong.fedorapeople.org/shutter/shutter-0.70.2-2.ppa3.fc10.src.rpm

SPEC: http://liangsuilong.fedorapeople.org/shutter/shutter.spec

(Removing NEEDSPONSOR)

Hi,

1] md5sum of tarball from Source0 differs from tarball in src.rpm
2] specfile have useless comments
# documents
#chmod -x debian/*

3] files are listed multiple times
Processing files: shutter-0.70.2-2.ppa3.fc10
warning: File listed twice: /usr/share/locale/ar/LC_MESSAGES/shutter.mo
warning: File listed twice: /usr/share/locale/bg/LC_MESSAGES/shutter.mo
warning: File listed twice: /usr/share/locale/ca/LC_MESSAGES/shutter.mo
warning: File listed twice: /usr/share/locale/cs/LC_MESSAGES/shutter.mo
warning: File listed twice: /usr/share/locale/da/LC_MESSAGES/shutter.mo
....

4] rpmlint is complaining a lot (on spec file, src.rpm and on final rpm)

Please read 
http://fedoraproject.org/wiki/Packaging/ReviewGuidelines
http://fedoraproject.org/wiki/Packaging/Guidelines

If you need help, do not hesitate to contact me 
or anybody from mentors ( https://fedoraproject.org/wiki/PackageMaintainers/Join#Getting_Help )

Created attachment 341386 [details]
the latest spec file

Jan Klepek,

I have corrected the spec file. Rpmlint does no longer complain anything. But now the problem is that rpmbuild add perl(Proc::Simple) to Requires, which does not exist in Fedora official repository. I can make sure that it is not needed. Because the old package which I build can run well on my Fedora 10. I upload the latest spec file as an attachment.

Should I obsolete perl(Proc::Simple)??

1]
what is meaning of share/shutter/resources/system/plugins directory in your tarball?
in bash plugins you are using convert from imagemagick, without imagemagick in requirements this plugins wouldn't work.
Are this plugins (both, perl and bash) needed for work of shutter? or they just to extend functionality?

2]
in bin/shutter you have "use Proc::Simple" that's why rpmbuild add it as requirements.

3] 
rpmlint is still not silent, no %build section in spec file
%build could be empty 

4] 
you have %doc without any file specified

5] 
during %prep you delete all files in app-install directory but no directory, why not remove whole directory?

6] 
why do you want to have share/resources/modules in /usr? Do you have any reason why not to put perl modules into better location?
see: http://fedoraproject.org/wiki/Packaging/Perl

7] 
please do not use "mv", use install -p or cp -p 
see: https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps

Hi, 

any news?

Jan Klepek，

Yes, I am still here. But you know Shutter depends on perl-Goo-Canvas and perl-Gnome2-Wnck. So I want to finish the submitting process of these two package. Now the process will be end soon. Later I will get into this process again.

Jan Klepek,

SRPM:http://liangsuilong.fedorapeople.org/shutter/shutter-0.70.2-2.ppa4.fc10.src.rpm
SPEC:http://liangsuilong.fedorapeople.org/shutter/shutter.spec

I upload the latest package for shutter. But I do not test it on koji server.

I test it in my local rpmlint. There is no warning and no error.

(In reply to comment #6)
> 1]
> what is meaning of share/shutter/resources/system/plugins directory in your
> tarball?
> in bash plugins you are using convert from imagemagick, without imagemagick in
> requirements this plugins wouldn't work.
> Are this plugins (both, perl and bash) needed for work of shutter? or they just
> to extend functionality?

this questions is still here. In other words, could shutter perform basic functionality without plugins?

Of capturing a screenshot, yes. goocanvas is also optional along with image manipulation plugins, but it would severely impact the basic features of the program.

Jan Klepek,

Do you know deluge and vuze? They also contain a lot of plugins by default. Although Some features are added into the application, they have an important effect on this program. 

So in my opinion there is no need to divide shutter into two packages -- shutter and shutter-plugins. As a matter of fact, it is not convenient for users to install shutter.

OK! I try to build it on koji. It is OK.

http://koji.fedoraproject.org/koji/taskinfo?taskID=1426116

I think that it should be approved.

1] why do you have download_version different from Version?
from specfile:
%define  download_version 0.70
Name:           shutter
Version:        0.70.2
Why don't use Version instead of download_version?

2] in %install you are creating directory named "0755" in bin/ and share/

3] after install you have following directories in /usr/share/shutter/resources:
pofiles: tar.gz-ed files from application, why do you need them here? 
modules: File, Proc perl modules, as they mostly have separate packages (except File::Spec and Proc::Simple which is still present in codes and seems that it is required in case that you want to cancel operation), you have to use packaged version of them.
Please create packages for missing dependencies (File::Spec and Proc::Simple).

Koji build isn't only one thing which is required before package is approved.

(In reply to comment #14)
> 1] why do you have download_version different from Version?
> from specfile:
> %define  download_version 0.70
> Name:           shutter
> Version:        0.70.2
> Why don't use Version instead of download_version?
 
The real version is 0.70.2, but the tarball of source code which author provides is shutter-0.70. I do not know why the author do it. Maybe we should contract with author and persuade him to correct the version.  

> 2] in %install you are creating directory named "0755" in bin/ and share/

Are there any problems? I think it is OK for us.

> 3] after install you have following directories in
> /usr/share/shutter/resources:
> pofiles: tar.gz-ed files from application, why do you need them here? 
> modules: File, Proc perl modules, as they mostly have separate packages (except
> File::Spec and Proc::Simple which is still present in codes and seems that it
> is required in case that you want to cancel operation), you have to use
> packaged version of them.
> Please create packages for missing dependencies (File::Spec and Proc::Simple).
> 
> Koji build isn't only one thing which is required before package is approved.  

In my opinion, these directories in /usr/share/shutter/resources: 

1. pofiles: maybe they should be remove. Let me have a try! If it is OK, I will remove them.

2. modules: It is not a good choice to remove them as I see. If we remove them, we will need to create packages for (File::Spec and Proc::Simple). If (File::Spec and Proc::Simple) upgrade to new version, it will be not compatible with shutter. So I think we should use the perl module which author offers. Just like some linux software with static libraries, It can provide the best compatibility with their software. 

Yes, I know koji build is an important thing which is require before package is approved, but not the only one.

(In reply to comment #15)
> (In reply to comment #14)
> > 1] why do you have download_version different from Version?
> > from specfile:
> > %define  download_version 0.70
> > Name:           shutter
> > Version:        0.70.2
> > Why don't use Version instead of download_version?
> 
> The real version is 0.70.2, but the tarball of source code which author
> provides is shutter-0.70. I do not know why the author do it. Maybe we should
> contract with author and persuade him to correct the version.  
> 

Upstream provides 0.70.2 ( http://shutter-project.org/wp-content/uploads/releases/tars/ )

> > 2] in %install you are creating directory named "0755" in bin/ and share/
> 
> Are there any problems? I think it is OK for us.
> 
install command create this directories, please fix your %install section. There is no reason to create them at first place.

> 2. modules: It is not a good choice to remove them as I see. If we remove them,
> we will need to create packages for (File::Spec and Proc::Simple). If
> (File::Spec and Proc::Simple) upgrade to new version, it will be not compatible
> with shutter. So I think we should use the perl module which author offers.
> Just like some linux software with static libraries, It can provide the best
> compatibility with their software. 

It is much wise choice to have them as separate packages. It will provide much more benefit to fedora. And as per my opinion it breaks following rule: http://fedoraproject.org/wiki/Packaging/Guidelines#Duplication_of_system_libraries

I could very hardly imagine that next release of File::Spec and Proc::Simple will be non-compatible with shutter. We could require specific version of File::Spec or Proc::Simple packages.
It is same as for Perl-Goo-Canvas or Perl-Gnome2-Wnck.

I have started creating package of File::Spec and Proc::Simple and will submit them during today to help you.

(In reply to comment #16)
> (In reply to comment #15)
> > (In reply to comment #14)
> > > 1] why do you have download_version different from Version?
> > > from specfile:
> > > %define  download_version 0.70
> > > Name:           shutter
> > > Version:        0.70.2
> > > Why don't use Version instead of download_version?
> > 
> > The real version is 0.70.2, but the tarball of source code which author
> > provides is shutter-0.70. I do not know why the author do it. Maybe we should
> > contract with author and persuade him to correct the version.  
> > 
> 
> Upstream provides 0.70.2 (
> http://shutter-project.org/wp-content/uploads/releases/tars/ )

OK！ I know I am wrong. Thank you for your notification.

> > > 2] in %install you are creating directory named "0755" in bin/ and share/
> > 
> > Are there any problems? I think it is OK for us.
> > 
> install command create this directories, please fix your %install section.
> There is no reason to create them at first place.

That is a good idea!

> > 2. modules: It is not a good choice to remove them as I see. If we remove them,
> > we will need to create packages for (File::Spec and Proc::Simple). If
> > (File::Spec and Proc::Simple) upgrade to new version, it will be not compatible
> > with shutter. So I think we should use the perl module which author offers.
> > Just like some linux software with static libraries, It can provide the best
> > compatibility with their software. 
> 
> It is much wise choice to have them as separate packages. It will provide much
> more benefit to fedora. And as per my opinion it breaks following rule:
> http://fedoraproject.org/wiki/Packaging/Guidelines#Duplication_of_system_libraries
> 
> I could very hardly imagine that next release of File::Spec and Proc::Simple
> will be non-compatible with shutter. We could require specific version of
> File::Spec or Proc::Simple packages.
> It is same as for Perl-Goo-Canvas or Perl-Gnome2-Wnck.
> 
> I have started creating package of File::Spec and Proc::Simple and will submit
> them during today to help you.  

Yes. thank you too. So I need to remove /usr/share/shutter/resources/module/File and /usr/share/shutter/resources/module/Proc. But I think that /usr/share/shutter/resources/module/Shutter should not be removed.

(In reply to comment #17)
> Yes. thank you too. So I need to remove
> /usr/share/shutter/resources/module/File and
> /usr/share/shutter/resources/module/Proc. But I think that
> /usr/share/shutter/resources/module/Shutter should not be removed.  


Yes, thats correct
File::Spec is in perl package
Proc::Simple was submitted for review
Shutter modules should stay

Jan Klepek

Now I have uploaded a new SRPM and a new SPEC. I have removed File and Proc module included in source code tarball. 

SRPM:http://liangsuilong.fedorapeople.org/shutter/shutter-0.70.2-3.ppa4.fc11.src.rpm
SPEC:http://liangsuilong.fedorapeople.org/shutter/shutter.spec

Also I find a new version shutter will be released. Shutter testing team is testing shutter-0.80 in LaunchPad. Shutter-0.80 add many new features. I build a package for shutter-0.80~ppa6. It also requires your perl-Proc-Simple. Here are SRPM and SPEC.

SRPM:http://liangsuilong.fedorapeople.org/shutter-testing/shutter-0.80-1.ppa6.fc11.src.rpm
SPEC:http://liangsuilong.fedorapeople.org/shutter-testing/shutter.spec

I hope your perl-Proc-Simple will be approved soon.

Jan Klepek

I upload the latest shutter to my fedorapeople.

SRPM:http://liangsuilong.fedorapeople.org/shutter/shutter-0.80-2.fc10.src.rpm
SPEC:http://liangsuilong.fedorapeople.org/shutter/shutter.spec

1] why is shutter.spec with 0777 rights in src.rpm
2] spec file mentioned https://bugzilla.redhat.com/show_bug.cgi?id=493246#c20
doesn't match one in src.rpm
3] dependency issues:
in shutter-0.80/share/shutter/resources/modules/Shutter/Screenshot/Window.pm you have require X11::Protocol; but it is not listed in requires (and is not picked up automatically by rpm)

(In reply to comment #21)
> 1] why is shutter.spec with 0777 rights in src.rpm
It seems not to be important, I think. But later I will recreate a new spec file with default right.

> 2] spec file mentioned https://bugzilla.redhat.com/show_bug.cgi?id=493246#c20
> doesn't match one in src.rpm
Oh, I know I uploaded a wrong spec file.

> 3] dependency issues:
> in shutter-0.80/share/shutter/resources/modules/Shutter/Screenshot/Window.pm
> you have require X11::Protocol; but it is not listed in requires (and is not
> picked up automatically by rpm)  
When I installed shutter-0.80 in Fedora 11, it seems that yum would automatically install all the requires that shutter needs. I could make sure I did not set up X11::Protocol before installing shutter. 

Even that, I will list X11::Protocol in requires.

Jan Klepek

Here is a new srpm and spec file.
SRPM:http://liangsuilong.fedorapeople.org/shutter/shutter-0.80-3.fc11.src.rpm
SPEC:http://liangsuilong.fedorapeople.org/shutter/shutter.spec

I have listed perl(X11::Protocol) in requires.

ping again

looks ok, last small things...
1] shutter.src: W: mixed-use-of-spaces-and-tabs (spaces: line 2, tab: line 12)

> > 2] in %install you are creating directory named "0755" in bin/ and share/
> 
> Are there any problems? I think it is OK for us.
> 
When i think about it, you probably want to do something else by following commands:
install -d -p 0755 $RPM_BUILD_ROOT%{_bindir}
install -d -p 0755 $RPM_BUILD_ROOT%{_datadir}
cp -fr -p 0755 bin/* $RPM_BUILD_ROOT%{_bindir}/
cp -rf -p 0755 share/* $RPM_BUILD_ROOT%{_datadir}/

if you want to set permissions, you have to use switch -m (for install, cp doesn't allow this as far as I know)
from this POV i don't think that spec file is legible (incorrectly used commands)

I'm sorry for lag between my answers/replies, work takes more time nowadays

(In reply to comment #25)
> looks ok, last small things...
> 1] shutter.src: W: mixed-use-of-spaces-and-tabs (spaces: line 2, tab: line 12)
> 
> > > 2] in %install you are creating directory named "0755" in bin/ and share/
> > 
> > Are there any problems? I think it is OK for us.
> > 
> When i think about it, you probably want to do something else by following
> commands:
> install -d -p 0755 $RPM_BUILD_ROOT%{_bindir}
> install -d -p 0755 $RPM_BUILD_ROOT%{_datadir}
> cp -fr -p 0755 bin/* $RPM_BUILD_ROOT%{_bindir}/
> cp -rf -p 0755 share/* $RPM_BUILD_ROOT%{_datadir}/
> 
> if you want to set permissions, you have to use switch -m (for install, cp
> doesn't allow this as far as I know)
> from this POV i don't think that spec file is legible (incorrectly used
> commands)
> 
> I'm sorry for lag between my answers/replies, work takes more time nowadays  

OK, Thanks a lot. 

Do you mean that I should replace cp with install. I know install command only can install files but not directories. If I replace cp with install, I will need to write a lot of commands to finish an installation action. I do not believe that it is a good way to maintain a package.

I remember that you ever told me that mv is illegal in the rule of package review. I read the spec file of shutter for Mandriva and PCLinuxOS. I found they are using mv not install. 

Here is installation script:
install -d -m 0755 %{buildroot}
install -d -m 0755 %{buildroot}/usr
mv bin %{buildroot}/usr
mv share %{buildroot}/usr

So I think I modify the script to that: 
install -d -p 0755 $RPM_BUILD_ROOT%{_bindir}
install -d -p 0755 $RPM_BUILD_ROOT%{_datadir}
cp -fr bin/* $RPM_BUILD_ROOT%{_bindir}/
cp -rf share/* $RPM_BUILD_ROOT%{_datadir}/
chmod 0755 -R $RPM_BUILD_ROOT%{_bindir}
chmod 0755 -R $RPM_BUILD_ROOT%{_datadir}

Is it OK? I think that option -p of cp is the same as -m of install. 

If it is not OK, Could you help me write an installation script for shutter. Thank you.

(In reply to comment #26)
> (In reply to comment #25)
> > looks ok, last small things...
> > 1] shutter.src: W: mixed-use-of-spaces-and-tabs (spaces: line 2, tab: line 12)
> > 

Have you fixed this?

> > > > 2] in %install you are creating directory named "0755" in bin/ and share/
> > > 
> > > Are there any problems? I think it is OK for us.
> > > 
> > When i think about it, you probably want to do something else by following
> > commands:
> > install -d -p 0755 $RPM_BUILD_ROOT%{_bindir}
> > install -d -p 0755 $RPM_BUILD_ROOT%{_datadir}
> > cp -fr -p 0755 bin/* $RPM_BUILD_ROOT%{_bindir}/
> > cp -rf -p 0755 share/* $RPM_BUILD_ROOT%{_datadir}/
> > 
> > if you want to set permissions, you have to use switch -m (for install, cp
> > doesn't allow this as far as I know)
> > from this POV i don't think that spec file is legible (incorrectly used
> > commands)
> > 
> > I'm sorry for lag between my answers/replies, work takes more time nowadays  
> 
> OK, Thanks a lot. 
> 
> Do you mean that I should replace cp with install. I know install command only
> can install files but not directories. If I replace cp with install, I will
> need to write a lot of commands to finish an installation action. I do not
> believe that it is a good way to maintain a package.

> I remember that you ever told me that mv is illegal in the rule of package
> review. I read the spec file of shutter for Mandriva and PCLinuxOS. I found
> they are using mv not install. 

mandriva/pclinuxos are not fedora, they probably have different packaging guidelines.
please read man cp(1), man install(1).

for install command, -p switch doesn't take any argument and you are passing argument to it. If you want to specify permissions/mode you have to use -m switch and still keep there -p for preserving timestamps.

for cp command, switch -p is to preserve timestamps(and other) and not to set mode/permissions. And if needed you have to chmod/chown later. 
 
> Here is installation script:
> install -d -m 0755 %{buildroot}
> install -d -m 0755 %{buildroot}/usr
> mv bin %{buildroot}/usr
> mv share %{buildroot}/usr
> 
> So I think I modify the script to that: 
> install -d -p 0755 $RPM_BUILD_ROOT%{_bindir}
> install -d -p 0755 $RPM_BUILD_ROOT%{_datadir}
> cp -fr bin/* $RPM_BUILD_ROOT%{_bindir}/
> cp -rf share/* $RPM_BUILD_ROOT%{_datadir}/
> chmod 0755 -R $RPM_BUILD_ROOT%{_bindir}
> chmod 0755 -R $RPM_BUILD_ROOT%{_datadir}
> 
> Is it OK? I think that option -p of cp is the same as -m of install. 
>

No,switch -p of cp is almost same as -p of install and none of it is same as -m of install.
 
> If it is not OK, Could you help me write an installation script for shutter.
> Thank you.

Thank you for helping me a lot.

Now I think that there is no need to reset the permissions/mode. I just need to copy the files and directories to $RPM_BUILD_ROOT%{_datadir} and $RPM_BUILD_ROOT%{_bindir}.

The install script is:
install -d -m 0755 $RPM_BUILD_ROOT%{_bindir}
install -d -m 0755 $RPM_BUILD_ROOT%{_datadir}
cp -fr bin/* $RPM_BUILD_ROOT%{_bindir}/
cp -fr share/* $RPM_BUILD_ROOT%{_datadir}/


I check the SRPM and RPM and spec file. The result is in this txt file and All of them are OK.
http://fedorapeople.org/~liangsuilong/rpmlint.check.txt

SRPM: http://fedorapeople.org/~liangsuilong/shutter/shutter-0.80-4.fc11.src.rpm
SPEC: http://fedorapeople.org/~liangsuilong/shutter/shutter.spec

Liang, 

why did you put -p switch off? I'm sorry if you got feeling that -p switch is not mandatory.

The install script should look like:

install -d -m 0755 -p $RPM_BUILD_ROOT%{_bindir}
install -d -m 0755 -p $RPM_BUILD_ROOT%{_datadir}
cp -pfr bin/* $RPM_BUILD_ROOT%{_bindir}/
cp -pfr share/* $RPM_BUILD_ROOT%{_datadir}/

Please read what I have written above and try to understand what is difference between what I have proposed and what do you have and which one meets packaging guidelines.
see: https://fedoraproject.org/wiki/Packaging/Guidelines#Timestamps

Please with next version of specfile do a scratch build for F-11 and F-10 in koji.

Thank you,

I am so sorry that I am too late to upload the new pacakage for the reason that I am busy for another thing. Now I upload them and correct the install script. 

SRPM: http://liangsuilong.fedorapeople.org/shutter/shutter-0.80-5.fc11.src.rpm
SPEC: http://liangsuilong.fedorapeople.org/shutter/shutter.spec

I also build it in koji build system. It is OK.

F-12: https://koji.fedoraproject.org/koji/taskinfo?taskID=1573736
F-11: https://koji.fedoraproject.org/koji/taskinfo?taskID=1573840
F-10: https://koji.fedoraproject.org/koji/taskinfo?taskID=1573755

I know I was wrong. Maybe the reaseon is that my English is too bad so that I was hardly understand what you said. But now，after reading the packaging guidelines, I correct my problem. 

Thank you, Jan.

approved

I upgrade a new version shutter. Here is shutter-0.80.1

SRPM: http://liangsuilong.fedorapeople.org/shutter/shutter-0.80-5.fc11.src.rpm
SPEC: http://liangsuilong.fedorapeople.org/shutter/shutter.spec

New Package CVS Request
=======================
Package Name: shutter
Short Description: Shutter is a GTK+ 2.0 screenshot application written in perl.
Owners: liangsuilong
Branches: F-10 F-11 devel
InitialCC: liangsuilong
Cvsextras Commits: yes

cvs done.

Hi Liang,

please close this ticket when package submitted into bodhi.

Thanks

Now, this bug report has been closed.

Package Change Request
======================
Package Name: shutter
New Branches: el6 epel7
Owners: cheeselee

Git done (by process-git-requests).