Bug 1124994

Fixed some rpmlint errors/warnings.

Spec URL: https://gbailey.fedorapeople.org/dl/0.12-2/dl.spec
SRPM URL: https://gbailey.fedorapeople.org/dl/0.12-2/dl-0.12-2.fc20.src.rpm

The only error shown by rpmlint is:

Checking: dl-0.12-2.fc20.noarch.rpm
          dl-0.12-2.fc20.src.rpm
dl.noarch: E: non-standard-dir-perm /var/spool/dl 0700L
2 packages and 0 specfiles checked; 1 errors, 0 warnings.

/var/spool/dl should be visible only to the webserver, so the "standard" directory permissions of 0755 don't make sense, IMHO.

Updated for DL 0.13, released today.

Spec URL: https://gbailey.fedorapeople.org/dl/0.13-1/dl.spec
SRPM URL: https://gbailey.fedorapeople.org/dl/0.13-1/dl-0.13-1.fc20.src.rpm

[herrold@centos-6 dl]$ rpmlint /home/herrold/rpmbuild/SRPMS/dl-0.13-1.orc6.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

[herrold@centos-6 dl]$ rpmlint /home/herrold/rpmbuild/RPMS/noarch/dl-0.13-1.orc6.noarch.rpm
dl.noarch: W: incoherent-version-in-changelog 0.13-1 ['0.13-1.orc6', '0.13-1.orc6']
dl.noarch: E: non-standard-dir-perm /var/spool/dl 0700L
1 packages and 0 specfiles checked; 1 errors, 1 warnings.


The changelog version issue is probably a local issue as we carry a custom %dist tag

a '700' does not seem 'non-standard' to me
http://www.redhat.com/archives/fedora-packaging/2008-September/msg00035.html
https://lintian.debian.org/tags/non-standard-dir-perm.html

there are potentially private matter in the existing case, so constraining access seems reasonable, and so a valid exception in the context of the program's services

so: rpmlint PASS

naming is acceptable (albeit short, but that matches upstream)

so: naming PASS

spec file name matches package name

so: specfile name PASS

license contained in COPYING

GPLv2+

but noted as: 
License: GPLv2 in the .spec file

Greg, could you please update this?

License is GPLv2+ according to AUTHORS.rst

Spec URL: https://gbailey.fedorapeople.org/dl/0.13-2/dl.spec
SRPM URL: https://gbailey.fedorapeople.org/dl/0.13-2/dl-0.13-2.fc20.src.rpm

Unbundle system provided php-phpass and php-php-gettext

rpmlint now shows:

Checking: dl-0.13-3.fc20.noarch.rpm
          dl-0.13-3.fc20.src.rpm
dl.noarch: W: dangling-symlink /usr/share/dl/include/gettext /usr/share/php/gettext
dl.noarch: W: dangling-symlink /usr/share/dl/include/PasswordHash.php /usr/share/php/phpass/PasswordHash.php
dl.noarch: E: non-standard-dir-perm /var/spool/dl 0700L
2 packages and 0 specfiles checked; 1 errors, 2 warnings.

I think the 2 new dangling symlink warnings can be ignored, as these symlinks point to the system provided (and required) php-phpass and php-php-gettext files.

Spec URL: https://gbailey.fedorapeople.org/dl/0.13-3/dl.spec
SRPM URL: https://gbailey.fedorapeople.org/dl/0.13-3/dl-0.13-3.fc20.src.rpm

as to comment 8, (symlinks) manual 'Requires' for php-phpass and php-php-gettext, to ensure they are present may be sufficient -- and I note the .spec file contains such

ok as to this .. perhaps it could be a '%ghost' type member in the %files section

see example: http://fedoraproject.org/wiki/PackagingDrafts/Logfiles , 
http://www.rpm.org/max-rpm-snapshot/s1-rpm-inside-files-list-directives.html#S3-RPM-INSIDE-FLIST-GHOST-DIRECTIVE

just a thought

[herrold@centos-6 dl]$ diff -u dl.spec-3 dl.spec-4
--- dl.spec-3   2014-08-01 10:51:41.452383001 -0400
+++ dl.spec-4   2014-08-01 10:50:28.542383001 -0400
@@ -5,7 +5,7 @@
 Name:       dl
 Version:    0.13
 Group:      Applications/Internet
-Release:    3%{?dist}
+Release:    4%{?dist}
 License:    GPLv2+

 Source0:    http://www.thregr.org/~wavexx/software/dl/releases/dl-%{version}.zip
@@ -114,8 +114,12 @@
 %{_datadir}/dl
 %dir %attr(0700,apache,apache) %{_localstatedir}/spool/dl
 %dir %attr(0755,apache,apache) %{_localstatedir}/spool/dl/data
+%ghost %{_datadir}/dl/include/gettext

 %changelog
+* Fri Aug  1 2014 R P Herrold <info> - 0.13-4
+- % ghost gettext link
+
 * Fri Aug  1 2014 Greg Bailey <gbailey> - 0.13-3
 - Unbundle php-phpass and php-php-gettext

[herrold@centos-6 dl]$

not that this works ... wonder how to fix this

[herrold@centos-6 dl]$ rpmlint -V ; rpmlint /home/herrold/rpmbuild/SRPMS/dl-0.13-4.orc6.src.rpm /home/herrold/rpmbuild/RPMS/noarch/dl-0.13-4.orc6.noarch.rpm
rpmlint version 1.5 Copyright (C) 1999-2007 Frederic Lepied, Mandriva
dl.noarch: W: dangling-symlink /usr/share/dl/include/gettext /usr/share/php/gettext
dl.noarch: W: dangling-symlink /usr/share/dl/include/PasswordHash.php /usr/share/php/phpass/PasswordHash.php
dl.noarch: E: non-standard-dir-perm /var/spool/dl 0700L
2 packages and 0 specfiles checked; 1 errors, 2 warnings.
[herrold@centos-6 dl]$


Greg -- I also see a warning you do not get ---  is your rpmlint current?

next review item: License file in %doc

%doc COPYING

PASS

6 Spec file in American English

sight reviewed -- 

PASS

7. 'legible' spec file

sight reviewed -- nice and clean

PASS

sha256 verfication

no upstream sums, signed available

6ee2813b39b038624c7c9ae23d4adb62a2d4383dbe71594fdcdf407fe6fa37b7  dl-0.13.zip

inspected:
   http://www.thregr.org/~wavexx/software/dl/releases/
and webpage per URL

PASS

attaching: README.dl-0.13.zip.txt.asc which I clearsign

(In reply to R P Herrold from comment #11)
> not that this works ... wonder how to fix this
> 

My opinion is that this is a valid warning to consider, which we did, so we can move on...

> Greg -- I also see a warning you do not get ---  is your rpmlint current?

[gbailey@localhost ~]$ rpm -q rpmlint
rpmlint-1.5-12.fc20.noarch

I see the same results as you do: 2 errors and 1 warning (comparing comment #8 and comment #11)

Created attachment 923333 [details]
clearsigned sha256sun of the sources

9. SRPM must build a binary

PASS (we were, after all able to run rpmlint against it

10. notation as to non-working arches

this yields a noarch file, so not applicable

PASS

11. call out all BR's save standard build environment members

no unusual requirements

PASS

12. locale handling

[herrold@centos-6 dl]$ grep locale dl.spec-3                                    
[herrold@centos-6 dl]$ rpm -qlp /home/herrold/rpmbuild/RPMS/noarch/dl-0.13-3.orc6.noarch.rpm | grep locale                                                      
/usr/share/dl/include/locale                                                    
/usr/share/dl/include/locale/cs_CZ                                              
/usr/share/dl/include/locale/cs_CZ/LC_MESSAGES                                  
/usr/share/dl/include/locale/cs_CZ/LC_MESSAGES/messages.mo                      
/usr/share/dl/include/locale/cs_CZ/LC_MESSAGES/messages.po                      
/usr/share/dl/include/locale/de_DE                                              
/usr/share/dl/include/locale/de_DE/LC_MESSAGES                                  
/usr/share/dl/include/locale/de_DE/LC_MESSAGES/messages.mo                      
/usr/share/dl/include/locale/de_DE/LC_MESSAGES/messages.po                      
/usr/share/dl/include/locale/es_ES                                              
/usr/share/dl/include/locale/es_ES/LC_MESSAGES                                  
/usr/share/dl/include/locale/es_ES/LC_MESSAGES/messages.mo                      
/usr/share/dl/include/locale/es_ES/LC_MESSAGES/messages.po
/usr/share/dl/include/locale/fr_FR
/usr/share/dl/include/locale/fr_FR/LC_MESSAGES
/usr/share/dl/include/locale/fr_FR/LC_MESSAGES/messages.mo
/usr/share/dl/include/locale/fr_FR/LC_MESSAGES/messages.po
/usr/share/dl/include/locale/it_IT
/usr/share/dl/include/locale/it_IT/LC_MESSAGES
/usr/share/dl/include/locale/it_IT/LC_MESSAGES/messages.mo
/usr/share/dl/include/locale/it_IT/LC_MESSAGES/messages.po
/usr/share/dl/include/locale/pt_BR
/usr/share/dl/include/locale/pt_BR/LC_MESSAGES
/usr/share/dl/include/locale/pt_BR/LC_MESSAGES/messages.mo
/usr/share/dl/include/locale/pt_BR/LC_MESSAGES/messages.po
/usr/share/doc/dl-0.13/client/thunderbird-filelink-dl/chrome/locale
/usr/share/doc/dl-0.13/client/thunderbird-filelink-dl/chrome/locale/en-US
/usr/share/doc/dl-0.13/client/thunderbird-filelink-dl/chrome/locale/en-US/compose.dtd
/usr/share/doc/dl-0.13/client/thunderbird-filelink-dl/chrome/locale/en-US/compose.properties
/usr/share/doc/dl-0.13/client/thunderbird-filelink-dl/chrome/locale/en-US/management.dtd
/usr/share/doc/dl-0.13/client/thunderbird-filelink-dl/chrome/locale/en-US/settings.dtd
[herrold@centos-6 dl]$


so seems to comply with the requirement

PASS

13. library handling

[herrold@centos-6 dl]$ rpm -qlp /home/herrold/rpmbuild/RPMS/noarch/dl-0.13-3.orc6.noarch.rpm | grep lib
[herrold@centos-6 dl]$


so not applicable

PASS

15. Reloactable / Prefix special casing

[herrold@centos-6 dl]$ grep -i prefix dl.spec-3
[herrold@centos-6 dl]$


not applicable

PASS

16. no bundled system libraries

no libraries at all 

also php module was unbundled earlier

PASS

17. if relocatable, justify special handling

as above - -not applicable 

PASS

ACTION ITEM

18. A package must own all directories that it creates

[herrold@centos-6 dl]$ grep -i mkdir dl.spec-3
mkdir -p ${RPM_BUILD_ROOT}%{_datadir}/dl
mkdir -p ${RPM_BUILD_ROOT}%{_sysconfdir}/dl
mkdir -p ${RPM_BUILD_ROOT}%{_sysconfdir}/httpd/conf.d
mkdir -p ${RPM_BUILD_ROOT}%{_localstatedir}/spool/dl
mkdir -p ${RPM_BUILD_ROOT}%{_localstatedir}/spool/dl/data
[herrold@centos-6 dl]$ grep -i install dl.spec-3
dl is usually installed as a "email attachments replacement" due to its
%install
# selinux: cleanup after uninstall
[herrold@centos-6 dl]$


Greg -- it seems that a:
Requires: httpd 
   ... or 'webserver'
and removal of that mkdir for %{_sysconfdir}/httpd/conf.d

is mandated here ...

https://fedoraproject.org/wiki/Packaging:Guidelines?rd=Packaging/Guidelines#FileAndDirectoryOwnership

(In reply to R P Herrold from comment #26)
> ACTION ITEM
> 
> 18. A package must own all directories that it creates
> 
> Greg -- it seems that a:
> Requires: httpd 
>    ... or 'webserver'
> and removal of that mkdir for %{_sysconfdir}/httpd/conf.d
> 
> is mandated here ...

There's already a "Requires: webserver" line in the spec file.

The directory "/etc/httpd/conf.d" is owned by the httpd package, so I believe that satisfies the requirement of not having unowned directories.

fair enough -- 18. A package must own

PASS

MUST: If the package is designed to be relocatable, the packager must state this fact in the request for review, along with the 

rpnbuild flags these, and is not doing so here, so

PASS

20. Permissions on files must be set properly

rpmlint flags deviations, -- we have the one known and excepted deviation for privacy purposes

PASS

21. consistently use packaging macros

spec file sight reviewed and clean per prior steps

no variations from consistency noted

PASS

22. The package must contain code, or permissable content

'permissable' so spelled in the wiki -- hmmm

it does -- this is a CGI script add on, well licensed for software freedom

PASS

typo reported for fix

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

23. Large documentation files must go in a -doc subpackage

inapplicable

PASS

24. %doc content must not affect the runtime

it does not

PASS

QUERY

actually as to # 24 it would appear that sub-packages for the CLI client, and for Thunderbird are appropriate

/usr/share/doc/dl-0.13/client/dl-cli.py                                         
/usr/share/doc/dl-0.13/client/dl-wx/dl-cli.py                                   
/usr/share/doc/dl-0.13/client/dl-wx/dl-icon.ico                                 
/usr/share/doc/dl-0.13/client/dl-wx/dl-wx.py                                    
/usr/share/doc/dl-0.13/client/dl-wx/dl.py                                       
/usr/share/doc/dl-0.13/client/dl-wx/newticket.xrc                               
/usr/share/doc/dl-0.13/client/dl-wx/preferences.xrc                             
/usr/share/doc/dl-0.13/client/dl-wx/upload.xrc   

Greg -- what think you here?

-- Russ

(In reply to R P Herrold from comment #36)
> QUERY
> 
> actually as to # 24 it would appear that sub-packages for the CLI client,
> and for Thunderbird are appropriate
> 
> Greg -- what think you here?

I considered it.  My reasons for leaving those clients as documentation for now were that:

- I don't (yet) have much experience with them, how they're configured, etc., and didn't want that to delay getting the primary web application (which I suspect is what most people use) packaged.

- The primary target I'm personally interested in is RHEL 6 / CentOS 6 (EPEL), and the documentation (README.rst) states that those clients require Python 2.7, which would force me to go down the road of using SCL, which I'd like to avoid.

I think we could add subpackages for the CLI, the wx-based client, and the thunderbird addon in future revisions if there's sufficient interest.

as to splitting out; * nod *

The alternative answer would be to add a packaging conditional and skip around the sub-packages when 2.7 is not in the 'stock' environment

I see this, reviewing me .spec file collection

https://fedoraproject.org/wiki/Features/Python_2.7/MassRebuild

# Python major version.
%{expand: %%define pyver %(python -c 'import sys;print(sys.version[0:3])')}

also seen: 
%define pyver %(python -c 'import sys ; print sys.version[:3]')

%global python_ver %(%{__python} -c "import sys ; print sys.version[:3]")                                                                 

we can in all cases on RH derived assume the presence of a python in the base install of course, so it should be reasonably straightforward to stub in that code to conditionally add the python 2.7 stuff

I annex a patch to this effect for the first part

Created attachment 923401 [details]
patch, adding a conditional CLI sub-package

per prior comment

as to 24 (subpackages), either way, the present packaging is fine and so:

PASS

25. Static libraries must be in a -static package

not applicable

PASS

26. Development files must be in a -devel package

not applicable

PASS

27. devel packages must require the base package using a fully versioned
dependency

Not applicable here

that said, the proposed patch for the -cli subpackage also carries this

PASS

Packages must NOT contain any .la libtool archives

not applicable as a noarch package

PASS

29. GUI applications must include a %{name}.desktop file

presently not applicable; might become so if / when the wx or Thinderbird packages are added

PASS

30. Packages must not own files or directories already owned by other packages

seems to be fine

PASS

31. All filenames in rpm packages must be valid UTF-8

seems to be unexceptional and fine

PASS

At this point, all the MUST items on the checklist page seems to be satisfied.  I am not formally 'approved' in F circles to do reviews, but will ping for a sponsor to look this over.  I sort of fergit the ornate process and so had not fought my way through it

(In reply to R P Herrold from comment #48)
> At this point, all the MUST items on the checklist page seems to be
> satisfied.  I am not formally 'approved' in F circles to do reviews, but
> will ping for a sponsor to look this over.  I sort of fergit the ornate
> process and so had not fought my way through it

Thanks for your informal review and feedback, Russ.  Much appreciated!

abadger1999 was kind enough to sponsor me into 'packager' so I think I can approve this now

now to figure out how to DO that in fact

flip on the satisfactory review flag

per
https://fedoraproject.org/wiki/Package_SCM_admin_requests

requesting VCS intervention

New Package SCM Request
=======================
Package Name: dl
Short Description: Download Ticket Service
Upstream URL: http://www.thregr.org/~wavexx/software/dl/
Owners: gbailey herrold
Branches: f19 f20 f21 el6 epel7
InitialCC: gbailey herrold

thank you for the co-maintainer addition -- I will probably poke at the other two sub-packages once it gets into the stream

Git done (by process-git-requests).

> 31. All filenames in rpm packages must be valid UTF-8
> 
> seems to be unexceptional and fine
> 
> PASS

Please, if possible, consider putting all the checks in one comment to lower the volume of bugzilla generated mails next time.  Thank you :)

CAN YOU PUT ALL YOUR WORDS IN ONE COMMENT?

clear the flag