Bug 734248 - Review Request: apf - Adventure PHP Framework
Summary: Review Request: apf - Adventure PHP Framework
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Orion Poplawski
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-08-29 21:28 UTC by Reiner Rottmann
Modified: 2016-11-24 04:48 UTC (History)
5 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-11-24 04:48:35 UTC
Type: ---


Attachments (Terms of Use)

Description Reiner Rottmann 2011-08-29 21:28:14 UTC
Spec URL:
http://www.rottmann.it/apf/review/apf.spec
http://www.rottmann.it/apf/review/apf-doc.spec
SRPM URL:
http://www.rottmann.it/apf/review/apf-1.13-1.fc15.src.rpm
http://www.rottmann.it/apf/review/apf-doc-1.13-1.fc15.src.rpm
Description:

I've just created my first rpm for submission to Fedora Extra. And I would appreciate a review to get some feedback and hints how I may improve my packaging.

I have selected the Adventure PHP Framework because this project has been started by a good friend of mine and so I'm double motivated in maintaining it.

The adventure php framework (http://adventure-php-framework.org) understands itself as a utility to implement object oriented and generic PHP web applications. It supports the developer to create programs in compliance with approved software design pattern and the code base already has answers to many everyday problems. The framework cannot be described as an application, that only has to be configured, but rather as a technical basis and design guide for the design of software.

The apf-1.13-1.fc15.src.rpm comes with an example implementation so that the user may start using the framework right away.

The apf-doc-1.13-1.fc15.src.rpm includes the complete API documentation.

Comment 1 Reiner Rottmann 2011-08-29 21:33:36 UTC
FE-NEEDSPONSOR

Comment 2 Reiner Rottmann 2011-08-29 22:18:05 UTC
[rottmrei@localhost SRPMS]$ rpmlint -i apf-1.13-1.fc15.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

[rottmrei@localhost noarch]$ rpmlint -i apf-1.13-1.fc15.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

[rottmrei@localhost SRPMS]$ rpmlint -i apf-doc-1.13-1.fc15.src.rpm 
apf-doc.src:32: W: macro-in-comment %{buildroot}
There is a unescaped macro after a shell style comment in the specfile. Macros
are expanded everywhere, so check if it can cause a problem in this case and
escape the macro with another leading % if appropriate.

apf-doc.src:32: W: macro-in-comment %{_localstatedir}
There is a unescaped macro after a shell style comment in the specfile. Macros
are expanded everywhere, so check if it can cause a problem in this case and
escape the macro with another leading % if appropriate.

apf-doc.src:32: W: macro-in-comment %{version}
There is a unescaped macro after a shell style comment in the specfile. Macros
are expanded everywhere, so check if it can cause a problem in this case and
escape the macro with another leading % if appropriate.

1 packages and 0 specfiles checked; 0 errors, 3 warnings.


[rottmrei@localhost noarch]$ rpmlint -i apf-doc-1.13-1.fc15.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.


[rottmrei@localhost SRPMS]$ mock apf-1.13-1.fc15.src.rpm
INFO: mock.py version 1.1.12 starting...
State Changed: init plugins
INFO: selinux enabled
State Changed: start
INFO: Start(apf-1.13-1.fc15.src.rpm)  Config(fedora-15-x86_64)
State Changed: lock buildroot
State Changed: clean
INFO: chroot (/var/lib/mock/fedora-15-x86_64) unlocked and deleted
State Changed: unlock buildroot
State Changed: init
State Changed: lock buildroot
Mock Version: 1.1.12
INFO: Mock Version: 1.1.12
INFO: enabled root cache
State Changed: unpacking root cache
INFO: enabled yum cache
State Changed: cleaning yum metadata
INFO: enabled ccache
State Changed: running yum
State Changed: unlock buildroot
State Changed: setup
State Changed: build
INFO: Done(apf-1.13-1.fc15.src.rpm) Config(default) 0 minutes 49 seconds
INFO: Results and/or logs in: /var/lib/mock/fedora-15-x86_64/result
State Changed: end


[rottmrei@localhost SRPMS]$ mock apf-doc-1.13-1.fc15.src.rpm 
INFO: mock.py version 1.1.12 starting...
State Changed: init plugins
INFO: selinux enabled
State Changed: start
INFO: Start(apf-doc-1.13-1.fc15.src.rpm)  Config(fedora-15-x86_64)
State Changed: lock buildroot
State Changed: clean
INFO: chroot (/var/lib/mock/fedora-15-x86_64) unlocked and deleted
State Changed: unlock buildroot
State Changed: init
State Changed: lock buildroot
Mock Version: 1.1.12
INFO: Mock Version: 1.1.12
INFO: enabled root cache
State Changed: unpacking root cache
INFO: enabled yum cache
State Changed: cleaning yum metadata
INFO: enabled ccache
State Changed: running yum
State Changed: unlock buildroot
State Changed: setup
State Changed: build
INFO: Done(apf-doc-1.13-1.fc15.src.rpm) Config(default) 1 minutes 57 seconds
INFO: Results and/or logs in: /var/lib/mock/fedora-15-x86_64/result
State Changed: end

Comment 3 Jason Tibbitts 2012-06-29 21:02:13 UTC
I'm just looking through some of the older NEEDSPONSOR tickets.

First point: one source package per review ticket, please.  If one package/ticket depends on another, you indicate that via the Depends On: and Blocks: fields.  Nobody will even start reviewing a ticket that doesn't meet basic rules like that, which may account for the reason nobody has looked at it until now.

Looking at just the apf package, the spec is a bit messy and has some definitely forbidden things (conflicting macro usage, output during scriptlets) and a few things that can be cleaned up (unnecessary buildroot stuff), but nothing that can't be easily fixed.

Are you still interested in submitting these packages?  PHP isn't really my strong suit and so I may attempt to get someone more familiar with the language involved, but only if you still wish to submit this.

Comment 4 Reiner Rottmann 2012-06-30 09:06:53 UTC
Thank you for having a look at the spec file and your greatly appreciated help.

I'm still interested in having the package included in Fedora. I want to learn about the overall process so that I may help in packaging other software, too.

Thanks for pointing out the one ticket per package rule. I will continue improving the apf.spec file and will create another ticket for the apf-doc documentation package.

Regarding the forbidden things:
I will remove the echo commands. What do you mean by conflicting macro usage?
With unnecessary buildroot stuff, you are refering to the mkdir commands, aren't you? I suppose you mean that the directories are already present because of the prerequisite rpm packages.

Comment 5 Reiner Rottmann 2012-06-30 14:37:08 UTC
I've updated the apf.spec file and created new rpm packages.

Spec URL:
http://www.rottmann.it/apf/review/apf.spec

SRPM URL:
http://www.rottmann.it/apf/review/apf-1.15-1.fc17.src.rpm

RPM URL:
http://www.rottmann.it/apf/review/apf-1.15-1.fc17.noarch.rpm

[rottmrei@fedora SPECS]$ rpmlint -i /home/rottmrei/rpmbuild/RPMS/noarch/apf-1.15-1.fc17.noarch.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[rottmrei@fedora SPECS]$ rpmlint -i /home/rottmrei/rpmbuild/SRPMS/apf-1.15-1.fc17.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[rottmrei@fedora SRPMS]$ mock apf-1.15-1.fc17.src.rpm 
INFO: mock.py version 1.1.22 starting...
State Changed: init plugins
INFO: selinux enabled
State Changed: start
INFO: Start(apf-1.15-1.fc17.src.rpm)  Config(fedora-17-x86_64)
State Changed: lock buildroot
State Changed: clean
INFO: chroot (/var/lib/mock/fedora-17-x86_64) unlocked and deleted
State Changed: unlock buildroot
State Changed: init
State Changed: lock buildroot
Mock Version: 1.1.22
INFO: Mock Version: 1.1.22
INFO: calling preinit hooks
INFO: enabled root cache
State Changed: unpacking root cache
INFO: enabled yum cache
State Changed: cleaning yum metadata
INFO: enabled ccache
State Changed: running yum
State Changed: unlock buildroot
INFO: Installed packages:
State Changed: setup
State Changed: build
INFO: Done(apf-1.15-1.fc17.src.rpm) Config(default) 0 minutes 41 seconds
INFO: Results and/or logs in: /var/lib/mock/fedora-17-x86_64/result
State Changed: end

Comment 6 Jason Tibbitts 2012-07-03 22:40:12 UTC
OK, let me take a quick look and make more specific comments.

This does build fine; I get a few rpmlint complaints:
  apf.noarch: W: wrong-file-end-of-line-encoding /var/www/apf-1.15/README_en.txt
  apf.noarch: W: wrong-file-end-of-line-encoding /var/www/apf-1.15/lgpl-3.0.txt
  apf.noarch: W: wrong-file-end-of-line-encoding /var/www/apf-1.15/gpl-3.0.txt
  apf.noarch: W: wrong-file-end-of-line-encoding /var/www/apf-1.15/README_de.txt
These all need to be cleaned up to remove DOS line endings.

Pretty much everything in this package is installed in the wrong place.  You cannot install into /var/www.  This is discussed explicitly in http://fedoraproject.org/wiki/Packaging:Guidelines#Web_Applications
/usr/share/apf (not versioned) or even a less generic /usr/share/adventure-framework would be a more reasonable place, I'd think.

BuildRoot, the first line of %install, the entire %clean section and the %defattr line in %files are completely unnecessary in Fedora and should be removed.

You don't need an empty %build section at all.

Packages should not depend on the base php package.  There's a draft about this here: http://fedoraproject.org/wiki/PackagingDrafts/PHP which should make its way into the actual guideline relatively soon.  Since this does include an apache conf file, depending on httpd is OK.

Why does this package require a local mysql server?  Wouldn't it work with a remote server?

Macro forms of basic comments like %{__rm} should not be used.  Just use "rm" instead; it's a whole lot less typing, and we're not going to remove rm from the path pretty much ever.

You call dos2unix without having any dependency on it.  I have no idea why it doesn't fail; maybe the find calls aren't working as you expect.

It's pretty suboptimal to do selinux setup in scriptlets.  It would be better, once a proper location for the files is chosen, to get the base selinux package to include the proper file contexts.  The selinux folks are very responsive.

You should use a proper systemd call to restart httpd, and include the proper dependencies for it, if indeed you really feel like doing that.  Personally I don't think that installing this package should mess with a running httpd, and the other webapps I checked don't do that.

There's no need at all to do this:
  %dir %{_localstatedir}/www/apf-%{version}/
  %{_localstatedir}/www/apf-%{version}/*
Instead, just use one line:
  %{_localstatedir}/www/apf-%{version}/

You shoud put the README files somewhere under docdir instead of spreading through all through the application directory, unless there's some specific reason they need to be where they are.

Comment 7 Reiner Rottmann 2012-07-04 21:23:25 UTC
First of all thank you very much for sharing all your knowledge about proper packaging for Fedora. This really helps me getting comfortable with the Fedora specifics.

I've updated the apf.spec file and created new rpm packages. Answers to your 
comment #6 may be found below.

Spec URL:
http://www.rottmann.it/apf/review/apf.spec

SRPM URL:
http://www.rottmann.it/apf/review/apf-1.15-2.fc17.src.rpm

RPM URL:
http://www.rottmann.it/apf/review/apf-1.15-2.fc17.noarch.rpm






MOCKBUILD:
[rottmrei@fedora SPECS]$ mock /home/rottmrei/rpmbuild/SRPMS/apf-1.15-2.fc17.src.rpm
INFO: mock.py version 1.1.22 starting...
State Changed: init plugins
INFO: selinux enabled
State Changed: start
INFO: Start(/home/rottmrei/rpmbuild/SRPMS/apf-1.15-2.fc17.src.rpm)  Config(fedora-17-x86_64)
State Changed: lock buildroot
State Changed: clean
INFO: chroot (/var/lib/mock/fedora-17-x86_64) unlocked and deleted
State Changed: unlock buildroot
State Changed: init
State Changed: lock buildroot
Mock Version: 1.1.22
INFO: Mock Version: 1.1.22
INFO: calling preinit hooks
INFO: enabled root cache
State Changed: unpacking root cache
INFO: enabled yum cache
State Changed: cleaning yum metadata
INFO: enabled ccache
State Changed: running yum
State Changed: unlock buildroot
INFO: Installed packages:
State Changed: setup
State Changed: build
INFO: Done(/home/rottmrei/rpmbuild/SRPMS/apf-1.15-2.fc17.src.rpm) Config(default) 1 minutes 33 seconds
INFO: Results and/or logs in: /var/lib/mock/fedora-17-x86_64/result
State Changed: end
[rottmrei@fedora SPECS]$ fg
-bash: fg: current: no such job





RPMLINT:
[rottmrei@fedora SPECS]$ rpmlint -i /home/rottmrei/rpmbuild/RPMS/noarch/apf-1.15-2.fc17.noarch.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
[rottmrei@fedora SPECS]$ rpmlint -i /home/rottmrei/rpmbuild/SRPMS/apf-1.15-2.fc17.src.rpm
apf.src: W: no-%build-section
The spec file does not contain a %build section.  Even if some packages don't
directly need it, section markers may be overridden in rpm's configuration to
provide additional "under the hood" functionality, such as injection of
automatic -debuginfo subpackages.  Add the section, even if empty.

1 packages and 0 specfiles checked; 0 errors, 1 warnings.





(In reply to comment #6)
> Pretty much everything in this package is installed in the wrong place.  You
I have heavily changed all the target locations according the filesystem hierarchy standard and symlinked them.

> BuildRoot, the first line of %install, the entire %clean section and the
> %defattr line in %files are completely unnecessary in Fedora and should be
> removed.
Removed them.

> You don't need an empty %build section at all.
Removed it, however rpmlint shows this as a warning.

> Packages should not depend on the base php package.  There's a draft about
> this here: http://fedoraproject.org/wiki/PackagingDrafts/PHP which should
> make its way into the actual guideline relatively soon.  Since this does
> include an apache conf file, depending on httpd is OK.
I've changed the php dependency to mod_php

> Why does this package require a local mysql server?  Wouldn't it work with a
> remote server?
Good point. Only php-mysql is needed.

> Macro forms of basic comments like %{__rm} should not be used.  Just use
> "rm" instead; it's a whole lot less typing, and we're not going to remove rm
> from the path pretty much ever.
Got rid of them.

> You call dos2unix without having any dependency on it.
Added dos2unix under BuildRequires.

> It's pretty suboptimal to do selinux setup in scriptlets.  It would be
> better, once a proper location for the files is chosen, to get the base
> selinux package to include the proper file contexts.  The selinux folks are
> very responsive.
Good point. Will get in touch with the selinux folks. Until the proper rules get included, I will leave the selinux config in place.

> You should use a proper systemd call to restart httpd, and include the
> proper dependencies for it, if indeed you really feel like doing that. 
> Personally I don't think that installing this package should mess with a
> running httpd, and the other webapps I checked don't do that.
Removed it. You're right. This is much safer.
>
> There's no need at all to do this:
>   %dir %{_localstatedir}/www/apf-%{version}/
>   %{_localstatedir}/www/apf-%{version}/*
> Instead, just use one line:
>   %{_localstatedir}/www/apf-%{version}/
Tried to remove them, however to follow the FHS, things got more complex.

> You shoud put the README files somewhere under docdir instead of spreading
> through all through the application directory, unless there's some specific
> reason they need to be where they are.
Done.

Comment 8 Reiner Rottmann 2012-07-13 15:09:11 UTC
As I still need sponsorship, I did an unofficial review of bz#838720 and tried to help in bz#807821.

Comment 9 Jason Tibbitts 2012-07-13 16:29:31 UTC
Thanks for that.  I'm trying to get back to this, but free time is scarce and I'm going to be on vacation and out of reach quite soon.

Comment 10 Reiner Rottmann 2012-10-09 13:47:41 UTC
Is there anything else I could do in order to get sponsored and to succeed with the Adventure PHP Framework review?

Comment 11 Reiner Rottmann 2012-11-20 20:02:43 UTC
A new version of the Adventure PHP Framework has been released. I've updated the RPM sources and also built the RPM on Fedora 18 Alpha to see whether it still works in the new Fedora release.

Spec URL:
http://www.rottmann.it/apf/review/apf.spec

SRPM URL:
http://www.rottmann.it/apf/review/apf-1.16-2.fc18.src.rpm

RPM URL:
http://www.rottmann.it/apf/review/apf-1.16-2.fc18.noarch.rpm



MOCK:
[rottmrei@fedora18 ~]$ mock /home/rottmrei/rpmbuild/SRPMS/apf-1.16-2.fc18.src.rpm > mock.txt
Password: 
INFO: mock.py version 1.1.28 starting...
Start: init plugins
INFO: selinux enabled
Finish: init plugins
Start: run
INFO: Start(/home/rottmrei/rpmbuild/SRPMS/apf-1.16-2.fc18.src.rpm)  Config(fedora-18-x86_64)
Start: lock buildroot
Start: clean chroot
INFO: chroot (/var/lib/mock/fedora-18-x86_64) unlocked and deleted
Finish: clean chroot
Finish: lock buildroot
Start: chroot init
Start: lock buildroot
Mock Version: 1.1.28
INFO: Mock Version: 1.1.28
INFO: calling preinit hooks
INFO: enabled root cache
Start: unpacking root cache
Finish: unpacking root cache
INFO: enabled yum cache
Start: cleaning yum metadata
Finish: cleaning yum metadata
INFO: enabled ccache
Start: device setup
Finish: device setup
Start: yum update
Finish: yum update
Finish: lock buildroot
Finish: chroot init
INFO: Installed packages:
Start: build phase for apf-1.16-2.fc18.src.rpm
Start: device setup
Finish: device setup
Start: build setup for apf-1.16-2.fc18.src.rpm
Finish: build setup for apf-1.16-2.fc18.src.rpm
Start: rpmbuild -bb apf-1.16-2.fc18.src.rpm
Finish: rpmbuild -bb apf-1.16-2.fc18.src.rpm
Finish: build phase for apf-1.16-2.fc18.src.rpm
INFO: Done(/home/rottmrei/rpmbuild/SRPMS/apf-1.16-2.fc18.src.rpm) Config(default) 2 minutes 1 seconds
INFO: Results and/or logs in: /var/lib/mock/fedora-18-x86_64/result
Finish: run

RPMLINT
[rottmrei@fedora18 ~]$ rpmlint -i /home/rottmrei/rpmbuild/RPMS/noarch/apf-1.16-2.fc18.noarch.rpm 
apf.noarch: W: incoherent-version-in-changelog 1.16-1 ['1.16-2.fc18', '1.16-2']
The latest entry in %changelog contains a version identifier that is not
coherent with the epoch:version-release tuple of the package.

1 packages and 0 specfiles checked; 0 errors, 1 warnings.
[rottmrei@fedora18 ~]$ rpmlint -i /home/rottmrei/rpmbuild/SRPMS/apf-1.16-2.fc18.src.rpm          
apf.src: W: no-%build-section
The spec file does not contain a %build section.  Even if some packages don't
directly need it, section markers may be overridden in rpm's configuration to
provide additional "under the hood" functionality, such as injection of
automatic -debuginfo subpackages.  Add the section, even if empty.

1 packages and 0 specfiles checked; 0 errors, 1 warnings.

Comment 12 Orion Poplawski 2013-10-20 05:02:04 UTC
I'll take this.  Are you still interested in packaging it?  Looks like 1.17 is out.

Comment 13 Reiner Rottmann 2013-10-20 06:45:02 UTC
Thank you! I am still interested in packaging the APF. Of course I will recreate the RPM with the current version.

Comment 14 Reiner Rottmann 2013-10-21 10:01:50 UTC
Spec URL:
http://www.rottmann.it/apf/review/apf.spec

SRPM URL:
http://www.rottmann.it/apf/review/apf-1.17-1.fc19.src.rpm

RPM URL:
http://www.rottmann.it/apf/review/apf-1.17-1.fc19.noarch.rpm



MOCK:
[rottmrei@fedora19 SRPMS]$ mock apf-1.17-1.fc19.src.rpm 
You are attempting to run "mock" which requires administrative
privileges, but more information is needed in order to do so.
Authenticating as "root"
Password: 
INFO: mock.py version 1.1.33 starting...
Start: init plugins
INFO: selinux enabled
Finish: init plugins
Start: run
INFO: Start(apf-1.17-1.fc19.src.rpm)  Config(fedora-19-x86_64)
Start: lock buildroot
Start: clean chroot
Finish: clean chroot
Finish: lock buildroot
Start: chroot init
Start: lock buildroot
Mock Version: 1.1.33
INFO: Mock Version: 1.1.33
INFO: calling preinit hooks
INFO: enabled root cache
INFO: enabled yum cache
Start: cleaning yum metadata
Finish: cleaning yum metadata
INFO: enabled ccache
Start: device setup
Finish: device setup
Start: yum update
Start: Outputting list of available packages
Finish: Outputting list of available packages
Finish: yum update
Start: creating cache
Finish: creating cache
Finish: lock buildroot
Finish: chroot init
INFO: Installed packages:
Start: build phase for apf-1.17-1.fc19.src.rpm
Start: device setup
Finish: device setup
Start: build setup for apf-1.17-1.fc19.src.rpm
Finish: build setup for apf-1.17-1.fc19.src.rpm
Start: rpmbuild -bb apf-1.17-1.fc19.src.rpm
Start: Outputting list of installed packages
Finish: Outputting list of installed packages
Finish: rpmbuild -bb apf-1.17-1.fc19.src.rpm
Finish: build phase for apf-1.17-1.fc19.src.rpm
INFO: Done(apf-1.17-1.fc19.src.rpm) Config(default) 5 minutes 14 seconds
INFO: Results and/or logs in: /var/lib/mock/fedora-19-x86_64/result
Finish: run



RPMLINT:
[rottmrei@fedora19 ~]$ rpmlint -i /home/rottmrei/rpmbuild/RPMS/noarch/apf-1.17-1.fc19.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 15 Christopher Meng 2013-10-21 10:27:43 UTC
1. You'd better leave a blank line for each changelog, this will let us see changes clearly.

2. %{_datarootdir} --> %{_datadir} is more common.

3. Remove old EL5 stuffs:

rm -rf %{buildroot} in %install

%clean

Comment 16 Orion Poplawski 2013-10-21 18:16:58 UTC
What he said :)  Also, no need for the mock output, it's not very useful (other than noting that it builds).  I'll do a full review after the above is fixed.

Comment 17 Reiner Rottmann 2013-10-21 18:23:44 UTC
Thanks for the comments. Made the changes and uploaded the following files:

Spec URL:
http://www.rottmann.it/apf/review/apf.spec

SRPM URL:
http://www.rottmann.it/apf/review/apf-1.17-2.fc19.src.rpm

RPM URL:
http://www.rottmann.it/apf/review/apf-1.17-2.fc19.noarch.rpm

Comment 18 Orion Poplawski 2013-10-21 18:32:07 UTC
What's your FAS account name?

Comment 19 Reiner Rottmann 2013-10-21 18:33:29 UTC
rrottmann

Comment 20 Orion Poplawski 2013-10-21 18:55:12 UTC
- The first show stopper is the license for apps/extensions/htmlheader/biz/JSMin.php:

* Copyright (c) 2002 Douglas Crockford (www.crockford.com)
*
* Permission is hereby granted, free of charge, to any person obtaining a copy of
* this software and associated documentation files (the "Software"), to deal in
* the Software without restriction, including without limitation the rights to
* use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
* of the Software, and to permit persons to whom the Software is furnished to do
* so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in all
* copies or substantial portions of the Software.
*
* The Software shall be used for Good, not Evil.

This last clause is unenforceable and is as such forbidden from Fedora.  See bug #455407.

- If (and only if) the source package includes the text of the license(s) in
  its own file, then that file, containing the text of the license(s) for the
  package is included in %doc.
  Note: Cannot find LICENSE in rpm(s)
  See:
  http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text

  Add %doc *.txt to %files to take care of this and ship the readmes.

  Also I don't think the License field is correct (even excluding the JSMin.php issue).  

- Looks like it bundles apps/modules/recaptcha/external/google/recaptchalib.php.  That will need to be packaged separately.

- Remove %clean completely, it is not needed

Comment 21 Reiner Rottmann 2013-10-21 19:39:21 UTC
Ok, I understand the issue. I make contact with the author whether it is possible to remove parts that inflict license issues during packaging - or even better in the APF release. As the framework is very modular, I think that would be an option.

Thanks for pointing out the licensing issues. I took the information from the download page where I got the source file. Seems like the license was migrated  to the LGPL and the text changed on the APF homepage.

I will ask the author whether the transition is complete now and adapt the license in the spec file.

Thank you for the fast review!

Comment 22 Reiner Rottmann 2013-10-23 19:20:55 UTC
Got feedback from the APF devs. The APF only uses the LGPL now. The recaptchalib.php and the JSMin.php will be made optional so that the Fedora package may omit them. A feature request has been filed and will be worked on in a couple of days.

Comment 23 Reiner Rottmann 2014-02-02 08:25:06 UTC
I've tested a beta release in November that made files with "problematic" licenses an optional download.

Meanwhile a new stable version has been released and I recreated the RPM:

Spec URL:
http://www.rottmann.it/apf/review/apf.spec

SRPM URL:
http://www.rottmann.it/apf/review/apf-2.0-2.fc19.src.rpm

A koji scratch build for Rawhide succeeded:

[rottmrei@vagrant-f19 SRPMS]$ koji build --scratch rawhide apf-2.0-2.fc19.src.rpm 
Uploading srpm: apf-2.0-2.fc19.src.rpm
[====================================] 100% 00:00:06   1.16 MiB 172.90 KiB/sec
Created task: 6480615
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=6480615
Watching tasks (this may be safely interrupted)...
6480615 build (rawhide, apf-2.0-2.fc19.src.rpm): open (arm02-builder22.arm.fedoraproject.org)
  6480616 buildArch (apf-2.0-2.fc19.src.rpm, noarch): open (buildvm-10.phx2.fedoraproject.org)
  6480616 buildArch (apf-2.0-2.fc19.src.rpm, noarch): open (buildvm-10.phx2.fedoraproject.org) -> closed
  0 free  1 open  1 done  0 failed
6480615 build (rawhide, apf-2.0-2.fc19.src.rpm): open (arm02-builder22.arm.fedoraproject.org) -> closed
  0 free  0 open  2 done  0 failed

6480615 build (rawhide, apf-2.0-2.fc19.src.rpm) completed successfully

Comment 24 Orion Poplawski 2014-02-02 22:16:51 UTC
- I believe the license should be "LGPLv3+"
- Remove %clean *completely*
- My last initial concern is whether we are unbundling jquery yet from packages, I've posted a message on the packaging list about that.  Once I've heard pack on that, I'll do the full review.

Comment 25 Reiner Rottmann 2014-06-09 13:57:21 UTC
Any news about the unbundling of jquery, yet?

Comment 26 Orion Poplawski 2015-01-06 02:57:55 UTC
Still not ready to unbundle jquery.  We should make sure that the bundled version is new, and not affected by recent CVEs.  Sorry for the delay.  I don't see the spec/src.rpm files anymore, still interested?

Comment 27 Reiner Rottmann 2015-01-06 12:52:17 UTC
I am still interested. I've updated the RPM/SPEC to include the latest version.
I've informed the project that there are CVEs in the jquery code and they will fix that this week.

http://www.rottmann.it/apf/review/apf.spec

http://www.rottmann.it/apf/review/apf-2.1-1.fc20.src.rpm

Comment 28 Orion Poplawski 2015-01-31 03:13:40 UTC
Any word on CVE fixes upstream?

Comment 29 Orion Poplawski 2015-01-31 04:15:57 UTC
rpmlint:
apf.noarch: E: non-executable-script /usr/share/adventure-php-framework/APF/migration/migrate-code.sh 0644L /bin/bash
apf.noarch: E: non-executable-script /usr/share/adventure-php-framework/APF/migration/relocate.sh 0644L /bin/bash
apf.noarch: E: non-executable-script /usr/share/adventure-php-framework/APF/migration/migrate-config.sh 0644L /bin/bash

- Was upstream ever notified about the line ending issue? (Just wondering if dos2unix is still needed - seems like a tarball should have the proper line-endings).

- I (still) believe the license should be "LGPLv3+"

- I don't think the jquery.ui vulnerability applies - doesn't look to use jquery.ui.

Comment 30 Orion Poplawski 2016-11-24 04:48:35 UTC
Closed due to inactivity.


Note You need to log in before you can comment on or make changes to this bug.