Bug 1268910
Summary: | Review Request: h2o - HTTP server for HTTP/1.x and HTTP/2 | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | marhag87 |
Component: | Package Review | Assignee: | Nobody's working on this, feel free to take it <nobody> |
Status: | CLOSED WONTFIX | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | unspecified | ||
Version: | rawhide | CC: | carl, package-review, vedran |
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: | 2020-07-23 09:54:11 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: | |||
Bug Depends On: | |||
Bug Blocks: | 177841 |
Description
marhag87
2015-10-05 15:18:21 UTC
Unofficial review: Source URL guidelines: https://fedoraproject.org/wiki/Packaging:SourceURL?rd=Packaging/SourceURL#Git_Tags Change: https://github.com/h2o/h2o/archive/v%{version}.tar.gz to: https://github.com/h2o/h2o/archive/v%{version}.tar.gz#/%{name}-%{version}.tar.gz and update your tarball file name. A few nice-to-haves, but not blockers, are to use %autosetup instead of %setup to negate requiring %patch lines and removing %defattr(-,root,root). marhag87's scratch build of h2o-1.5.0-3.fc22.x86_64.rpm for f22 failed http://koji.fedoraproject.org/koji/taskinfo?taskID=11337281 marhag87's scratch build of h2o-1.5.0-3.fc22.src.rpm for f22 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11337302 Thanks for the feedback! I've fixed the problems you mentioned in 1.5.0-3: Spec URL: https://github.com/marhag87/h2o-rpm/blob/master/h2o.spec SRPM URL: https://github.com/marhag87/h2o-rpm-src/raw/master/h2o-1.5.0-3.fc22.src.rpm > Summary: An optimized HTTP server with support for HTTP/1.x and HTTP/2 > %description > H2O is a very fast HTTP server written in C. Why is %summary longer and more detailed than %description? Typically, the latter expands on the former: https://fedoraproject.org/wiki/Packaging:Guidelines#Summary_and_description Suggestion: Summary: HTTP server %description H2O is a very fast HTTP server written in C. It supports HTTP/1.x and HTTP/2. > %post > /sbin/ldconfig > %systemd_post %{name}.service > %postun > /sbin/ldconfig Running ldconfig here doesn't achieve anything related to this package, because this package doesn't store any shared lib in runtime linker's search path. > %doc %{_datarootdir}/doc/%{name} %_datarootdir is %_datadir, which is /usr/share, and files below /usr/share/doc are implicitly marked as %doc. See "rpm -E %__docdir_path". > %doc %{_mandir}/man1/%{name}.1.gz Same for %_mandir. > %config(noreplace) %{_sysconfdir}/%{name}/%{name}.conf > > %{_libexecdir}/%{name}/setuidgid The directories %_sysconfdir/%name and %_libexecdir/%{name} are not included yet. https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership https://fedoraproject.org/wiki/Packaging:UnownedDirectories Consider pointing the fedora-review tool at this ticket: fedora-review -b 1268910 Thanks for the comments! I have updated the spec file as per your suggestions. I couldn't quite get the fedora-review tool to work, but I'll give it another try with the updated spec file path from this comment (forgot to use raw in the last one). Spec URL: https://raw.githubusercontent.com/marhag87/h2o-rpm/master/h2o.spec SRPM URL: https://raw.githubusercontent.com/marhag87/h2o-rpm-src/master/h2o-1.5.0-4.fc22.src.rpm marhag87's scratch build of h2o-1.5.0-4.fc22.src.rpm for f22 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11339443 You may want to practise a bit more %files sections and ownership of files and directories. In particular %dir entries and inclusion of directory trees as covered here: https://fedoraproject.org/wiki/Packaging:UnownedDirectories > %{_sysconfdir}/%{name} > %config(noreplace) %{_sysconfdir}/%{name}/%{name}.conf The second line here tells that %{_sysconfdir}/%{name} is a directory, or else there could not be a %name.conf file be stored within it. But then the first line %{_sysconfdir}/%{name} includes the directory *and* anything in it. It other words, it includes the directory regardless of whether it's empty or whether it contains a huge tree of files and subdirs. > %{_libexecdir}/%{name} > %{_libexecdir}/%{name}/setuidgid Same here. Typically, rpmbuild warns about such cases that lead to files being listed twice in a %files section. From your scratch build: https://kojipkgs.fedoraproject.org//work/tasks/9447/11339447/build.log warning: File listed twice: /etc/h2o/h2o.conf warning: File listed twice: /usr/libexec/h2o/setuidgid There is more than one solution to this problem. One involves %dir attributes to include *only* an entry for a directory but not any files within it. The files then must be listed explicitly in the %files section: %dir %{_sysconfdir}/%{name} %config(noreplace) %{_sysconfdir}/%{name}/%{name}.conf %dir %{_libexecdir}/%{name} %{_libexecdir}/%{name}/setuidgid An alternative is to include a directory tree directly, but that would conflict with %config and other attributes on specific files. The guidelines give a few more examples. Also in the build.log: > No tests were found!!! That refers to the %check section. > -o h2o -rdynamic libressl-build/lib/libssl.a > libressl-build/lib/libcrypto.a -lz -lpthread -ldl -lrt That seems to be related to "%cmake -DWITH_BUNDLED_SSL=on ." and ignores the openssl-devel BuildRequires, too. I hope it's just a bug in the package and not intentional: https://fedoraproject.org/wiki/Packaging:Guidelines#Duplication_of_system_libraries As I understand it, the recent changes to the No Bundling policies are still being discussed because they are considered controversial. marhag87's scratch build of h2o-1.5.2-1.fc22.src.rpm for f22 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11516898 Thanks for the feedback! I have updated the specfile with your suggestions, and to the latest version that is available. In the link you gave they define libraries like this: "compiled third party source code resulting in shared or static linkable files, interpreted third party source code such as Python, PHP and others." The bundling for this program doesn't produce any new libraries. I have already removed the bundled SSL, but there are several other dependencies: "cloexec golombset klib libyrmcds mruby mruby-dir mruby-env mruby-iijson mruby-io mruby-onig-regexp mruby-pack mruby-require neverbleed picohttpparser picotest yaml yoml". Only yaml seems to have a package already. Should I try to poke upstream about decoupling them and work on making packages for all of them? Do you have a link to the discussion about the policy discussion? Spec URL: https://raw.githubusercontent.com/marhag87/h2o-rpm/master/h2o.spec SRPM URL: https://raw.githubusercontent.com/marhag87/h2o-rpm-src/master/h2o-1.5.2-1.fc22.src.rpm marhag87's scratch build of h2o-1.5.4-1.fc22.src.rpm for f23 completed http://koji.fedoraproject.org/koji/taskinfo?taskID=11835100 marhag87, here are the latest docs regarding bundled libraries. https://docs.fedoraproject.org/en-US/packaging-guidelines/#bundling yaml is already packaged as libyaml, so at a minimum that must be unbundled. mruby was submitted in bug 818414, but has since been closed. You should consider reviving that review and then unbundling mruby if possible. This is an automatic check from review-stats script. This review request ticket hasn't been updated for some time. We're sorry it is taking so long. If you're still interested in packaging this software into Fedora repositories, please respond to this comment clearing the NEEDINFO flag. You may want to update the specfile and the src.rpm to the latest version available and to propose a review swap on Fedora devel mailing list to increase chances to have your package reviewed. If this is your first package and you need a sponsor, you may want to post some informal reviews. Read more at https://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group. Without any reply, this request will shortly be considered abandoned and will be closed. Thank you for your patience. |