Spec URL: https://download.copr.fedorainfracloud.org/results/jered/trafficserver/fedora-rawhide-x86_64/04324060-trafficserver/trafficserver.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/jered/trafficserver/fedora-rawhide-x86_64/04324060-trafficserver/trafficserver-9.1.2-3.fc37.src.rpm Description: Traffic Server is a high-performance building block for cloud services. It's more than just a caching proxy server; it also has support for plugins to build large scale web applications. Key features: Caching - Improve your response time, while reducing server load and bandwidth needs by caching and reusing frequently-requested web pages, images, and web service calls. Proxying - Easily add keep-alive, filter or anonymize content requests, or add load balancing by adding a proxy layer. Fast - Scales well on modern SMP hardware, handling 10s of thousands of requests per second. Extensible - APIs to write your own plug-ins to do anything from modifying HTTP headers to handling ESI requests to writing your own cache algorithm. Proven - Handling over 400TB a day at Yahoo! both as forward and reverse proxies, Apache Traffic Server is battle hardened. Fedora Account System Username: jered
I've added a github repo for the packaging components: https://github.com/jeredfloyd/rpm-trafficserver
Bumped github and copr repo to 9.1.2-3 due to minor SELinux change to address RHEL 8 AVC warnings.
Taking this review.
I've updated the links in the description so I can run fedora-review. But for future reference, please use direct links to the SPEC and SRPM that fedora-review can consume. When updating, please make a new comment with Spec URL and SRPM URL lines pointing to updated files.
Initial spec review: > # Upstream does not support 32-bit architectures: > # https://github.com/apache/trafficserver/issues/4432 > ExcludeArch: armhfp Please use "%{arm} %{ix86}" here (which expands to all 32-bit arm and x86 arches), "armhfp" isn't a real architecture. (I'd prefer to use "%{arm32}" for 32-bit arm arches, but that only exists in RHEL9+ right now, and nobody backported it to EPEL 8 yet...) > # trafficserver does not work properly with OpenSSL 3.0.2 yet > # Upstream issue: https://github.com/apache/trafficserver/issues/7341 > %if 0%{?fedora} >= 36 || 0%{?rhel} >= 9 > BuildRequires: openssl1.1-devel > %else > BuildRequires: openssl-devel > %endif [...] > %if 0%{?fedora} >= 36 || 0%{?rhel} >= 9 > Requires: openssl1.1 > %else > Requires: openssl > # Require an OpenSSL which supports PROFILE=SYSTEM > Conflicts: openssl-libs < 1:1.0.1h-4 > %endif openssl1.1-devel does not exist in RHEL/EPEL 9, so you cannot build for RHEL 9. The situation around openssl1.1 is weird. There's compat-openssl11 in RHEL 9, but it doesn't provide devel content, and it's unusually crippled (no FIPS support, for example). I'm not sure what we should do in EPEL about it. > --enable-layout=Gentoo \ Umm, what? > %ghost %verify(not md5 size mode mtime) %{_sharedstatedir}/selinux/%{selinuxtype}/active/modules/200/%{modulename} The %verify stanza is redundant for a %ghost file (which already doesn't do any verification)
(In reply to Neal Gompa from comment #5) > > ExcludeArch: armhfp > > Please use "%{arm} %{ix86}" here (which expands to all 32-bit arm and x86 > arches), "armhfp" isn't a real architecture. Will do. > > # trafficserver does not work properly with OpenSSL 3.0.2 yet > > # Upstream issue: https://github.com/apache/trafficserver/issues/7341 > > %if 0%{?fedora} >= 36 || 0%{?rhel} >= 9 > > openssl1.1-devel does not exist in RHEL/EPEL 9, so you cannot build for RHEL > 9. The situation around openssl1.1 is weird. There's compat-openssl11 in > RHEL 9, but it doesn't provide devel content, and it's unusually crippled > (no FIPS support, for example). I'm not sure what we should do in EPEL about > it. Ideally we fix this upstream so we can use OpenSSL 3.0 -- I haven't looked into this yet. Are you recommending I remove the "|| 0%{?rhel} >= 9" condition for now? > > --enable-layout=Gentoo \ > > Umm, what? It's not possible to get all the files installed in the right locations with command-line flags to configure, and the "Gentoo" layout for config.layout is exactly correct. If you'd prefer I can duplicate the config stanza and call it redhat? I fretted over that for a few minutes. See upstream: https://github.com/apache/trafficserver/blob/master/config.layout > > %ghost %verify(not md5 size mode mtime) %{_sharedstatedir}/selinux/%{selinuxtype}/active/modules/200/%{modulename} > > The %verify stanza is redundant for a %ghost file (which already doesn't do > any verification) This is directly from the Fedora doc here: https://fedoraproject.org/wiki/SELinux/IndependentPolicy#The_.25files_Section Are you suggesting a change to just: %ghost %{_sharedstatedir}/selinux/%{selinuxtype}/active/modules/200/%{modulename} ?
Upon more research, it looks that %ghost + %verify is for post-install verification (e.g. rpm -V) behavior; so I think this is correct as written.
(In reply to Jered Floyd from comment #6) > (In reply to Neal Gompa from comment #5) > > > > ExcludeArch: armhfp > > > > Please use "%{arm} %{ix86}" here (which expands to all 32-bit arm and x86 > > arches), "armhfp" isn't a real architecture. > > Will do. > > > > # trafficserver does not work properly with OpenSSL 3.0.2 yet > > > # Upstream issue: https://github.com/apache/trafficserver/issues/7341 > > > %if 0%{?fedora} >= 36 || 0%{?rhel} >= 9 > > > > openssl1.1-devel does not exist in RHEL/EPEL 9, so you cannot build for RHEL > > 9. The situation around openssl1.1 is weird. There's compat-openssl11 in > > RHEL 9, but it doesn't provide devel content, and it's unusually crippled > > (no FIPS support, for example). I'm not sure what we should do in EPEL about > > it. > > Ideally we fix this upstream so we can use OpenSSL 3.0 -- I haven't looked > into this yet. Are you recommending I remove the "|| 0%{?rhel} >= 9" > condition for now? > > > > > --enable-layout=Gentoo \ > > > > Umm, what? > > It's not possible to get all the files installed in the right locations with > command-line flags to configure, and the "Gentoo" layout for config.layout > is exactly correct. If you'd prefer I can duplicate the config stanza and > call it redhat? I fretted over that for a few minutes. See upstream: > https://github.com/apache/trafficserver/blob/master/config.layout > The Gentoo layout isn't correct. The libexecdir is /usr/libexec on Red Hat systems. It's actually also that now on Debian systems, too. Also, libraries go into /usr/lib64 on 64-bit arches on Red Hat systems, not /usr/lib...
(In reply to Jered Floyd from comment #6) > (In reply to Neal Gompa from comment #5) > > > # trafficserver does not work properly with OpenSSL 3.0.2 yet > > > # Upstream issue: https://github.com/apache/trafficserver/issues/7341 > > > %if 0%{?fedora} >= 36 || 0%{?rhel} >= 9 > > > > openssl1.1-devel does not exist in RHEL/EPEL 9, so you cannot build for RHEL > > 9. The situation around openssl1.1 is weird. There's compat-openssl11 in > > RHEL 9, but it doesn't provide devel content, and it's unusually crippled > > (no FIPS support, for example). I'm not sure what we should do in EPEL about > > it. > > Ideally we fix this upstream so we can use OpenSSL 3.0 -- I haven't looked > into this yet. Are you recommending I remove the "|| 0%{?rhel} >= 9" > condition for now? > > Yeah, don't use OpenSSL 1.1 for RHEL 9. > > > > %ghost %verify(not md5 size mode mtime) %{_sharedstatedir}/selinux/%{selinuxtype}/active/modules/200/%{modulename} > > > > The %verify stanza is redundant for a %ghost file (which already doesn't do > > any verification) > > This is directly from the Fedora doc here: > https://fedoraproject.org/wiki/SELinux/IndependentPolicy#The_. > 25files_Section > > Are you suggesting a change to just: > %ghost > %{_sharedstatedir}/selinux/%{selinuxtype}/active/modules/200/%{modulename} > > ? That policy is a draft, and to the best of my knowledge hasn't been agreed upon yet.
Spec URL: https://download.copr.fedorainfracloud.org/results/jered/trafficserver/fedora-rawhide-x86_64/04356235-trafficserver/trafficserver.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/jered/trafficserver/fedora-rawhide-x86_64/04356235-trafficserver/trafficserver-9.1.2-4.fc37.src.rpm I've made all of the suggested changes EXCEPT removing the %verify clause. Without that, I've confirmed that "rpm -V" on the package fails: # rpm -V trafficserver-selinux .M....... g /var/lib/selinux/targeted/active/modules/200/trafficserver I am also submitting the config.layout patch upstream.
> --libexecdir=%{_libdir}/%{name}/plugins \ Why are plugins in libexecdir? This seems very wrong...
(In reply to Neal Gompa from comment #11) > > --libexecdir=%{_libdir}/%{name}/plugins \ > > Why are plugins in libexecdir? This seems very wrong... Because this is where upstream puts them; you'll note that this makes sure they are installed in /usr/lib64/traffficserver/plugins. Should they be someplace else?
(In reply to Jered Floyd from comment #12) > (In reply to Neal Gompa from comment #11) > > > --libexecdir=%{_libdir}/%{name}/plugins \ > > > > Why are plugins in libexecdir? This seems very wrong... > > Because this is where upstream puts them; you'll note that this makes sure > they are installed in /usr/lib64/traffficserver/plugins. Should they be > someplace else? If they're library files, that makes sense, but it shouldn't be what --libexecdir is pointed to. --libexecdir is for helper programs, so this is a misuse of an option.
(In reply to Neal Gompa from comment #13) > (In reply to Jered Floyd from comment #12) > > (In reply to Neal Gompa from comment #11) > > > > --libexecdir=%{_libdir}/%{name}/plugins \ > > > > > > Why are plugins in libexecdir? This seems very wrong... > > > > Because this is where upstream puts them; you'll note that this makes sure > > they are installed in /usr/lib64/traffficserver/plugins. Should they be > > someplace else? > > If they're library files, that makes sense, but it shouldn't be what > --libexecdir is pointed to. --libexecdir is for helper programs, so this is > a misuse of an option. To clarify, plugins should use $(LIBDIR)/$(NAME)/plugins rather than $(LIBEXECDIR) (that's in makefile variable parlance, obviously it should be however the autofoo structures it)
I agree that makes sense, but that's not what upstream does. https://github.com/apache/trafficserver/blob/master/README: [...] DEFAULT DIR CONTENTS /usr/local/var/log/trafficserver log files created at runtime /usr/local/var/trafficserver runtime files /usr/local/etc/trafficserver configuration files /usr/local/bin executable binaries /usr/local/libexec/trafficserver plugins [...] So this option in the spec file is to make the upstream configure do exactly what you are suggesting. I can make the case upstream that they are wrong, but that's separate from packaging to meet Fedora guidelines.
(And, yes, they are shlibs not helper apps -- so they absolutely belong in /usr/lib64/trafficserver and not /usr/libexec/...)
Hi, Neal! Any other comments or concerns?
(In reply to Jered Floyd from comment #17) > Hi, Neal! Any other comments or concerns? Could you put a comment in the spec about this, and a link to an upstream discussion about it if you plan to start one?
> --enable-layout=Redhat \ Also, me being nitpicky, this should be RedHat since it's two words :)
Spec URL: https://download.copr.fedorainfracloud.org/results/jered/trafficserver/fedora-rawhide-x86_64/04368071-trafficserver/trafficserver.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/jered/trafficserver/fedora-rawhide-x86_64/04368071-trafficserver/trafficserver-9.1.2-5.fc37.src.rpm Done, and done.
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated Issues: ======= - No %config files under /usr. Note: %config(noreplace) /usr/lib/systemd/system/trafficserver.service%config(noreplace) /usr/lib/sysusers.d/trafficserver.conf%config(noreplace) /usr/lib/tmpfiles.d/trafficserver.conf See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#_configuration_files - Package does not use a name that already exists. Note: A package with this name already exists. Please check https://src.fedoraproject.org/rpms/trafficserver See: https://docs.fedoraproject.org/en-US/packaging- guidelines/Naming/#_conflicting_package_names ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "*No copyright* Apache License 2.0", "Apache License 2.0 [generated file]", "Apache License 2.0", "FSF Unlimited License (with License Retention) [generated file]", "FSF Unlimited License [generated file]", "FSF All Permissive License", "GNU General Public License v3.0 or later", "FSF Unlimited License (with License Retention) GNU General Public License, Version 2", "FSF Unlimited License (with License Retention)", "GNU General Public License v2.0 or later", "GNU General Public License v2.0 or later [generated file]", "X11 License [generated file]", "MIT License", "BSD 3-Clause License", "ISC License BSD 3-Clause License Apache License 2.0", "ISC License BSD 3-Clause License", "Boost Software License 1.0", "*No copyright* BSD 2-Clause License", "BSD 2-Clause License". 861 files have unknown license. Detailed output of licensecheck in /home/ngompa/2076374-trafficserver/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [!]: Package requires other packages for directories it uses. Note: No known owner of /var/lib/selinux/targeted/active/modules/200, /usr/share/selinux/packages/targeted, /usr/share/perl5/Apache/TS, /usr/share/perl5/Apache/TS/Config, /usr/share/selinux/devel/include/distributed, /usr/share/perl5/Apache, /usr/include/tscpp [!]: Package must own all directories that it creates. Note: Directories without known owners: /usr/include/tscpp, /usr/share/perl5/Apache/TS/Config, /usr/share/selinux/devel/include/distributed, /usr/share/selinux/devel, /var/lib/selinux/targeted/active/modules/200, /usr/share/perl5/Apache, /usr/share/selinux/devel/include, /usr/share/selinux/packages/targeted, /usr/share/perl5/Apache/TS [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [x]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [-]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 51200 bytes in 6 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: 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 %license. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: %config files are marked noreplace or the reason is justified. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: systemd_post is invoked in %post, systemd_preun in %preun, and systemd_postun in %postun for Systemd service files. Note: Systemd service file(s) in trafficserver [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local Perl: [!]: Package contains the mandatory BuildRequires and Requires:. Note: Requires: perl(:MODULE_COMPAT_%(eval "`%{__perl} -V:version`"; echo $version)) missing? ===== SHOULD items ===== Generic: [!]: Uses parallel make %{?_smp_mflags} macro. [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [!]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in trafficserver-selinux , trafficserver-devel , trafficserver-perl [x]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Patches link to upstream bugs/comments/lists or are otherwise justified. [x]: Scriptlets must be sane, if used. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: Sources 3, 4, 5, 6, 7, 8, 9 and 10 are not passed to gpgverify. [-]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Files in /run, var/run and /var/lock uses tmpfiles.d when appropriate [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: The placement of pkgconfig(.pc) files are correct. [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: There are rpmlint messages (see attachment). [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Cannot parse rpmlint output: Rpmlint (debuginfo) ------------------- Cannot parse rpmlint output: Rpmlint (installed packages) ---------------------------- Cannot parse rpmlint output: Unversioned so-files -------------------- trafficserver: /usr/lib64/trafficserver/libtscore.so trafficserver: /usr/lib64/trafficserver/libtscppapi.so trafficserver: /usr/lib64/trafficserver/libtscpputil.so trafficserver: /usr/lib64/trafficserver/libtsmgmt.so trafficserver: /usr/lib64/trafficserver/plugins/access_control.so trafficserver: /usr/lib64/trafficserver/plugins/acme.so trafficserver: /usr/lib64/trafficserver/plugins/authproxy.so trafficserver: /usr/lib64/trafficserver/plugins/background_fetch.so trafficserver: /usr/lib64/trafficserver/plugins/cache_fill.so trafficserver: /usr/lib64/trafficserver/plugins/cache_promote.so trafficserver: /usr/lib64/trafficserver/plugins/cache_range_requests.so trafficserver: /usr/lib64/trafficserver/plugins/cachekey.so trafficserver: /usr/lib64/trafficserver/plugins/cert_reporting_tool.so trafficserver: /usr/lib64/trafficserver/plugins/certifier.so trafficserver: /usr/lib64/trafficserver/plugins/collapsed_forwarding.so trafficserver: /usr/lib64/trafficserver/plugins/combo_handler.so trafficserver: /usr/lib64/trafficserver/plugins/compress.so trafficserver: /usr/lib64/trafficserver/plugins/conf_remap.so trafficserver: /usr/lib64/trafficserver/plugins/cookie_remap.so trafficserver: /usr/lib64/trafficserver/plugins/custom_redirect.so trafficserver: /usr/lib64/trafficserver/plugins/escalate.so trafficserver: /usr/lib64/trafficserver/plugins/esi.so trafficserver: /usr/lib64/trafficserver/plugins/fq_pacing.so trafficserver: /usr/lib64/trafficserver/plugins/generator.so trafficserver: /usr/lib64/trafficserver/plugins/geoip_acl.so trafficserver: /usr/lib64/trafficserver/plugins/header_freq.so trafficserver: /usr/lib64/trafficserver/plugins/header_rewrite.so trafficserver: /usr/lib64/trafficserver/plugins/healthchecks.so trafficserver: /usr/lib64/trafficserver/plugins/hook-trace.so trafficserver: /usr/lib64/trafficserver/plugins/icap.so trafficserver: /usr/lib64/trafficserver/plugins/inliner.so trafficserver: /usr/lib64/trafficserver/plugins/ja3_fingerprint.so trafficserver: /usr/lib64/trafficserver/plugins/libloader.so trafficserver: /usr/lib64/trafficserver/plugins/memory_profile.so trafficserver: /usr/lib64/trafficserver/plugins/metalink.so trafficserver: /usr/lib64/trafficserver/plugins/money_trace.so trafficserver: /usr/lib64/trafficserver/plugins/mp4.so trafficserver: /usr/lib64/trafficserver/plugins/multiplexer.so trafficserver: /usr/lib64/trafficserver/plugins/prefetch.so trafficserver: /usr/lib64/trafficserver/plugins/rate_limit.so trafficserver: /usr/lib64/trafficserver/plugins/redo_cache_lookup.so trafficserver: /usr/lib64/trafficserver/plugins/regex_remap.so trafficserver: /usr/lib64/trafficserver/plugins/regex_revalidate.so trafficserver: /usr/lib64/trafficserver/plugins/remap_purge.so trafficserver: /usr/lib64/trafficserver/plugins/remap_stats.so trafficserver: /usr/lib64/trafficserver/plugins/s3_auth.so trafficserver: /usr/lib64/trafficserver/plugins/server_push_preload.so trafficserver: /usr/lib64/trafficserver/plugins/slice.so trafficserver: /usr/lib64/trafficserver/plugins/sslheaders.so trafficserver: /usr/lib64/trafficserver/plugins/statichit.so trafficserver: /usr/lib64/trafficserver/plugins/stats_over_http.so trafficserver: /usr/lib64/trafficserver/plugins/stream_editor.so trafficserver: /usr/lib64/trafficserver/plugins/system_stats.so trafficserver: /usr/lib64/trafficserver/plugins/tcpinfo.so trafficserver: /usr/lib64/trafficserver/plugins/tls_bridge.so trafficserver: /usr/lib64/trafficserver/plugins/traffic_dump.so trafficserver: /usr/lib64/trafficserver/plugins/tsmemcache.so trafficserver: /usr/lib64/trafficserver/plugins/url_sig.so trafficserver: /usr/lib64/trafficserver/plugins/xdebug.so Source checksums ---------------- https://github.com/catchorg/Catch2/releases/download/v2.13.8/catch.hpp : CHECKSUM(SHA256) this package : f5adf6a4c3237920421d51a11112904084043b015acf2c9058e8f36b92ab2921 CHECKSUM(SHA256) upstream package : f5adf6a4c3237920421d51a11112904084043b015acf2c9058e8f36b92ab2921 https://downloads.apache.org/trafficserver/KEYS : CHECKSUM(SHA256) this package : 8414a52c96be4cf791609b37dd87ef09d9fd16f19c0667a6166882b88ee83b07 CHECKSUM(SHA256) upstream package : 8414a52c96be4cf791609b37dd87ef09d9fd16f19c0667a6166882b88ee83b07 http://www.apache.org/dist/trafficserver/trafficserver-9.1.2.tar.bz2.asc : CHECKSUM(SHA256) this package : 4b98544e899c4a76d455376c0468048e3b4fdb347d9fe66ad85049bc937fc9c6 CHECKSUM(SHA256) upstream package : 4b98544e899c4a76d455376c0468048e3b4fdb347d9fe66ad85049bc937fc9c6 http://www.apache.org/dist/trafficserver/trafficserver-9.1.2.tar.bz2 : CHECKSUM(SHA256) this package : 62f27d4e16a515e7ec85393186f909d934a79db41c7905f21d15a9eacb82232f CHECKSUM(SHA256) upstream package : 62f27d4e16a515e7ec85393186f909d934a79db41c7905f21d15a9eacb82232f Requires -------- trafficserver (rpmlib, GLIBC filtered): (trafficserver-selinux = 9.1.2-5.fc37 if selinux-policy-targeted) /bin/sh /usr/bin/perl config(trafficserver) expat hwloc ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libcap.so.2()(64bit) libcrypto.so.1.1()(64bit) libcrypto.so.1.1(OPENSSL_1_1_0)(64bit) libcurl.so.4()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3.1)(64bit) libgcc_s.so.1(GCC_3.4)(64bit) libhwloc.so.15()(64bit) liblzma.so.5()(64bit) liblzma.so.5(XZ_5.0)(64bit) libm.so.6()(64bit) libncursesw.so.6()(64bit) libpcre.so.1()(64bit) libresolv.so.2()(64bit) libssl.so.1.1()(64bit) libssl.so.1.1(OPENSSL_1_1_0)(64bit) libssl.so.1.1(OPENSSL_1_1_1)(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(CXXABI_1.3.5)(64bit) libstdc++.so.6(CXXABI_1.3.7)(64bit) libstdc++.so.6(CXXABI_1.3.8)(64bit) libstdc++.so.6(CXXABI_1.3.9)(64bit) libtinfo.so.6()(64bit) libtscore.so.9()(64bit) libtscppapi.so.9()(64bit) libtscpputil.so.9()(64bit) libtsmgmt.so.9()(64bit) libz.so.1()(64bit) libz.so.1(ZLIB_1.2.0)(64bit) ncurses openssl1.1 pcre pkgconfig rtld(GNU_HASH) systemd xz trafficserver-selinux (rpmlib, GLIBC filtered): /bin/sh libselinux-utils policycoreutils policycoreutils-python-utils selinux-policy selinux-policy-base selinux-policy-targeted trafficserver-devel (rpmlib, GLIBC filtered): /usr/bin/bash /usr/bin/pkg-config trafficserver trafficserver-perl (rpmlib, GLIBC filtered): trafficserver trafficserver-debuginfo (rpmlib, GLIBC filtered): trafficserver-debugsource (rpmlib, GLIBC filtered): Provides -------- trafficserver: config(trafficserver) group(trafficserver) libloader.so()(64bit) libtscore.so.9()(64bit) libtscppapi.so.9()(64bit) libtscpputil.so.9()(64bit) libtsmgmt.so.9()(64bit) trafficserver trafficserver(x86-64) user(trafficserver) trafficserver-selinux: trafficserver-selinux trafficserver-devel: pkgconfig(trafficserver) trafficserver-devel trafficserver-devel(x86-64) trafficserver-perl: trafficserver-perl trafficserver-perl(x86-64) trafficserver-debuginfo: debuginfo(build-id) libloader.so-9.1.2-5.fc37.x86_64.debug()(64bit) libtscore.so.9.1.2-9.1.2-5.fc37.x86_64.debug()(64bit) libtscppapi.so.9.1.2-9.1.2-5.fc37.x86_64.debug()(64bit) libtscpputil.so.9.1.2-9.1.2-5.fc37.x86_64.debug()(64bit) libtsmgmt.so.9.1.2-9.1.2-5.fc37.x86_64.debug()(64bit) trafficserver-debuginfo trafficserver-debuginfo(x86-64) trafficserver-debugsource: trafficserver-debugsource trafficserver-debugsource(x86-64) Generated by fedora-review 0.8.0 (e988316) last change: 2022-04-07 Command line :/usr/bin/fedora-review -b 2076374 -m fedora-rawhide-x86_64 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Perl, C/C++, Shell-api Disabled plugins: Java, PHP, Haskell, Python, SugarActivity, R, Ocaml, fonts Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH
Issues: ======= > - No %config files under /usr. > Note: %config(noreplace) > /usr/lib/systemd/system/trafficserver.service%config(noreplace) > /usr/lib/sysusers.d/trafficserver.conf%config(noreplace) > /usr/lib/tmpfiles.d/trafficserver.conf > See: https://docs.fedoraproject.org/en-US/packaging- > guidelines/#_configuration_files Yeah, these are not config files, drop %config stanzas here. User configuration and overrides for files in /usr/lib will be written in /etc as needed by the user and systemd will do the right thing automatically. > - Package does not use a name that already exists. > Note: A package with this name already exists. Please check > https://src.fedoraproject.org/rpms/trafficserver > See: https://docs.fedoraproject.org/en-US/packaging- > guidelines/Naming/#_conflicting_package_names This can be ignored, since you're unretiring a package. > [!]: Package requires other packages for directories it uses. > Note: No known owner of /var/lib/selinux/targeted/active/modules/200, > /usr/share/selinux/packages/targeted, /usr/share/perl5/Apache/TS, > /usr/share/perl5/Apache/TS/Config, > /usr/share/selinux/devel/include/distributed, /usr/share/perl5/Apache, > /usr/include/tscpp > [!]: Package must own all directories that it creates. > Note: Directories without known owners: /usr/include/tscpp, > /usr/share/perl5/Apache/TS/Config, > /usr/share/selinux/devel/include/distributed, > /usr/share/selinux/devel, > /var/lib/selinux/targeted/active/modules/200, /usr/share/perl5/Apache, > /usr/share/selinux/devel/include, > /usr/share/selinux/packages/targeted, /usr/share/perl5/Apache/TS The SELinux and Perl module subpackages need correct ownership of their directories. > [!]: Uses parallel make %{?_smp_mflags} macro. This is it misdetecting the SELinux module make call, it can be ignored. > [!]: Fully versioned dependency in subpackages if applicable. > Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in > trafficserver-selinux , trafficserver-devel , trafficserver-perl The selinux subpackage is fine as-is, but the rest need their "%{name} = %{version}-%{release}" dependency updated to "%{name}%{?_isa} = %{version}-%{release}".
> > [!]: Fully versioned dependency in subpackages if applicable. > > Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in > > trafficserver-selinux , trafficserver-devel , trafficserver-perl > > The selinux subpackage is fine as-is, but the rest need their "%{name} = > %{version}-%{release}" dependency updated to "%{name}%{?_isa} = > %{version}-%{release}". For trafficserver-perl, would it actually be more correct to add: BuildArch: noarch rather than adding the architecture to the dependencies?
> > [!]: Package requires other packages for directories it uses. > > Note: No known owner of /var/lib/selinux/targeted/active/modules/200, > > /usr/share/selinux/packages/targeted, /usr/share/perl5/Apache/TS, > > /usr/share/perl5/Apache/TS/Config, > > /usr/share/selinux/devel/include/distributed, /usr/share/perl5/Apache, > > /usr/include/tscpp > > [!]: Package must own all directories that it creates. > > Note: Directories without known owners: /usr/include/tscpp, > > /usr/share/perl5/Apache/TS/Config, > > /usr/share/selinux/devel/include/distributed, > > /usr/share/selinux/devel, > > /var/lib/selinux/targeted/active/modules/200, /usr/share/perl5/Apache, > > /usr/share/selinux/devel/include, > > /usr/share/selinux/packages/targeted, /usr/share/perl5/Apache/TS > > The SELinux and Perl module subpackages need correct ownership of their > directories. I agree regarding the Perl directories (and will fix, as well as /usr/include/tscpp) but I don't think this is correct for selinux. All of those are system-created directories that are not specific to this package.
(In reply to Jered Floyd from comment #24) > > > [!]: Package requires other packages for directories it uses. > > > Note: No known owner of /var/lib/selinux/targeted/active/modules/200, > > > /usr/share/selinux/packages/targeted, /usr/share/perl5/Apache/TS, > > > /usr/share/perl5/Apache/TS/Config, > > > /usr/share/selinux/devel/include/distributed, /usr/share/perl5/Apache, > > > /usr/include/tscpp > > > [!]: Package must own all directories that it creates. > > > Note: Directories without known owners: /usr/include/tscpp, > > > /usr/share/perl5/Apache/TS/Config, > > > /usr/share/selinux/devel/include/distributed, > > > /usr/share/selinux/devel, > > > /var/lib/selinux/targeted/active/modules/200, /usr/share/perl5/Apache, > > > /usr/share/selinux/devel/include, > > > /usr/share/selinux/packages/targeted, /usr/share/perl5/Apache/TS > > > > The SELinux and Perl module subpackages need correct ownership of their > > directories. > > I agree regarding the Perl directories (and will fix, as well as > /usr/include/tscpp) but I don't think this is correct for selinux. All of > those are system-created directories that are not specific to this package. Please file a bug with selinux-policy to ensure those directories are owned, then.
(In reply to Jered Floyd from comment #23) > > > [!]: Fully versioned dependency in subpackages if applicable. > > > Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in > > > trafficserver-selinux , trafficserver-devel , trafficserver-perl > > > > The selinux subpackage is fine as-is, but the rest need their "%{name} = > > %{version}-%{release}" dependency updated to "%{name}%{?_isa} = > > %{version}-%{release}". > > For trafficserver-perl, would it actually be more correct to add: > > BuildArch: noarch > > rather than adding the architecture to the dependencies? Yes. I also just noticed you should probably be using "%{perl_vendorlib}" instead of "%{_datadir}/perl5" in your file lists. You need to add BuildRequires for perl-generators and perl-interpreter too.
Spec URL: https://download.copr.fedorainfracloud.org/results/jered/trafficserver/fedora-rawhide-x86_64/04404276-trafficserver/trafficserver.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/jered/trafficserver/fedora-rawhide-x86_64/04404276-trafficserver/trafficserver-9.1.2-6.fc37.src.rpm Alright; I believe I have addressed all of your identified issues! I also added a missing perl Requires based on fedora-review feedback as well as the Perl module packaging guidelines.
> * Thu May 5 2022 Jered Floyd <jered> 9.1.2-5 There's a tab here that shouldn't be here, but you can fix this on import.
Overall, I think we're in decent shape now, so... PACKAGE APPROVED.
Thanks, Neal! I've fixed the changelog spacing issue in my repo and will roll it into the Pagure import.
This package is now in rawhide and in testing for other releases: https://packages.fedoraproject.org/pkgs/trafficserver/trafficserver/