Bug 841418

- drop BuildRoot and defattr (below %files)
- the use of %attr seems a little bit over the top. is it really necessary to specify all those?
- make sure to use cp -a whenever possible
- rename spec file to prey.spec

- The BuildRoot is because I want to support el5. I removed defattr
- The use of %attr is because the source archive has incorrect permissions, and a too broad %attr (for the while dir) is not possible either, because the version "scripts" cannot have executable permissions
- cp -a fixed
- I put it as prey.spec

new Spec URL: http://puiterwijk.fedorapeople.org/packages/prey/prey.spec
new SRPM URL: http://puiterwijk.fedorapeople.org/packages/prey/prey-0.5.3-2.fc17.src.rpm
new Koji URL: http://koji.fedoraproject.org/koji/taskinfo?taskID=4324586

I'll review it.

REVIEW:

Legend: + = PASSED, - = FAILED, 0 = Not Applicable

+ rpmlint is silent

work ~/Desktop: rpmlint prey-0.5.3-2.fc18.noarch.rpm prey-0.5.3-2.fc18.src.rpm
2 packages and 0 specfiles checked; 0 errors, 0 warnings.
work ~/Desktop: 

+ The package is named according to the  Package Naming Guidelines.
+ The spec file name matches the base package %{name}, in the format %{name}.spec.
+ The package meets the Packaging Guidelines.
+ The package is licensed with a Fedora approved license and meets the Licensing Guidelines.
+ The License field in the package spec file matches the actual license (strict GPLv3).
+ The file, containing the text of the license(s) for the package, is included in %doc.
+ The spec file is written in American English.
+ The spec file for the package is legible.
+ The sources used to build the package, match the upstream source, as provided in the spec URL.

work ~/Desktop: sha256sum prey-0.5.3-linux.zip*
fdeed1987d30b4ca585e19ac9884194bc8a108bd56112d2913206fc85be3bd9b  prey-0.5.3-linux.zip
fdeed1987d30b4ca585e19ac9884194bc8a108bd56112d2913206fc85be3bd9b  prey-0.5.3-linux.zip.1
work ~/Desktop: 

+ The package successfully compiles and builds into binary rpms on at least one primary architecture.
+ All build dependencies are listed in BuildRequires.
0 No need to handle locales.
0 No shared library files.
+ The package does NOT bundle copies of system libraries.
+ The package is not designed to be relocatable.
+ The package owns all directories that it creates.
+ The package does not list a file more than once in the spec file's %files listings.
+ Permissions on files are set properly.

-/0 The package DOESN'T have a %clean section, which contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT). That's a problem if you plan to build for EL5.

+ The package consistently uses macros.
+ The package contains code, or permissible content.
0 No extremely large documentation files.
+ Anything, the package includes as %doc, does not affect the runtime of the application.
0 No header files.
0 No static libraries.
0 No pkgconfig(.pc) files.
0 The package doesn't contain library files with a suffix (e.g. libfoo.so.1.1).
0 No devel sub-package.
+ The package does NOT contain any .la libtool archives.
0 Not a GUI application.
+ The package does not own files or directories already owned by other packages.

-/0 At the beginning of %install, the package DOESN'T run rm -rf %{buildroot} (or $RPM_BUILD_ROOT). That's a problem if you plan to build for EL5.

+ All filenames in rpm packages are valid UTF-8.

Please, consider adding %clean section and add rm -rf %{buildroot} if you plan to build for EL5.

I don't see any other issues so this package is 

APPROVED.

(In reply to comment #4)

> I don't see any other issues so this package is 

But I do. This package is _far_, _far_ from being FHS compliant. It installs scripts, config files and OS depended files into ${datadir}.

I wonder how many false positives you will get behind hotspots. perhaps it should first check fedoraproject.org/static/hotspot.txt ?

(In reply to comment #5)
> (In reply to comment #4)
> 
> > I don't see any other issues so this package is 
> 
> But I do. This package is _far_, _far_ from being FHS compliant. It installs
> scripts, config files and OS depended files into ${datadir}.
I will look into the locations, but the config file which should override all others is placed as /etc/sysconfig/prey, and I wonder which OS dependant files you mean, as it is just a bunch of bash scripts?

(In reply to comment #6)
> I wonder how many false positives you will get behind hotspots. perhaps it 
> should first check fedoraproject.org/static/hotspot.txt ?
It is not activated by IP, but by the user registering the device as stolen on the control panel, or removing a file on a web server.

Thanks for all the comments, I am sorry if my answers were harsh and non-welcoming.


I have looked into the locations, and after re-reading the FHS, I have placed the scripts in libexecdir/prey, and all of the config files in /etc/sysconfig/prey.

I also added the check for hotspot.txt.


new Spec URL: http://puiterwijk.fedorapeople.org/packages/prey/prey.spec
new SRPM URL: http://puiterwijk.fedorapeople.org/packages/prey/prey-0.5.3-3.fc17.src.rpm
new Koji URL: http://koji.fedoraproject.org/koji/taskinfo?taskID=4332846

(In reply to comment #7)
> and I wonder which OS dependant
> files you mean, as it is just a bunch of bash scripts?

The FHS defines datadir as "read-only architecture-independent data"

Note the "data" and "architecture-independent":

=> These files are arguably "non-arch-independent":
%attr(755, -, -) %{_datadir}/%{name}/platform/linux/functions
%attr(644, -, -) %{_datadir}/%{name}/platform/linux/prey-config.glade
%attr(755, -, -) %{_datadir}/%{name}/platform/linux/prey-config.py*
%attr(755, -, -) %{_datadir}/%{name}/platform/linux/settings
%attr(755, -, -) %{_datadir}/%{name}/platform/linux/bin/sendEmail

(The idea behind datadir (/usr/share) is them begin sharable between different OSes/architectures, e.g. through nfs mounts)


=> Theses files are executable:
%attr(755, -, -) %{_datadir}/%{name}/platform/linux/functions
%attr(755, -, -) %{_datadir}/%{name}/platform/linux/prey-config.py*
%attr(755, -, -) %{_datadir}/%{name}/platform/linux/settings
%attr(755, -, -) %{_datadir}/%{name}/platform/linux/bin/sendEmail

They aren't data, they are executables and therefore do not belong into %{_datadir} but should be placed into %{_libdir}/%{name} or %{_libexecdir}/%{name}


That said, a simple solution would be to install all these files into %{_libdir}/%{name} instead of %{_datadir}/%{name}

(In reply to comment #9)
> (In reply to comment #7)
> > and I wonder which OS dependant
> > files you mean, as it is just a bunch of bash scripts?
> 
> The FHS defines datadir as "read-only architecture-independent data"
> 
> Note the "data" and "architecture-independent":
> 
> => These files are arguably "non-arch-independent":
> %attr(755, -, -) %{_datadir}/%{name}/platform/linux/functions
> %attr(644, -, -) %{_datadir}/%{name}/platform/linux/prey-config.glade
> %attr(755, -, -) %{_datadir}/%{name}/platform/linux/prey-config.py*
> %attr(755, -, -) %{_datadir}/%{name}/platform/linux/settings
> %attr(755, -, -) %{_datadir}/%{name}/platform/linux/bin/sendEmail

Actually those python, bash and perl scripts are arch independent. They don't must be OS independent.

Well, they are arch independant, but I agree they are not "data".
That's why I placed them in libexec, according to the packaging guidelines: "Libexecdir (aka, /usr/libexec on Fedora systems) should only be used as the directory for executable programs that are designed primarily to be run by other programs rather than by users. ", as they are executable, but the user will not execute them (crond will).

I removed the review approval flag because the review did not catch serious mistakes. Also the current spec file does not conform to the current packaging guidelines (e.g. the LICENSE file is misplaced).

This is an automatic check from review-stats script.

This review request ticket hasn't been updated for some time. We're sorry
it is taking so long. If you're still interested in packaging this software
into Fedora repositories, please respond to this comment clearing the
NEEDINFO flag.

You may want to update the specfile and the src.rpm to the latest version
available and to propose a review swap on Fedora devel mailing list to increase
chances to have your package reviewed. If this is your first package and you
need a sponsor, you may want to post some informal reviews. Read more at
https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group.

Without any reply, this request will shortly be considered abandoned
and will be closed.
Thank you for your patience.

This is an automatic action taken by review-stats script.

The ticket submitter failed to clear the NEEDINFO flag in a month.
As per https://fedoraproject.org/wiki/Policy_for_stalled_package_reviews
we consider this ticket as DEADREVIEW and proceed to close it.