Bug 627032

I'll do an unofficial review.

This is an unofficial review. You'll need someone else to do the official review for you.

+ = OK
- = NA
? = issue

+ Package meets naming and packaging guidelines
+ Spec file matches base package name.
+ Spec has consistant macro usage.
? Meets Packaging Guidelines.
+ License
+ License field in spec matches
? License file included in package
+ Spec in American English
+ Spec is legible.
- Sources match upstream md5sum: 
    Upstream does not provide md5 sum or any other hash for the tar.gz file. Please report a bug in the upstream and ask them to add an md5sum.
- Package needs ExcludeArch
+ BuildRequires correct
- Spec handles locales/find_lang
- Package is relocatable and has a reason to be.
+ Package has %defattr and permissions on files is good.
+ Package has a correct %clean section.
+ Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
+ Package is code or permissible content.
- Doc subpackage needed/used.
- Packages %doc files don't affect runtime.

- Headers/static libs in -devel subpackage.
- Spec has needed ldconfig in post and postun
- .pc files in -devel subpackage/requires pkgconfig
- .so files in -devel subpackage.
- -devel package Requires: %{name} = %{version}-%{release}
- .la files are removed.

- Package is a GUI app and has a .desktop file

+ Package compiles and builds on at least one arch.
+ Package has no duplicate files in %files.
+ Package doesn't own any directories other packages own.
- Package owns all the directories it creates.
? No rpmlint output.
Please fix the following errors:

w3c-linkchecker.src: W: spelling-error %description -l en_US dereferenceable -> teleconference
w3c-linkchecker.src: E: unknown-key (MD5
w3c-linkchecker.src: W: invalid-url Source0: http://www.cpan.org/authors/id/S/SC/SCOP/W3C-LinkChecker-4.6.tar.gz HTTP Error 404: Not Found
1 packages and 0 specfiles checked; 1 errors, 2 warnings.


+ final provides and requires are sane.

SHOULD Items:

- Should build in mock.
- Should build on all supported archs
- Should function as described.
- Should have sane scriptlets.
- Should have subpackages require base package with fully versioned depend.
+ Should have dist tag
? Should package latest version
- check for outstanding bugs on package. (For core merge reviews)

Issues:

1. Please ask upstream to include the license in the tarball.
2. Upstream does not provide md5 sum or any other hash for the tar.gz file. Please report a bug in the upstream and ask them to add an md5sum.
3. rpmlint couldn't find the Source URL because it is no longer avilable in upstream. Please update the package to the latest version.
4. Do not place files in /var/www. Please read: https://fedoraproject.org/wiki/Packaging:Guidelines#Web_Applications

Warnings:
1. Clean section is not required for Fedora 13 or above.
2. I'm not sure about it, but I think the upstream URL should be http://search.cpan.org/dist/W3C-LinkChecker/
   Read https://fedoraproject.org/wiki/Packaging:Perl#URL_tag for more info.

http://scop.fedorapeople.org/packages/w3c-linkchecker.spec
http://scop.fedorapeople.org/packages/w3c-linkchecker-4.7-1.fc16.src.rpm

* Mon Mar 28 2011 Ville Skyttä <ville.skytta> - 4.7-1
- Update to 4.7, update dependencies.
- Remove spec file constructs no longer needed in F-14+ and EL-6+.

(In reply to comment #2)

> w3c-linkchecker.src: W: spelling-error %description -l en_US dereferenceable ->
> teleconference

Bogus message from the spell checker, won't fix.

> w3c-linkchecker.src: E: unknown-key (MD5

Not an issue, it's just signed with my key which isn't known to your system (and the package will not include my signature once it's in Fedora).  Anyway, the 4.7-1 srpm is not signed.

> w3c-linkchecker.src: W: invalid-url Source0:
> http://www.cpan.org/authors/id/S/SC/SCOP/W3C-LinkChecker-4.6.tar.gz HTTP Error
> 404: Not Found

Updated to 4.7.

> 1. Please ask upstream to include the license in the tarball.

Already done a long time ago (for w3c-markup-validator but the copyright holder is the same), no reply received:
http://lists.w3.org/Archives/Public/public-qa-dev/2010Jul/0004.html

> 2. Upstream does not provide md5 sum or any other hash for the tar.gz file.
> Please report a bug in the upstream and ask them to add an md5sum.

I'm curious, where does this requirement come from?  Anyway, won't fix, this is intentional, and note also that I'm pretty much the upstream for this package.  If you wish, the expanded tarball contents can be verified with "cpansign" (from the perl-Module-Signature package).

> 3. rpmlint couldn't find the Source URL because it is no longer avilable in
> upstream. Please update the package to the latest version.

Done.

> 4. Do not place files in /var/www. Please read:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Web_Applications

This would mean that I'd have to package configuration files for all web servers this could be run with, whereas I believe most of them execute CGI scripts from /var/www/cgi-bin out of the box and there's no other location that would provide the same functionality without additional configuration.  This would get too messy for my taste (config file maintenance, duplicate config dir ownership or dependency or subpackage hell, note that this can also be run without a web server) - if this is a requirement for the package to pass review, I will most likely cancel the submission.

> 1. Clean section is not required for Fedora 13 or above.

Removed stuff that's not required for F-14+ and EL-6+.  Usually I leave those in intentionally so that packages work unmodified for example on EL-5, but this one has dependencies to newer versions of things than EL-5 has so it won't work anyway.

> 2. I'm not sure about it, but I think the upstream URL should be
> http://search.cpan.org/dist/W3C-LinkChecker/
>    Read https://fedoraproject.org/wiki/Packaging:Perl#URL_tag for more info.

Not changed, what's in the current package is its official home page URL, and even though this is distributed from CPAN, it's not a usual perl module package.

----

Actually, when I think about it now, I will post to fedora-devel to see if someone else would be interested in finishing the review and be the maintainer.  I haven't been actually using the packaged version of this in a while (I use the online service or my local bleeding edge setup for developing it), and that doesn't make me a suitable package manintainer for the package in Fedora.  Unless someone picks it up in a few weeks, I'll cancel the submission.

(In reply to comment #3)
> http://scop.fedorapeople.org/packages/w3c-linkchecker.spec
> http://scop.fedorapeople.org/packages/w3c-linkchecker-4.7-1.fc16.src.rpm
> 
> * Mon Mar 28 2011 Ville Skyttä <ville.skytta> - 4.7-1
> - Update to 4.7, update dependencies.
> - Remove spec file constructs no longer needed in F-14+ and EL-6+.
> 
> (In reply to comment #2)
> 
> > w3c-linkchecker.src: W: spelling-error %description -l en_US dereferenceable ->
> > teleconference
> 
> Bogus message from the spell checker, won't fix.
Ok.
> 
> > w3c-linkchecker.src: E: unknown-key (MD5
> 
> Not an issue, it's just signed with my key which isn't known to your system
> (and the package will not include my signature once it's in Fedora).  Anyway,
> the 4.7-1 srpm is not signed.
Ok, but review request rpms are usually not signed.
> 
> > w3c-linkchecker.src: W: invalid-url Source0:
> > http://www.cpan.org/authors/id/S/SC/SCOP/W3C-LinkChecker-4.6.tar.gz HTTP Error
> > 404: Not Found
> 
> Updated to 4.7.
> 
> > 1. Please ask upstream to include the license in the tarball.
> 
> Already done a long time ago (for w3c-markup-validator but the copyright holder
> is the same), no reply received:
> http://lists.w3.org/Archives/Public/public-qa-dev/2010Jul/0004.html
> 
> > 2. Upstream does not provide md5 sum or any other hash for the tar.gz file.
> > Please report a bug in the upstream and ask them to add an md5sum.
> 
> I'm curious, where does this requirement come from?  Anyway, won't fix, this is
> intentional, and note also that I'm pretty much the upstream for this package. 
> If you wish, the expanded tarball contents can be verified with "cpansign"
> (from the perl-Module-Signature package).
> 
I'm not sure if it is a requirement, but it is recommended. The person that will do the official review will probably give you a better answer.
> > 3. rpmlint couldn't find the Source URL because it is no longer avilable in
> > upstream. Please update the package to the latest version.
> 
> Done.
> 
> > 4. Do not place files in /var/www. Please read:
> > https://fedoraproject.org/wiki/Packaging:Guidelines#Web_Applications
> 
> This would mean that I'd have to package configuration files for all web
> servers this could be run with, whereas I believe most of them execute CGI
> scripts from /var/www/cgi-bin out of the box and there's no other location that
> would provide the same functionality without additional configuration.  This
> would get too messy for my taste (config file maintenance, duplicate config dir
> ownership or dependency or subpackage hell, note that this can also be run
> without a web server) - if this is a requirement for the package to pass
> review, I will most likely cancel the submission.
This IS required, sorry. You won't need configuration for every web server, you can select just few common ones or put a simple file somewhere in /usr/share and write in a readme file or a manual that it should be copied to the appropriate directory. (and i'm sure there are other solution for this issue)
> 
> > 1. Clean section is not required for Fedora 13 or above.
> 
> Removed stuff that's not required for F-14+ and EL-6+.  Usually I leave those
> in intentionally so that packages work unmodified for example on EL-5, but this
> one has dependencies to newer versions of things than EL-5 has so it won't work
> anyway.
> 
> > 2. I'm not sure about it, but I think the upstream URL should be
> > http://search.cpan.org/dist/W3C-LinkChecker/
> >    Read https://fedoraproject.org/wiki/Packaging:Perl#URL_tag for more info.
> 
> Not changed, what's in the current package is its official home page URL, and
> even though this is distributed from CPAN, it's not a usual perl module
> package.
> 
I think it's ok then.
> ----
> 
> Actually, when I think about it now, I will post to fedora-devel to see if
> someone else would be interested in finishing the review and be the maintainer.
>  I haven't been actually using the packaged version of this in a while (I use
> the online service or my local bleeding edge setup for developing it), and that
> doesn't make me a suitable package manintainer for the package in Fedora. 
> Unless someone picks it up in a few weeks, I'll cancel the submission.
I'm sorry to hear that. Hope you'll find someone else to maintain it.

(In reply to comment #4)

(regarding upstream md5sum etc)
> I'm not sure if it is a requirement, but it is recommended.

Where is it recommended?  This is the first time I've heard of such a recommendation.

(regarding placing files in /var/www)
> This IS required, sorry.

Note that the packaging guidelines say "should", not "MUST".

> You won't need configuration for every web server, you
> can select just few common ones

...and if those config files are installed in proper locations, there either
a) Needs to be a dependency on the servers that provide those dirs which is a no go because the software will run on command line without any web server, or
b) The config files need to be in web server specific subpackages that have the dependencies which is IMO quite ugly, or
c) This package would need to own those dirs, which is also quite ugly.

> or put a simple file somewhere in /usr/share
> and write in a readme file or a manual that it should be copied to the
> appropriate directory.

That's not what I personally would expect of a properly packaged application.

So given the options above, my opinion remains that simply placing the script in /var/www/cgi-bin is a superior approach, even if it's a "should not" per the packaging guidelines.

> I'm sorry to hear that. Hope you'll find someone else to maintain it.

You wouldn't happen to be interested, would you?

(In reply to comment #5)
> (In reply to comment #4)
> 
> (regarding upstream md5sum etc)
> > I'm not sure if it is a requirement, but it is recommended.
> 
> Where is it recommended?  This is the first time I've heard of such a
> recommendation.
> 
> (regarding placing files in /var/www)
> > This IS required, sorry.
> 
> Note that the packaging guidelines say "should", not "MUST".
The "not" is in capital letters, which made me think it is important. I'll ask more experienced developers about it. 
> 
> > You won't need configuration for every web server, you
> > can select just few common ones
> 
> ...and if those config files are installed in proper locations, there either
> a) Needs to be a dependency on the servers that provide those dirs which is a
> no go because the software will run on command line without any web server, or
You can make sub-packages in your package, eg. w3c-linkchecker-apache that will supply the configuration files for apache, and do so for every sever you like (one is enough, if you don't want too many subpackages), and the main package would only provide the command line application.
> b) The config files need to be in web server specific subpackages that have the
> dependencies which is IMO quite ugly, or
> c) This package would need to own those dirs, which is also quite ugly.
> 
C is not an option, it is against the guidelines.
> > or put a simple file somewhere in /usr/share
> > and write in a readme file or a manual that it should be copied to the
> > appropriate directory.
> 
> That's not what I personally would expect of a properly packaged application.
> 
That's was just an idea.
> So given the options above, my opinion remains that simply placing the script
> in /var/www/cgi-bin is a superior approach, even if it's a "should not" per the
> packaging guidelines.
> 
> > I'm sorry to hear that. Hope you'll find someone else to maintain it.
> 
> You wouldn't happen to be interested, would you?
As I'm not a packager yet (I'm doing unofficial reviews to prove that I am "worthy"), I don't know how to take this review request. I'll try to ask someone in #fedora-devel, to see if (and how) I can do that.

MD5 is not required. Sorry, my mistake.

Also I have been told that if the guidelines says should, an exception can be made, but it should be avoided because of the reasons listed in the guidelines.

(In reply to comment #6)
> You can make sub-packages in your package, eg. w3c-linkchecker-apache that will
> supply the configuration files for apache, and do so for every sever you like
> (one is enough, if you don't want too many subpackages), and the main package
> would only provide the command line application.

Yes, that describes the b) option I mentioned, and my opinion about it is unchanged.

> C is not an option, it is against the guidelines.

No it's not, it's standard packaging practice in certain situations.  See "multiple ownership" and "The directory is owned by a package which is not required for your package to function" at
http://fedoraproject.org/wiki/Packaging/Guidelines#File_and_Directory_Ownership

Withdrawing review request due to apparent lack of interest.  If someone wants to pick it up later, the SRPM is available at http://cachalot.mine.nu/unmaintained/