Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
Summary: Review Request: apf - Adventure PHP Framework
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Summary: Review Request: apf - Adventure PHP Framework Product: Fedora Version: rawhide Platform: All OS/Version: Linux Status: NEW Severity: medium Priority: unspecified Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: reiner@rottmann.it QAContact: extras-qa@fedoraproject.org CC: notting@redhat.com, package-review@lists.fedoraproject.org Classification: Fedora Story Points: --- Type: ---
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.
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #1 from Reiner Rottmann reiner@rottmann.it 2011-08-29 17:33:36 EDT --- FE-NEEDSPONSOR
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Reiner Rottmann reiner@rottmann.it changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |177841(FE-NEEDSPONSOR)
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #2 from Reiner Rottmann reiner@rottmann.it 2011-08-29 18:18:05 EDT --- [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
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Jason Tibbitts tibbs@math.uh.edu changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |tibbs@math.uh.edu
--- Comment #3 from Jason Tibbitts tibbs@math.uh.edu --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #4 from Reiner Rottmann reiner@rottmann.it --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #5 from Reiner Rottmann reiner@rottmann.it --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #6 from Jason Tibbitts tibbs@math.uh.edu --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #7 from Reiner Rottmann reiner@rottmann.it --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #8 from Reiner Rottmann reiner@rottmann.it --- As I still need sponsorship, I did an unofficial review of bz#838720 and tried to help in bz#807821.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #9 from Jason Tibbitts tibbs@math.uh.edu --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #10 from Reiner Rottmann reiner@rottmann.it --- Is there anything else I could do in order to get sponsored and to succeed with the Adventure PHP Framework review?
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #11 from Reiner Rottmann reiner@rottmann.it --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Orion Poplawski orion@cora.nwra.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED CC| |orion@cora.nwra.com, | |reiner@rottmann.it Assignee|nobody@fedoraproject.org |orion@cora.nwra.com Flags| |needinfo?(reiner@rottmann.i | |t)
--- Comment #12 from Orion Poplawski orion@cora.nwra.com --- I'll take this. Are you still interested in packaging it? Looks like 1.17 is out.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Reiner Rottmann reiner@rottmann.it changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(reiner@rottmann.i | |t) |
--- Comment #13 from Reiner Rottmann reiner@rottmann.it --- Thank you! I am still interested in packaging the APF. Of course I will recreate the RPM with the current version.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #14 from Reiner Rottmann reiner@rottmann.it --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |cickumqt@gmail.com
--- Comment #15 from Christopher Meng cickumqt@gmail.com --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #16 from Orion Poplawski orion@cora.nwra.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #17 from Reiner Rottmann reiner@rottmann.it --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #18 from Orion Poplawski orion@cora.nwra.com --- What's your FAS account name?
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #19 from Reiner Rottmann reiner@rottmann.it --- rrottmann
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Orion Poplawski orion@cora.nwra.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks|177841 (FE-NEEDSPONSOR) |
--- Comment #20 from Orion Poplawski orion@cora.nwra.com --- - 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
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #21 from Reiner Rottmann reiner@rottmann.it --- 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!
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #22 from Reiner Rottmann reiner@rottmann.it --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Orion Poplawski orion@cora.nwra.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(reiner@rottmann.i | |t)
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Reiner Rottmann reiner@rottmann.it changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(reiner@rottmann.i |needinfo- |t) |
--- Comment #23 from Reiner Rottmann reiner@rottmann.it --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Reiner Rottmann reiner@rottmann.it changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo- |
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #24 from Orion Poplawski orion@cora.nwra.com --- - 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.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Christopher Meng cickumqt@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC|cickumqt@gmail.com | Flags| |needinfo?(reiner@rottmann.i | |t)
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #25 from Reiner Rottmann reiner@rottmann.it --- Any news about the unbundling of jquery, yet?
https://bugzilla.redhat.com/show_bug.cgi?id=734248
--- Comment #26 from Orion Poplawski orion@cora.nwra.com --- 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?
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Reiner Rottmann reiner@rottmann.it changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(reiner@rottmann.i | |t) |
--- Comment #27 from Reiner Rottmann reiner@rottmann.it --- 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
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Orion Poplawski orion@cora.nwra.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |needinfo?(reiner@rottmann.i | |t)
--- Comment #28 from Orion Poplawski orion@cora.nwra.com --- Any word on CVE fixes upstream?
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Orion Poplawski orion@cora.nwra.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|needinfo?(reiner@rottmann.i | |t) |
--- Comment #29 from Orion Poplawski orion@cora.nwra.com --- 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.
https://bugzilla.redhat.com/show_bug.cgi?id=734248
Orion Poplawski orion@cora.nwra.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |NOTABUG Last Closed| |2016-11-23 23:48:35
--- Comment #30 from Orion Poplawski orion@cora.nwra.com --- Closed due to inactivity.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla