Bug 438811 (php-pear-HTML-AJAX)

Summary: Review Request: php-pear-HTML-AJAX - PHP and JavaScript AJAX library
Product: [Fedora] Fedora Reporter: David Hollis <dhollis>
Component: Package ReviewAssignee: Remi Collet <fedora>
Status: CLOSED WONTFIX QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: chris.stone, fedora, fedora-package-review, itamar, mtasaka, notting, shamardin, susi.lehtola
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-03-01 08:23:50 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description David Hollis 2008-03-25 12:55:01 UTC
Spec URL: http://web.davehollis.com:81/packages/php-pear-HTML-AJAX.spec
SRPM URL: http://web.davehollis.com:81/packages/php-pear-HTML-AJAX-0.5.2-1.fc8.src.rpm
Description: 

Provides PHP and JavaScript libraries for performing AJAX (Communication
from JavaScript to your browser without reloading the page)

Comment 1 Remi Collet 2008-08-16 09:50:25 UTC
Please fix the %files

Comment 2 Lev Shamardin 2008-11-22 12:16:01 UTC
I'm not a sponsor, so this is not a formal review, but I hope this can
speed things up a bit.

- FAILED: rpmlint complaints:

php-pear-HTML-AJAX.spec: W: mixed-use-of-spaces-and-tabs (spaces: line 4, tab: line 50)

You shoud use spaces for indentation. Just expand all tabs.

php-pear-HTML-AJAX.noarch: W: file-not-utf8 /usr/share/doc/php-pear-HTML-AJAX-0.5.2/examples/proxy_usage_server.php
php-pear-HTML-AJAX.noarch: W: file-not-utf8 /usr/share/doc/php-pear-HTML-AJAX-0.5.2/examples/support/test.class.php

I see that you've tried to fix this issue in the %install script, but
you are iconv'ing wrong files. You should iconv files in docdir/%{pear_name}/examples/

- PASSED: The package is named according to the Package Naming
  Guidelines.
- PASSED: Spec file name is ok.
- FAILED: The package dos not meet the Packaging Guidelines for PHP.

Please check with https://fedoraproject.org/wiki/Packaging/PHP

A PEAR package MUST have:

BuildRequires: php-pear(PEAR)
Requires: php-pear(PEAR)
Requires(post): %{__pear}
Requires(postun): %{__pear}
Provides:     php-pear(foo) = %{version}

post-scriptlets are fine.

- PASSED: The license is fedora-compatible.
- FAILED: The license specified on the project homepage is LGPL, the
  license specified in spec file is LGPLv3.
- PASSED: The spec file is written in American English in legible.
- PASSED: The sources match the upstream source (md5 verified).
- PASSED: The package may be successfully built.
- FAILED: Not all build build dependencies are listed in BuildRequires.
You should add at least dos2unix.
- FAILED: The package does not build in mock due to missing BuildRequires.
- PASSED: A package does not contain any duplicate files in the %files listing.
- PASSED: Permissions are sane.
- PASSED: %clean section is fine.
- PASSED: macros usage is fine.
- PASSED: The package must contains code.
- PASSED: The package does not contain large documentation.
- PASSED: %doc contents is fine.
- FAILED: This package owns directory /usr/share/pear/HTML. It is
  already owned by php-pear-HTML-Common, so this package must Require
  it, and own only it's own subdirectory in /usr/share/pear/HTML.
- PASSED: There is rm -rf $RPM_BUILD_ROOT in the beginning of %install.
- PASSED: All filenames in rpm package are valid UTF-8.

Some other general notes:

You are packaging version 0.5.2 of the package which is a beta. There
is a newer upstream beta available. Could you please justify why are
you packaging an older release, or please upgrade to the newer
upstream version.

Comment 3 Remi Collet 2008-11-23 08:20:58 UTC
I must disagree with Lev.

As this package run without HTML_Common (check all the "require_once" in the scripts), you don't have to "Requires" it. So you must also own the /usr/share/pear/HTML directory.

But %file must be cleared

Comment 4 Lev Shamardin 2008-11-23 08:34:39 UTC
Agreed, I forgot to check the requires in the sorce code. BTW, may be this (https://fedoraproject.org/wiki/PackagingDrafts/DirectoryOwnershipImprovement) is already should be included into the guidelines?

Comment 5 Remi Collet 2008-11-23 08:54:10 UTC
See http://fedoraproject.org/wiki/Packaging/Guidelines#FileAndDirectoryOwnership
See 2 (Animal example).

++

Comment 6 Mamoru TASAKA 2008-11-23 16:49:10 UTC
Then why does this package have to install files under %_datadir/pear/HTML ??

By the way currently on koji-11:
# repoquery --repoid=koji-11 --whatprovides '/usr/share/pear/HTML/*'
php-pear-HTML-QuickForm-advmultiselect-0:1.4.1-1.fc10.noarch
php-pear-HTML-Common-0:1.2.4-2.fc10.noarch
php-pear-HTML-QuickForm-0:3.2.10-1.fc9.noarch
php-pear-HTML-Table-0:1.8.2-1.fc9.noarch
php-pear-HTML-QuickForm-ElementGrid-0:0.1.1-2.fc10.noarch

which all Requires php-pear-HTML-Common directly or indirectly.

Comment 7 Christopher Stone 2008-11-23 17:04:20 UTC
I would recommend removing dos2unix entirely, and just sed which is part of the core build environment.

Untested:
find docdir -type f -exec sed -i 's/\r//' {} \;

Comment 8 Remi Collet 2008-11-23 17:15:14 UTC
In PEAR extension, the tree is a thematic classification (not a dependency one)
It's an upstream choice that packager cannot change.

Requires must be taken from the PEAR documentation or (best) from the source

In QuickForm.php => require_once 'HTML/Common.php';
So php-pear-HTML-QuickForm must requires php-pear-HTML-Common

In Table.php => require_once 'HTML/Common.php';
So php-pear-HTML-Table must requires php-pear-HTML-Common

In all the script from HTML_AJAX, there is no require/include outside of HTML/AJAX. So no need to require php-pear-HTML-Common

++

Comment 9 Remi Collet 2009-03-01 08:23:50 UTC
No reponse, I close this bug to allow anynody else to submit this package again.