[Bug 493246] Review Request: Shutter -- a feature-rich screenshot program.

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=493246


Jan Klepek <jan.klepek at hp.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
               Flag|                            |needinfo?(liangsuilong at gmai
                   |                            |l.com)




--- Comment #27 from Jan Klepek <jan.klepek at hp.com>  2009-07-29 10:24:41 EDT ---
(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.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.