https://bugzilla.redhat.com/show_bug.cgi?id=1150393
Bug ID: 1150393 Summary: Review Request: tengine - A high performance web server and reverse proxy server Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: jkaluza@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: http://jkaluza.fedorapeople.org/tengine.spec SRPM URL: http://jkaluza.fedorapeople.org/tengine-2.0.3-1.fc22.src.rpm Description: Tengine is a web-server and a reverse proxy server based on Nginx project supporting many advanced features which can be used as drop-in Nginx replacement. Fedora Account System Username: jkaluza
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
--- Comment #1 from Jan Kaluža jkaluza@redhat.com --- Note that I plan to support this package only in F21+, because of nginx-filesystem dependency.
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
Marcela Mašláňová mmaslano@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |mmaslano@redhat.com
--- Comment #2 from Marcela Mašláňová mmaslano@redhat.com --- rpmlint tengine-* tengine.x86_64: W: non-standard-uid /var/log/nginx nginx tengine.x86_64: W: non-standard-gid /var/log/nginx nginx tengine.x86_64: E: non-standard-dir-perm /var/log/nginx 0700L tengine.x86_64: W: non-standard-uid /var/lib/nginx nginx tengine.x86_64: W: non-standard-gid /var/lib/nginx nginx tengine.x86_64: E: non-standard-dir-perm /var/lib/nginx 0700L tengine.x86_64: W: non-standard-uid /var/lib/nginx/tmp nginx tengine.x86_64: W: non-standard-gid /var/lib/nginx/tmp nginx tengine.x86_64: E: non-standard-dir-perm /var/lib/nginx/tmp 0700L Could you comment on permissions? I know servers have a little unusual settings. tengine.x86_64: W: dangerous-command-in-%post chmod I agree chmod should be done in the install section. tengine-devel.x86_64: W: no-documentation tengine-devel.x86_64: W: no-manual-page-for-binary dso_tool Could you create at least one man-page, which would tell at least some information?
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
--- Comment #3 from Marcela Mašláňová mmaslano@redhat.com --- Does the nginx code is copied or copied and modified? It seems to me in core/src. I guess you will need an exception and add Provides: bundled(nginx) for tracking of possible CVEs. More comments will follow...
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
--- Comment #4 from Jan Kaluža jkaluza@redhat.com --- Tengine is fork of nginx. The nginx code is patched heavily to bring new features. Upstream merges changes done by nginx upstream into Tengine when new nginx version is released.
It's the same situation as for MySQL vs. MariaDB. I don't see any bundling exception for MySQL vs. MariaDB.
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
--- Comment #5 from Marcela Mašláňová mmaslano@redhat.com --- But still it's questionable. I know why you need it, but it's ugly :-/
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
--- Comment #6 from Marcela Mašláňová mmaslano@redhat.com --- Other sources like 404.html 50x.html contains Powered by nginx. Shouldn't it be powered by Tengine? tengine.init contains nginx.conf and calling of nginx, which is probably fine, because Fedora will be using tengine.service. Personally, I dislike calling binary nginx. There is set a strict conflict with nginx, so it should be functionally fine.
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
--- Comment #7 from Marcela Mašláňová mmaslano@redhat.com --- Only man page is called nginx, which doesn't seem right. At least you should create link from tengine man page to this nginx.
You are missing check section in specfile. Did you think about running tests during build time? At least some? They could be conditionalized for running on local if it's not possible to run them at koji.
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
--- Comment #8 from Jan Kaluža jkaluza@redhat.com --- (In reply to Marcela Mašláňová from comment #6)
Other sources like 404.html 50x.html contains Powered by nginx. Shouldn't it be powered by Tengine?
Yes, I overlooked that, I'll fix that in next SRPM.
tengine.init contains nginx.conf and calling of nginx, which is probably fine, because Fedora will be using tengine.service. Personally, I dislike calling binary nginx. There is set a strict conflict with nginx, so it should be functionally fine.
I must admit calling the binary differently would be better, but it's upstream decision to stay binary compatible even when it comes to binary name (so you can replace nginx with tengine without even rewriting the initscript and so on).
(In reply to Marcela Mašláňová from comment #6)
Only man page is called nginx, which doesn't seem right. At least you should create link from tengine man page to this nginx.
I will create man-page for "dso_tool" and "tengine" link to "nginx".
You are missing check section in specfile. Did you think about running tests during build time? At least some? They could be conditionalized for running on local if it's not possible to run them at koji.
The problem with "make test" is that currently it needs tengine to be installed to test it. That's not possible to do during RPM creation. I will try to find out how to persuade that test-framework to test tengine without installing it, but if I won't success, I will have to follow the way used by nginx package maintainer - do not run make test during compilation.
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
--- Comment #9 from Jan Kaluža jkaluza@redhat.com --- Spec URL: http://jkaluza.fedorapeople.org/tengine-2.0.3-2.fc22.src.rpm SRPM URL: http://jkaluza.fedorapeople.org/tengine.spec
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
--- Comment #10 from Christopher Meng i@cicku.me --- You may need to revise the package since this package shares the same source code from nginx.
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
--- Comment #11 from Jan Kaluža jkaluza@redhat.com --- (In reply to Christopher Meng from comment #10)
You may need to revise the package since this package shares the same source code from nginx.
As I already stated in description of this review, Tengine is fork of nginx keeping the binary compatibility with nginx, but bringing more features. It's therefore clear the source code will be shared. It's the same situation as with mysql vs. mariadb.
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
Jan Kaluža jkaluza@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|mmaslano@redhat.com |nobody@fedoraproject.org
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
Jan Kaluža jkaluza@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
Vladimir Stackov amigo.elite@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |amigo.elite@gmail.com
--- Comment #12 from Vladimir Stackov amigo.elite@gmail.com --- I would like to see tengine in EPEL and I can become reviewer (and co-maintainer for EPEL branches) because you don't need sponsorship but I see a reason that could prevent tengine from joining Fedora package collection:
According to packaging guidelines for conflicts [1] you should contact Fedora Packaging Committee [2] because tengine may be considered as a drop-in replacement for nginx but not vice versa and this case isn't described properly in packaging guidelines.
I think that you could use alternatives [3] for tengine and this will be the best solution but this decision is not in my responsibility.
Could you contact Fedora Packaging Committee?
[1] https://fedoraproject.org/wiki/Packaging:Conflicts#Other_Uses_of_Conflicts: [2] https://fedoraproject.org/wiki/Packaging_Committee [3] https://fedoraproject.org/wiki/Packaging:Alternatives
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
Renich Bon Ciric renich@woralelandia.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |renich@woralelandia.com
--- Comment #13 from Renich Bon Ciric renich@woralelandia.com --- Any progress on this?
https://bugzilla.redhat.com/show_bug.cgi?id=1150393
Robert-André Mauchin zebob.m@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jkaluza@redhat.com, | |zebob.m@gmail.com Flags| |needinfo?(jkaluza@redhat.co | |m)
--- Comment #14 from Robert-André Mauchin zebob.m@gmail.com --- @Jan Kaluža: Are you still interested in packaging Tangine? If yes please provide a refreshed SPEC so I can continue the review.
package-review@lists.fedoraproject.org